[ofa-general] [PATCH][REPOST] drivers/infiniband/ulp/srpt: Fixtarget data corruption

[Vladislav Bolkhovitin]

Vu Pham wrote:
> Vladislav Bolkhovitin wrote:
> 
>> Robert Pearson wrote:
>>
>>> Vlad,
>>>
>>> I think we agree. But, when we tried the experiment of running 
>>> without the
>>> local memory allocator scst hung when we did large IO operations. 
>>> Probably
>>> something simple.
>>
>> Why do you think it scst hung, not something else? Do you have 
>> evidences for that? ;) According to Vu, you can't simply switch to 
>> SCST memory allocator, because IB hardware has very limited number of 
>> available SG entries in commands (few tens), so for large request, 
>> where there are too many SG entries, they should be "iomapped" using 
>> the corresponding IB facility.
> 
> No - you can easily switch to SCST memory by set srpt's module parameter 
> mem_element=0. There is limited number of sg entries per IB work request 
> (29); however, the current srpt can submit several IB work requests to 
> cover large sg entries IO.

What's the point in the internal memory management in the SRPT driver then?

> -vu
> 
>>
>>> We can look harder. Next step for us is to sync up with Vu
>>> on a few other changes in the works.
>>>
>>> Bob Pearson
>>>
>>> -----Original Message-----
>>> From: general-bounces at lists.openfabrics.org
>>> [mailto:general-bounces at lists.openfabrics.org] On Behalf Of Vladislav
>>> Bolkhovitin
>>> Sent: Saturday, January 12, 2008 3:51 AM
>>> To: davem at systemfabricworks.com
>>> Cc: vu at mellanox.com; general at lists.openfabrics.org
>>> Subject: Re: [ofa-general] [PATCH][REPOST] drivers/infiniband/ulp/srpt:
>>> Fixtarget data corruption
>>>
>>> davem at systemfabricworks.com wrote:
>>>
>>>> This is an updated version of [PATCH] drivers/infiniband/ulp/srpt: Fix
>>>> target data corruption
>>>>
>>>> It was pointed out to me that the code to round up to a power of 2 was
>>>> not as clean as it should be, plus I extracted two unrelated patches 
>>>> and
>>>> submitted them separately.
>>>>
>>>> =====================================================================
>>>>
>>>>   Change the local buffer allocator to use a spin-lock protected linked
>>>>   list instead of an array of atomic_t used/free variables.  The 
>>>> atomic_t
>>>>   code was open to a multi-thread race between test and set.  This has
>>>>   been observed with the result that the same data buffer was used for
>>>>   more than one SCSI operation, either writing the wrong data to the 
>>>> disk
>>>>   or sending the wrong data to the initiator.
>>>
>>>
>>>
>>> I, as a main SCST developer and implementor, would suggest to 
>>> completely remove internal memory management from the SRPT driver and 
>>> use SCST memory management instead. It will provide the following 
>>> advantages:
>>>
>>> 1. Simplify SRPT driver and completely remove such kind of bugs.
>>>
>>> 2. Make SRPT target driver compatible with scst_user module, i.e. 
>>> will allow to use SRPT target driver with backstorage devices, 
>>> implemented in user space. Usual example of such devices is a VTL 
>>> (Virtual Tape Library).
>>>
>>> 3. (Most likely, since I'm not too familiar with SRPT drivers 
>>> internals, but for me it looks like so) Allow SRPT driver to reliably 
>>> work with many outstanding commands with big data transfer sizes (>=1MB)
>>>
>>> 4. Might improve performance by caching and reusing already allocated 
>>> and "iomaped" to Infiniband hardware SG vectors. Vu knows the 
>>> details, we discussed them with him. It will require some minor SCST 
>>> modifications (extending its interface with target drivers), but I'm 
>>> willing to make them if somebody ask for it.
>>>
>>> Vlad
>>> _______________________________________________
>>> general mailing list
>>> general at lists.openfabrics.org
>>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>>>
>>> To unsubscribe, please visit
>>> http://openib.org/mailman/listinfo/openib-general
>>>
>>>
>>
> 
>