Re: [PATCH] scsi: lpfc: Fix ndlp reference leak in lpfc_release_rpi()

[Wentao Liang <vulab@iscas.ac.cn>]

Hi Paul,

Sorry for answering so late.  You asked me about the -EIO return a month
ago and I am only getting back to it now; that delay is on me.

You are right about it.  lpfc_release_rpi() cannot tell a negative errno
from a mailbox status, and the value was outside the MBX_* code space the
rest of the function returns.  lpfc_sli_issue_mbox_s4() now returns
MBXERR_ERROR for the non-polled command it cannot issue while interrupts
are disabled, which is also what lpfc_sli4_post_sync_mbox() returns when
it cannot post a command.

While going through the callers for that change I found one that does
depend on the sign: lpfc_bsg_issue_mbox() passes the raw result to bsg,
and bsg-lib only reports a negative result as an error, so a positive MBX
status reaches the ioctl caller as success.  That is already true for
MBX_NOT_FINISHED today; 2/3 fixes it so that 3/3 does not add another way
to hit it.

The series is:

  [PATCH v2 1/3] scsi: lpfc: Fix ndlp reference leak in lpfc_release_rpi()
  [PATCH v2 2/3] scsi: lpfc: Report bsg mailbox failures as an errno
  [PATCH v2 3/3] scsi: lpfc: Return MBX status when a non-polled mbox cannot be issued

1/3 is the patch you were testing.  The check itself is unchanged; it is
still `rc != MBX_SUCCESS && rc != MBX_BUSY`, the test
lpfc_config_port_post() and lpfc_issue_hb_mbox() already use for the same
purpose, and it covers both -EIO and MBXERR_ERROR.  I only rewrote the
comment above it.  3/3 is the change you asked for.  I kept 1/3 apart
because it is the leak fix I would like in stable; 2/3 and 3/3 are not
stable candidates.

One thing I saw and did not touch: NLP_UNREG_INP is set before the issue
and is not cleared when the command is not handed to the port, so a later
lpfc_release_rpi() for the same node returns early at the "1435
release_rpi SKIP UNREG" message.  That is pre-existing on the
MBX_NOT_FINISHED path too.  Clearing it would make a second unreg possible
for the same NPort, so I would rather ask than guess - tell me if you want
it in this series.

Testing: I have no lpfc adapter, so this is build tested only.  Mainline
at d48723711bae, drivers/scsi/lpfc/ builds with W=1 and no warnings.
checkpatch.pl --strict reports nothing on the three patches except
"unknown commit id" on the Fixes tag, which is an artifact of my shallow
checkout - 4430f7fd09ec is in mainline.  I could not run it on hardware,
and the changelogs say so.

Thanks,
Wentao