[PATCH 1/3] scsi: qla2xxx: Fix warning after FC target reset

[PATCH 1/3] scsi: qla2xxx: Fix warning after FC target reset

[Viacheslav Dubeyko]

From: Viacheslav Dubeyko <v.dubeiko@yadro.com>
Date: Fri, 10 Apr 2020 11:07:08 +0300
Subject: [PATCH 1/3] scsi: qla2xxx: Fix warning after FC target reset

Currently, FC target reset finishes with the warning
message.

[84010.596893] ------------[ cut here ]------------
[84010.596917] WARNING: CPU: 238 PID: 279973 at ../drivers/scsi/qla2xxx/qla_target.c:6644 qlt_enable_vha+0x1d0/0x260 [qla2xxx]
[84010.596918] Modules linked in: vrf af_packet 8021q garp mrp stp llc netlink_diag target_tatlin_tblock(OEX) dm_ec(OEX) ttln_rdma(OEX) dm_frontend(OEX) nvme_rdma nvmet tcm_qla2xxx iscsi_target_mod target_core_mod at24 nvmem_core pnv_php ipmi_watchdog ipmi_ssif vmx_crypto gf128mul crct10dif_vpmsum qla2xxx rpcrdma nvme_fc powernv_flash(X) nvme_fabrics uio_pdrv_genirq mtd rtc_opal(X) ibmpowernv(X) opal_prd(X) uio scsi_transport_fc i2c_opal(X) ses enclosure ipmi_poweroff ast i2c_algo_bit ttm bmc_mcu(OEX) drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm drm_panel_orientation_quirks agpgart nfsd auth_rpcgss nfs_acl ipmi_powernv(X) lockd ipmi_devintf ipmi_msghandler grace dummy ext4 crc16 jbd2 mbcache sd_mod rdma_ucm ib_iser rdma_cm ib_umad iw_cm ib_ipoib libiscsi scsi_transport_iscsi ib_cm
[84010.596975]  configfs mlx5_ib ib_uverbs ib_core mlx5_core crc32c_vpmsum xhci_pci xhci_hcd mpt3sas(OEX) tg3 usbcore mlxfw tls raid_class libphy scsi_transport_sas devlink ptp pps_core nvme nvme_core sunrpc dm_mirror dm_region_hash dm_log sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua scsi_mod autofs4
[84010.597001] Supported: Yes, External
[84010.597004] CPU: 238 PID: 279973 Comm: bash Tainted: G           OE      4.12.14-197.29-default #1 SLE15-SP1
[84010.597006] task: c000000a104c0000 task.stack: c000000b52188000
[84010.597007] NIP: d00000001ffd7f78 LR: d00000001ffd7f6c CTR: c0000000001676c0
[84010.597008] REGS: c000000b5218b910 TRAP: 0700   Tainted: G           OE       (4.12.14-197.29-default)
[84010.597008] MSR: 900000010282b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE,TM[E]>
[84010.597015]   CR: 48242424  XER: 00000000
[84010.597016] CFAR: d00000001ff45d08 SOFTE: 1
               GPR00: d00000001ffd7f6c c000000b5218bb90 d00000002001b228 0000000000000102
               GPR04: 0000000000000001 0000000000000001 00013d91ed0a5e2d 0000000000000000
               GPR08: c000000007793300 0000000000000000 0000000000000000 c000000a086e7818
               GPR12: 0000000000002200 c000000007793300 0000000000000000 000000012bc937c0
               GPR16: 000000012bbf7ed0 0000000000000000 000000012bc3dd10 0000000000000000
               GPR20: 000000012bc4db28 0000010036442810 000000012bc97828 000000012bc96c70
               GPR24: 00000100365b1550 0000000000000000 00000100363f3d80 c000000be20d3080
               GPR28: c000000bda7eae00 c000000be20db7e8 c000000be20d3778 c000000be20db7e8
[84010.597042] NIP [d00000001ffd7f78] qlt_enable_vha+0x1d0/0x260 [qla2xxx]
[84010.597051] LR [d00000001ffd7f6c] qlt_enable_vha+0x1c4/0x260 [qla2xxx]
[84010.597051] Call Trace:
[84010.597061] [c000000b5218bb90] [d00000001ffd7f6c] qlt_enable_vha+0x1c4/0x260 [qla2xxx] (unreliable)
[84010.597064] [c000000b5218bc20] [d000000009820b6c] tcm_qla2xxx_tpg_enable_store+0xc4/0x130 [tcm_qla2xxx]
[84010.597067] [c000000b5218bcb0] [d0000000185d0e68] configfs_write_file+0xd0/0x190 [configfs]
[84010.597072] [c000000b5218bd00] [c0000000003d0edc] __vfs_write+0x3c/0x1e0
[84010.597074] [c000000b5218bd90] [c0000000003d2ea8] vfs_write+0xd8/0x220
[84010.597076] [c000000b5218bde0] [c0000000003d4ddc] SyS_write+0x6c/0x110
[84010.597079] [c000000b5218be30] [c00000000000b188] system_call+0x3c/0x130
[84010.597080] Instruction dump:
[84010.597082] 7d0050a8 7d084b78 7d0051ad 40c2fff4 7fa3eb78 4bf73965 60000000 7fa3eb78
[84010.597086] 4bf6dcd9 60000000 2fa30000 419eff40 <0fe00000> 4bffff38 e95f0058 a12a0180
[84010.597090] ---[ end trace e32abaf6e6fee826 ]---

It is possible to use such path with the goal
to reproduce the issue:

echo 0x7fffffff > /sys/module/qla2xxx/parameters/logging
modprobe target_core_mod
modprobe tcm_qla2xxx
mkdir /sys/kernel/config/target/qla2xxx
mkdir /sys/kernel/config/target/qla2xxx/<port-name>
mkdir /sys/kernel/config/target/qla2xxx/<port-name>/tpgt_1
echo 1 > /sys/kernel/config/target/qla2xxx/<port-name>/tpgt_1/enable
echo 0 > /sys/kernel/config/target/qla2xxx/<port-name>/tpgt_1/enable
echo 1 > /sys/kernel/config/target/qla2xxx/<port-name>/tpgt_1/enable

SYSTEM START
kernel: pid 327:drivers/scsi/qla2xxx/qla_init.c:2174 qla2x00_initialize_adapter(): vha->flags.online 0x0
<...>
kernel: pid 327:drivers/scsi/qla2xxx/qla_os.c:3444 qla2x00_probe_one(): vha->flags.online 0x1

echo 1 > /sys/kernel/config/target/qla2xxx/21:00:00:24:ff:86:a6:2a/tpgt_1/enable
kernel: pid 348:drivers/scsi/qla2xxx/qla_init.c:6641 qla2x00_abort_isp_cleanup(): vha->flags.online 0x0, ISP_ABORT_NEEDED 0x0
<...>
kernel: pid 348:drivers/scsi/qla2xxx/qla_init.c:6998 qla2x00_restart_isp(): vha->flags.online 0x0

echo 0 > /sys/kernel/config/target/qla2xxx/21:00:00:24:ff:86:a6:2a/tpgt_1/enable
kernel: pid 348:drivers/scsi/qla2xxx/qla_init.c:6641 qla2x00_abort_isp_cleanup(): vha->flags.online 0x0, ISP_ABORT_NEEDED 0x0
<...>
kernel: pid 1404:drivers/scsi/qla2xxx/qla_os.c:1107 qla2x00_wait_for_hba_online(): base_vha->flags.online 0x0

echo 1 > /sys/kernel/config/target/qla2xxx/21:00:00:24:ff:86:a6:2a/tpgt_1/enable
kernel: pid 1404:drivers/scsi/qla2xxx/qla_os.c:1107 qla2x00_wait_for_hba_online(): base_vha->flags.online 0x0
kernel: -----------[ cut here ]-----------
kernel: WARNING: CPU: 1 PID: 1404 at drivers/scsi/qla2xxx/qla_target.c:6654 qlt_enable_vha+0x1e0/0x280 [qla2xxx]

The issue takes place because no real ISP reset is executed.
The qla2x00_abort_isp(scsi_qla_host_t *vha) method expects that
vha->flags.online will be not zero for ISP reset procedure.
This patch sets vha->flags.online to 1 before calling
->abort_isp() for starting the ISP reset.

Signed-off-by: Viacheslav Dubeyko <v.dubeiko@yadro.com>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index d9072ea7c42b..0a2286e324af 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -6872,6 +6872,7 @@ qla2x00_do_dpc(void *data)
 
 			if (do_reset && !(test_and_set_bit(ABORT_ISP_ACTIVE,
 			    &base_vha->dpc_flags))) {
+				base_vha->flags.online = 1;
 				ql_dbg(ql_dbg_dpc, base_vha, 0x4007,
 				    "ISP abort scheduled.\n");
 				if (ha->isp_ops->abort_isp(base_vha)) {
-- 
2.17.1

Re: [PATCH 1/3] scsi: qla2xxx: Fix warning after FC target reset

[Bart Van Assche]

On 2020-04-24 05:10, Viacheslav Dubeyko wrote:
> From: Viacheslav Dubeyko <v.dubeiko@yadro.com>
> Date: Fri, 10 Apr 2020 11:07:08 +0300
> Subject: [PATCH 1/3] scsi: qla2xxx: Fix warning after FC target reset
> 
> Currently, FC target reset finishes with the warning
> message.

Hi Slava,

For future patch submissions, please include a cover letter with the
patch series and also use threaded mode, e.g. by setting
sendemail.thread = true in ~/.gitconfig.

A summary of what Martin Petersen expects from contributors is available
at https://lore.kernel.org/ksummit-discuss/yq1o8zqeqhb.fsf@oracle.com/.

Thanks,

Bart.

Re: [PATCH 1/3] scsi: qla2xxx: Fix warning after FC target reset

[Viacheslav Dubeyko]

Hi Bart,

On Sun, 2020-04-26 at 18:55 -0700, Bart Van Assche wrote:
> On 2020-04-24 05:10, Viacheslav Dubeyko wrote:
> > From: Viacheslav Dubeyko <v.dubeiko@yadro.com>
> > Date: Fri, 10 Apr 2020 11:07:08 +0300
> > Subject: [PATCH 1/3] scsi: qla2xxx: Fix warning after FC target
> > reset
> > 
> > Currently, FC target reset finishes with the warning
> > message.
> 
> Hi Slava,
> 
> For future patch submissions, please include a cover letter with the
> patch series and also use threaded mode, e.g. by setting
> sendemail.thread = true in ~/.gitconfig.
> 
> A summary of what Martin Petersen expects from contributors is
> available
> at 
> https://lore.kernel.org/ksummit-discuss/yq1o8zqeqhb.fsf@oracle.com/.
> 

Frankly speaking, I don't see the logical relations among these three
fixes. So, I didn't prepare the cover letter because of this. I believe
that three independent patches could be better than patchset. What do
you think?

I am using the Evolution for sending the patches. I am not completely
sure that sendemail.thread = true in ~/.gitconfig can change the
Evolution's behavior. Is it something wrong with the Evolution's
settings?

Thanks,
Slava.

Re: [PATCH 1/3] scsi: qla2xxx: Fix warning after FC target reset

[James Bottomley]

On Mon, 2020-04-27 at 12:18 +0300, Viacheslav Dubeyko wrote:
[...]
> I am using the Evolution for sending the patches. I am not completely
> sure that sendemail.thread = true in ~/.gitconfig can change the
> Evolution's behavior.

It can't.  That setting only works with git-send-email

>  Is it something wrong with the Evolution's settings?

Not the settings, with your sending process: if you send a manual email
you need to send the rest of the threaded patches as a reply to the
first one or the cover letter.

I do this in evolution by replying to the first email from my sent
folder.

James

Re: [PATCH 1/3] scsi: qla2xxx: Fix warning after FC target reset

[Bart Van Assche]

On 2020-04-27 02:18, Viacheslav Dubeyko wrote:
> Hi Bart,
> 
> On Sun, 2020-04-26 at 18:55 -0700, Bart Van Assche wrote:
>> On 2020-04-24 05:10, Viacheslav Dubeyko wrote:
>>> From: Viacheslav Dubeyko <v.dubeiko@yadro.com>
>>> Date: Fri, 10 Apr 2020 11:07:08 +0300
>>> Subject: [PATCH 1/3] scsi: qla2xxx: Fix warning after FC target
>>> reset
>>>
>>> Currently, FC target reset finishes with the warning
>>> message.
>>
>> Hi Slava,
>>
>> For future patch submissions, please include a cover letter with the
>> patch series and also use threaded mode, e.g. by setting
>> sendemail.thread = true in ~/.gitconfig.
>>
>> A summary of what Martin Petersen expects from contributors is
>> available
>> at 
>> https://lore.kernel.org/ksummit-discuss/yq1o8zqeqhb.fsf@oracle.com/.
>>
> 
> Frankly speaking, I don't see the logical relations among these three
> fixes. So, I didn't prepare the cover letter because of this. I believe
> that three independent patches could be better than patchset. What do
> you think?

Most people who are subscribed to kernel mailing lists use an e-mail
client that supports threaded mode. Including a cover letter helps
e-mail clients that support threaded mode a lot.

> I am using the Evolution for sending the patches. I am not completely
> sure that sendemail.thread = true in ~/.gitconfig can change the
> Evolution's behavior. Is it something wrong with the Evolution's
> settings?

Please consider to use git send-email instead of Evolution. Most kernel
developers use git branches to prepare patches. Using git send-email to
send out a patch series from a git branch is much less work than copying
these patches into Evolution and next telling Evolution to send out the
patch series. Additionally, last time I used Evolution myself to send
out patches I encountered a weird character encoding bug. I had to
change character encoding from UTF-8 into UTF-7 to make sure that
Evolution handles diacritics in names of people correctly.

Bart.

Re: [PATCH 1/3] scsi: qla2xxx: Fix warning after FC target reset

[Martin K. Petersen]

Hi Viacheslav!

I applied your three patches to 5.8/scsi-queue. Please use git
send-email in the future and please include a cover letter. Both b4 and
patchwork failed to parse your mails. I had to reconstitute each of your
patches by hand and manually add links and review tags.

Thank you!

-- 
Martin K. Petersen	Oracle Linux Engineering