Re: [PATCH 10/20] qla2xxx: Fix interaction issue between qla2xxx and Target Core Module

Quinn Tran
2015-12-14T21:57:33Z

[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]

Christoph, Thanks for reviewing. I¹ll withdraw this patch. Will rework
with new code and submit at a later time.

Regards,
Quinn Tran

On 12/14/15, 2:34 AM, "Christoph Hellwig" <hch@infradead.org> wrote:

>On Wed, Dec 09, 2015 at 10:07:32PM +0000, Quinn Tran wrote:
>> >Err, no. Looking into the refcount inside a kref is never the
>> >right thing to do.
>>
>> QT> even for debug purpose??
>
>No. Please treat struct kref as opaque.
>
>> QT> These bits provide indication as to where the command has traversed
>>in
>> the QLA code. Each bit is set one time. Due to the async nature of the
>> TMR code, it triggers QLA driver to repeat this specific free path in
>>the
>> double free case. This BUG_ON allows us trap it early on.
>>
>> In one of the corner case (below), I need to overloaded it + lock for
>>the
>> cleanup process.
>
>Setting bits fundamentaly is a read/modify/write cycle. You either
>need to use {set,clear,test}_bit or lock around these manipulations.
>
>> QT> The cmd->aborted flag is used to track the CMD_T_ABORT flag at TCM
>> level. If the command have been requested to be aborted by TCM or
>>already
>> aborted, we advance it to the ?free" state because our hardware have
>> already started freeing up resources associated to this
>>command/exchange.
>> In this specific case(above), a XFER RDY was aborted by the TMR.
>> Returning the cmd to TCM to generate SCSI Status would generate
>>erroneous
>> HW error due to freed resource.
>
>I really think this nees to be updated on top of Bat's changes as a
>start and re-reviewed. The amoutn of special casing and second guessing
>here is simply not sustainable in the long run.

[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 5204 bytes --]