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

Christoph Hellwig
2015-12-14T10:34:49Z

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.