Re: [PATCH v4 0/5] qla2xxx: Add FC-NVMe Target support

2018-11-08 Thread Malavali, Giridhar 
Thanks James. Please let us know. 

-- Giri

On 11/8/18, 9:18 AM, "linux-scsi-ow...@vger.kernel.org on behalf of James 
Smart"  
wrote:

External Email

Madhani,

I'll be looking through it over the weekend.

-- james


On 11/8/2018 8:58 AM, Madhani, Himanshu wrote:
> Hi James,
>
> Any more review comments?
>
>> On Oct 31, 2018, at 9:40 AM, Himanshu Madhani 
 wrote:
>>
>> Hi Martin,
>>
>> This series adds support for FC-NVMe Target.
>>
>> Patch #1 adds infrastructure to support FC-NVMeT Link Service processing.
>> Patch #2 adds addes new qla_nvmet.[ch] files for FC-NVMe Target support.
>> Patch #3 has bulk of changes to add hooks into common code infrastucture 
and
>>  adds support for FC-NVMe Target LS4 processing via Purex path.
>> Patch #4 adds SysFS hook to enable NVMe Target for the port.
>>
>> Please apply them to 4.21/scsi-queue at your earliest convenience.
>>
>> Changes from v3 -> v4
>> o Rebased Series on current 4.20/scsi-queue
>> o Removed NVMET_FCTGTFEAT_{CMD|OPDONE}_IN_ISR as per James Smart's 
review comment.
>>
>> Changes from v2 -> v3
>> o Reordered patches so that each patch compiles individually and is 
bisectable.
>>
>> Changes from v1 -> v2
>> o Addressed all comments from Bart.
>> o Consolidated Patch 1 and Patch 2 into single patch.
>> o Fixed smatch warning reported by kbuild autommation.
>> o NVMe Target mode is exclusive at the moment. Cavium driver does not 
support both
>>   FCP Target and NVMe Target at the same time. This will be fixed in 
later updates.
>>
>> Thanks,
>> Himanshu
>>
>> Anil Gurumurthy (4):
>>   qla2xxx_nvmet: Add FC-NVMe Target Link Service request handling
>>   qla2xxx_nvmet: Add files for FC-NVMe Target support
>>   qla2xxx_nvmet: Add FC-NVMe Target handling
>>   qla2xxx_nvmet: Add SysFS node for FC-NVMe Target
>>
>> Himanshu Madhani (1):
>>   qla2xxx: Update driver version to 11.00.00.00-k
>>
>> drivers/scsi/qla2xxx/Makefile  |   3 +-
>> drivers/scsi/qla2xxx/qla_attr.c|  33 ++
>> drivers/scsi/qla2xxx/qla_dbg.c |   1 +
>> drivers/scsi/qla2xxx/qla_dbg.h |   2 +
>> drivers/scsi/qla2xxx/qla_def.h |  35 +-
>> drivers/scsi/qla2xxx/qla_fw.h  | 263 ++
>> drivers/scsi/qla2xxx/qla_gbl.h |  24 +-
>> drivers/scsi/qla2xxx/qla_gs.c  |  16 +-
>> drivers/scsi/qla2xxx/qla_init.c|  49 +-
>> drivers/scsi/qla2xxx/qla_iocb.c|   8 +-
>> drivers/scsi/qla2xxx/qla_isr.c | 112 -
>> drivers/scsi/qla2xxx/qla_mbx.c | 101 +++-
>> drivers/scsi/qla2xxx/qla_nvme.h|  33 --
>> drivers/scsi/qla2xxx/qla_nvmet.c   | 831 +++
>> drivers/scsi/qla2xxx/qla_nvmet.h   | 129 +
>> drivers/scsi/qla2xxx/qla_os.c  |  75 ++-
>> drivers/scsi/qla2xxx/qla_target.c  | 977 
-
>> drivers/scsi/qla2xxx/qla_target.h  |  90 
>> drivers/scsi/qla2xxx/qla_version.h |   4 +-
>> 19 files changed, 2711 insertions(+), 75 deletions(-)
>> create mode 100644 drivers/scsi/qla2xxx/qla_nvmet.c
>> create mode 100644 drivers/scsi/qla2xxx/qla_nvmet.h
>>
>> --
>> 2.12.0
>>
> Hi Martin,
>
> if there are no more review comments. Can we merge this into 
4.21/scsi-queue.
>
> Thanks,
> - Himanshu
>

Re: [PATCH] qla2xxx: Protect access to qpair members with qpair->qp_lock

2017-06-23 Thread Malavali, Giridhar 
Yes, it¹s needed. We will post a patch.

On 6/23/17, 9:34 AM, "Julia Lawall"  wrote:

>Please check on whether an unlock is neeed before line 1965.
>
>julia
>
>-- Forwarded message --
>Date: Fri, 23 Jun 2017 15:23:00 +0800
>From: kbuild test robot 
>To: kbu...@01.org
>Cc: Julia Lawall 
>Subject: Re: [PATCH] qla2xxx: Protect access to qpair members with
>qpair->qp_lock
>
>CC: kbuild-...@01.org
>In-Reply-To: <20170622134325.26931-1-jthumsh...@suse.de>
>
>Hi Johannes,
>
>[auto build test WARNING on scsi/for-next]
>[also build test WARNING on v4.12-rc6 next-20170622]
>[if your patch is applied to the wrong git tree, please drop us a note to
>help improve the system]
>
>url:
>https://github.com/0day-ci/linux/commits/Johannes-Thumshirn/qla2xxx-Protec
>t-access-to-qpair-members-with-qpair-qp_lock/20170623-123844
>base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git
>for-next
>:: branch date: 3 hours ago
>:: commit date: 3 hours ago
>
>>> drivers/scsi/qla2xxx/qla_iocb.c:1965:3-9: preceding lock on line 1952
>
>git remote add linux-review https://github.com/0day-ci/linux
>git remote update linux-review
>git checkout 4a35d720268dbe9ac016957a3c4fc644398d68ba
>vim +1965 drivers/scsi/qla2xxx/qla_iocb.c
>
>d74595278 Michael Hernandez  2016-12-12  1946  /* Only process
>protection or >16 cdb in this routine */
>d74595278 Michael Hernandez  2016-12-12  1947  if 
>(scsi_get_prot_op(cmd)
>== SCSI_PROT_NORMAL) {
>d74595278 Michael Hernandez  2016-12-12  1948  if 
>(cmd->cmd_len <= 16)
>d74595278 Michael Hernandez  2016-12-12  1949  return
>qla2xxx_start_scsi_mq(sp);
>d74595278 Michael Hernandez  2016-12-12  1950  }
>d74595278 Michael Hernandez  2016-12-12  1951
>4a35d7202 Johannes Thumshirn 2017-06-22 @1952
>   spin_lock_irqsave(>qp_lock, flags);
>4a35d7202 Johannes Thumshirn 2017-06-22  1953
>d74595278 Michael Hernandez  2016-12-12  1954  /* Setup qpair pointers 
>*/
>d74595278 Michael Hernandez  2016-12-12  1955  rsp = qpair->rsp;
>d74595278 Michael Hernandez  2016-12-12  1956  req = qpair->req;
>d74595278 Michael Hernandez  2016-12-12  1957
>d74595278 Michael Hernandez  2016-12-12  1958  /* So we know we haven't
>pci_map'ed anything yet */
>d74595278 Michael Hernandez  2016-12-12  1959  tot_dsds = 0;
>d74595278 Michael Hernandez  2016-12-12  1960
>d74595278 Michael Hernandez  2016-12-12  1961  /* Send marker if
>required */
>d74595278 Michael Hernandez  2016-12-12  1962  if (vha->marker_needed 
>!=
>0) {
>d74595278 Michael Hernandez  2016-12-12  1963  if 
>(qla2x00_marker(vha,
>req, rsp, 0, 0, MK_SYNC_ALL) !=
>d74595278 Michael Hernandez  2016-12-12  1964  QLA_SUCCESS)
>d74595278 Michael Hernandez  2016-12-12 @1965  return
>QLA_FUNCTION_FAILED;
>d74595278 Michael Hernandez  2016-12-12  1966  
>vha->marker_needed = 0;
>d74595278 Michael Hernandez  2016-12-12  1967  }
>d74595278 Michael Hernandez  2016-12-12  1968
>
>:: The code at line 1965 was first introduced by commit
>:: d74595278f4ab192af66d9e60a9087464638beee scsi: qla2xxx: Add
>multiple queue pair functionality.
>
>:: TO: Michael Hernandez 
>:: CC: Martin K. Petersen 
>
>---
>0-DAY kernel test infrastructureOpen Source Technology
>Center
>https://lists.01.org/pipermail/kbuild-all   Intel
>Corporation

Re: [PATCH] qla2xxx: don't disable a not previously enabled PCI device

2017-05-24 Thread Malavali, Giridhar 


On 5/24/17, 8:47 AM, "Bart Van Assche"  wrote:

>On Tue, 2017-05-23 at 16:50 +0200, Johannes Thumshirn wrote:
>> When pci_enable_device() or pci_enable_device_mem() fail in
>> qla2x00_probe_one() we bail out but do a call to
>> pci_disable_device(). This causes the dev_WARN_ON() in
>> pci_disable_device() to trigger, as the device wasn't enabled
>> previously.
>> 
>> So instead of taking the 'probe_out' error path we can directly return
>> *iff* one of the pci_enable_device() calls fails.
>> 
>> Additionally rename the 'probe_out' goto label's name to the more
>> descriptive 'disable_device'.
>> 
>> Signed-off-by: Johannes Thumshirn 
>> Fixes: e315cd28b9ef ("[SCSI] qla2xxx: Code changes for qla data
>>structure refactoring")
>
>Hello Johannes,
>
>Please consider adding a Cc: stable tag to this patch. Since otherwise
>this
>patch looks fine to me:
>
>Reviewed-by: Bart Van Assche 

Looks good to me. 

Reviewed-by: Giridhar Malavali

Re: [PATCH v3 00/14] qla2xxx: Bug Fixes and updates for target.

2017-03-09 Thread Malavali, Giridhar 


On 3/8/17, 7:42 PM, "Nicholas A. Bellinger" <n...@linux-iscsi.org> wrote:

>Hi Giri,
>
>On Wed, 2017-03-08 at 18:30 +, Malavali, Giridhar wrote:
>> 
>> On 3/8/17, 7:20 AM, "Bart Van Assche" <bart.vanass...@sandisk.com>
>>wrote:
>> 
>> >On Tue, 2017-03-07 at 23:34 -0800, Nicholas A. Bellinger wrote:
>> >> Btw, the regression reported here in v2:
>> >> 
>> >> http://www.spinics.net/lists/target-devel/msg14348.html
>> >> 
>> >> is completely different from what you've reported here.
>> >
>> >The call traces differ but the root cause is probably the same.
>> >
>> >> It would be useful to explain how you reproduced this, instead of
>>just
>> >> posting backtrace with zero context..?
>> >> 
>> >> Can we at least identify which patch in this series is causing
>>this..?
>> >> 
>> >> Also, I assume you are running this on stock v4.11-rc1 with only this
>> >> qla2xxx series applied, and not all of your other stuff, right..?
>> >
>> >The test I ran against v4.11-rc1 + this patch series is to start LIO
>>on a
>> >system equipped with two back-to-back connected QLogic FC HBAs (no
>>switch
>> >inbetween), to load the tcm_qla2xxx driver, to configure LUNs and to
>>wait
>> >until the SCSI stack reports that these LUNs have appeared. What I see
>>in
>> >the lsscsi output with both v2 and v3 of this patch series is that
>>these
>> >LUNs appear briefly and then disappear and that a little bit later the
>> >kernel reports that a hang occurred. Without this patch series the LUNs
>> >are
>> >detected and do not disappear automatically and no hang is reported. I
>> >think the next step is that Cavium verifies whether they can reproduce
>> >this
>> >behavior and if they can reproduce it to run a bisect. BTW, since there
>> >are
>> >login-related patches in this series I wouldn't be surprised if one of
>> >these
>> >patches introduced the regression.
>> 
>> We generally go through switch. We will try to reproduce with back to
>>back
>> and bisect the patches.
>> 
>
>Just a heads up given this series mixes bug-fixes with a few other
>miscellaneous improvements, I'd like to get it pushed to Linus with the
>next round of patches no later than -rc3.
>
>If it's beyond -rc3, then the bug-fixes will need to be split out for
>v4.11-rc separate from the other improvements.

Thanks for the heads-up. We are working on Bart reported issue. We are
able to recreate this internally.
We should be able to wrap it up in next few days and send the final
patches. I don¹t think anything else is outstanding.

‹ Giri

>

Re: [PATCH v3 00/14] qla2xxx: Bug Fixes and updates for target.

2017-03-08 Thread Malavali, Giridhar 


On 3/8/17, 7:20 AM, "Bart Van Assche"  wrote:

>On Tue, 2017-03-07 at 23:34 -0800, Nicholas A. Bellinger wrote:
>> Btw, the regression reported here in v2:
>> 
>> http://www.spinics.net/lists/target-devel/msg14348.html
>> 
>> is completely different from what you've reported here.
>
>The call traces differ but the root cause is probably the same.
>
>> It would be useful to explain how you reproduced this, instead of just
>> posting backtrace with zero context..?
>> 
>> Can we at least identify which patch in this series is causing this..?
>> 
>> Also, I assume you are running this on stock v4.11-rc1 with only this
>> qla2xxx series applied, and not all of your other stuff, right..?
>
>The test I ran against v4.11-rc1 + this patch series is to start LIO on a
>system equipped with two back-to-back connected QLogic FC HBAs (no switch
>inbetween), to load the tcm_qla2xxx driver, to configure LUNs and to wait
>until the SCSI stack reports that these LUNs have appeared. What I see in
>the lsscsi output with both v2 and v3 of this patch series is that these
>LUNs appear briefly and then disappear and that a little bit later the
>kernel reports that a hang occurred. Without this patch series the LUNs
>are
>detected and do not disappear automatically and no hang is reported. I
>think the next step is that Cavium verifies whether they can reproduce
>this
>behavior and if they can reproduce it to run a bisect. BTW, since there
>are
>login-related patches in this series I wouldn't be surprised if one of
>these
>patches introduced the regression.

We generally go through switch. We will try to reproduce with back to back
and bisect the patches.

‹ Giri


>
>Bart.