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

[Vladislav Bolkhovitin]

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.

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