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

[Vu Pham]

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.

-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
>>
>>
>