Re: [RFC PATCH 0/3] lpfc: nodelist pointer cleanup

[Hannes Reinecke <hare@suse.de>]

On 10/18/19 11:45 PM, James Smart wrote:
> On 10/18/2019 12:50 AM, Hannes Reinecke wrote:
>> Hi James,
>>
>> trying to figure this annoying lpfc_set_rrq_active() bug
>> I've found the nodelist pointer handling in the lpfc io buffers
>> a bit strange; there's a 'ndlp' pointer, but for scsi the nodelist
>> is primarily accessed via the 'rdata' pointer (although not everywhere).
>> For NVMe it's primarily the 'ndlp' pointer, apparently, but the
>> usage is quite confusing.
>> So here's a patchset to straighten things up; it primarily moves
>> the anonymous protocol-specific structure in the io buffer to a named
>> one, and always accesses the nodelist through the protocol-specific
>> rport data structure.
>>
>> It also has the nice side-effect that the protocol-specific areas are
>> aligned now, so clearing the 'rdata' pointer on the scsi side will
>> be equivalent to clearing the 'rport' pointer on the nvme side.
>> And it reduces the size of the io buffer.
>>
>> Let me know what you think.
>>
>> Hannes Reinecke (3):
>>    lpfc: use named structure for combined I/O buffer
>>    lpfc: access nodelist through scsi-specific rdata pointer
>>    lpfc: access nvme nodelist through nvme rport structure
>>
>>   drivers/scsi/lpfc/lpfc_init.c |   2 +-
>>   drivers/scsi/lpfc/lpfc_nvme.c |  56 ++++++------
>>   drivers/scsi/lpfc/lpfc_scsi.c | 196 
>> +++++++++++++++++++++---------------------
>>   drivers/scsi/lpfc/lpfc_sli.c  |  26 +++---
>>   drivers/scsi/lpfc/lpfc_sli.h  |   6 +-
>>   5 files changed, 143 insertions(+), 143 deletions(-)
>>
> 
> Well, the problem I think you are trying to solve is ultimately the root 
> issue that is solved by this patch in the just-posted 12.6.0.0 patch set:
> [PATCH 05/16] lpfc: Fix bad ndlp ptr in xri aborted handling
> 
> As such, I'd like to see if the 12.6.0.0 patch resolves the issue before 
> going through all the shifting in your patches.
> Note: the failing routine can change as it's totally dependent on where 
> the bogus pointer value takes you.  The key is the 
> lpfc_sli4_sp_handle_abort_xri_wcqe() routine on the stack.
> 
Fair enough.

I'll be giving your patchset a spin once submitted.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                              +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)