From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932464AbcJMPQU convert rfc822-to-8bit (ORCPT ); Thu, 13 Oct 2016 11:16:20 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:45486 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753897AbcJMPQE (ORCPT ); Thu, 13 Oct 2016 11:16:04 -0400 Subject: Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly To: Johannes Thumshirn , "Martin K . Petersen" References: Cc: Christoph Hellwig , Hannes Reinecke , Linux Kernel Mailinglist , Linux SCSI Mailinglist , Martin Schwidefsky , Heiko Carstens , Anil Gurumurthy , Sudarsana Kalluru , "James E.J. Bottomley" , Tyrel Datwyler , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Johannes Thumshirn , James Smart , Dick Kennedy , "supporter:QLOGIC QLA2XXX FC-SCSI DRIVER" , "open list:S390 ZFCP DRIVER" , "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" , "open list:FCOE SUBSYSTEM (libfc, libfcoe, fcoe)" From: Steffen Maier Date: Thu, 13 Oct 2016 17:15:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8BIT X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16101315-0008-0000-0000-000002DEDD2C X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16101315-0009-0000-0000-00001A539E5C Message-Id: <2ea07f3f-88eb-b795-fa37-a223bf80e581@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-10-13_08:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1610130262 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org I'm puzzled. $ git bisect start fc_bsg master Bisecting: 8 revisions left to test after this (roughly 3 steps) [005d51510eee6102636d5dbb06310531c5d46151] scsi: fc: implement kref backed reference counting $ git bisect bad Bisecting: 3 revisions left to test after this (roughly 2 steps) [bef6da201de1bb81bb4d9511f9a155862efc251f] scsi: Unify interfaces of fc_bsg_jobdone and bsg_job_done $ git bisect bad Bisecting: 1 revision left to test after this (roughly 1 step) [3087864ce3d7282f59021245d8a5f83ef1caef18] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly $ git bisect bad Bisecting: 0 revisions left to test after this (roughly 0 steps) [81aea44720d22d2e0c4a2613ae8b1c256ef6b0cb] scsi: Get rid of struct fc_bsg_buffer > $ git bisect good > 3087864ce3d7282f59021245d8a5f83ef1caef18 is the first bad commit > commit 3087864ce3d7282f59021245d8a5f83ef1caef18 > Author: Johannes Thumshirn > Date: Wed Oct 12 15:06:28 2016 +0200 > > scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly > > Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but use > helper variables bsg_request and bsg_reply. This will be helpfull when > transitioning to bsg-lib. > > Signed-off-by: Johannes Thumshirn > > :040000 040000 140c4b6829d5cfaec4079716e0795f63f8bc3bd2 0d9fe225615679550be91fbd9f84c09ab1e280fc M drivers From there (on the reverse bisect path) I get the following Oops, except for the full patch set having another stack trace as in my previous mail (dying in zfcp code). > [ 46.942452] Oops: 0004 ilc:2 [#1] [ 46.942460] PREEMPT SMP [ 46.942465] > [ 46.942470] Modules linked in: nf_log_ipv6 xt_pkttype nf_log_ipv4 nf_log_common xt_LOG xt_limit ip6t_REJECT nf_reject_ipv6 xt_tcpudp nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_raw ipt_REJECT nf_reject_ipv4 iptable_raw xt_CT iptable_filter ip6table_mangle nf_conntrack_netbios_ns nf_conntrack_broadcast nf_conntrack_ipv4 nf_defrag_ipv4 ip_tables xt_conntrack nf_conntrack ip6table_filter ip6_tables x_tables qeth_l2 ghash_s390 prng ecb aes_s390 des_s390 des_generic sha512_s390 sha256_s390 sha1_s390 sha_common dm_mod qeth ccwgroup zfcp qdio autofs4 > [ 46.942547] CPU: 1 PID: 1714 Comm: zfcp_ping Not tainted 4.8.0fcbsg+ #9 > [ 46.942550] Hardware name: IBM 2964 N96 702 (z/VM) > [ 46.942556] task: 000000005c988008 task.stack: 000000005d2ec000 > [ 46.942560] Krnl PSW : 0704e00180000000 00000000007c91ec[ 46.942574] (fc_bsg_request_handler+0x404/0x4b0) > [ 46.942579] > [ 46.942583] R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:000: > [ 46.942598] RI:0 EA:3 > [ 46.942601] > [ 46.942601] Krnl GPRS: 0000000000000000 00000000ffffffcb 0000000000000000 0000000080000001 > [ 46.942603] 00000000007c8fe8 0000000064398c68 0000000069f967e8 000000006a3d8008 > [ 46.942605] 000000006a5e02c8 00000000698b5490 0000000000000000 0000000000000000 > [ 46.942607] 000000006a9ef5f8 0000000000a36840 00000000007c8fe8 000000005d2efa00 > [ 46.942619] Krnl Code: 00000000007c91de: e55dc08c0003 clfhsi 140(%r12),3[ 46.942622] > [ 46.942622] 00000000007c91e4: a7240004 brc 2,7c91ec > #00000000007c91e8: a7f40001 brc 15,7c91ea[ 46.942629] > [ 46.942629] >00000000007c91ec: 5010b000 st %r1,0(%r11) > 00000000007c91f0: e54cb0040000 mvhi 4(%r11),0[ 46.942635] > [ 46.942635] 00000000007c91f6: e54cc08c0004 mvhi 140(%r12),4 > 00000000007c91fc: b904002c lgr %r2,%r12[ 46.942643] > [ 46.942643] 00000000007c9200: c0e5ffffe2c0 brasl %r14,7c5780 > [ 46.942646] > [ 46.942647] Call Trace: > [ 46.942650] ([<00000000007c8fe8>] fc_bsg_request_handler+0x200/0x4b0) > [ 46.942656] ([<00000000006b8e0a>] __blk_run_queue+0x52/0x68) > [ 46.942661] ([<00000000006c549a>] blk_execute_rq_nowait+0xf2/0x110) > [ 46.942664] ([<00000000006c557a>] blk_execute_rq+0xa2/0x110) > [ 46.942668] ([<00000000006de0ee>] bsg_ioctl+0x1f6/0x268) > [ 46.942675] ([<000000000036ca20>] do_vfs_ioctl+0x680/0x6d8) > [ 46.942677] ([<000000000036caf4>] SyS_ioctl+0x7c/0xb0) > [ 46.942685] ([<00000000009a541e>] system_call+0xd6/0x270) > [ 46.942687] INFO: lockdep is turned off. > [ 46.942688] Last Breaking-Event-Address: > [ 46.942692] [<00000000007c91e4>] fc_bsg_request_handler+0x3fc/0x4b0 > [ 46.942696] [ 46.942698] Kernel panic - not syncing: Fatal exception: panic_on_oops On 10/12/2016 03:06 PM, Johannes Thumshirn wrote: > Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but use > helper variables bsg_request and bsg_reply. This will be helpfull when > transitioning to bsg-lib. > > Signed-off-by: Johannes Thumshirn > --- > drivers/s390/scsi/zfcp_fc.c | 9 +- > drivers/scsi/bfa/bfad_bsg.c | 40 +++--- > drivers/scsi/ibmvscsi/ibmvfc.c | 22 ++-- > drivers/scsi/libfc/fc_lport.c | 23 ++-- > drivers/scsi/lpfc/lpfc_bsg.c | 194 +++++++++++++++++----------- > drivers/scsi/qla2xxx/qla_bsg.c | 264 ++++++++++++++++++++++----------------- > drivers/scsi/qla2xxx/qla_iocb.c | 5 +- > drivers/scsi/qla2xxx/qla_isr.c | 46 ++++--- > drivers/scsi/qla2xxx/qla_mr.c | 10 +- > drivers/scsi/scsi_transport_fc.c | 37 +++--- > 10 files changed, 387 insertions(+), 263 deletions(-) > diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c > index 8ff2067..eafc7555 100644 > --- a/drivers/scsi/scsi_transport_fc.c > +++ b/drivers/scsi/scsi_transport_fc.c > @@ -3588,9 +3588,10 @@ fc_bsg_jobdone(struct fc_bsg_job *job) > { > struct request *req = job->req; > struct request *rsp = req->next_rq; > + struct fc_bsg_reply *bsg_reply = job->reply; > int err; > > - err = job->req->errors = job->reply->result; > + err = job->req->errors = bsg_reply->result; > > if (err < 0) > /* we're only returning the result field in the reply */ > @@ -3602,10 +3603,10 @@ fc_bsg_jobdone(struct fc_bsg_job *job) > req->resid_len = 0; > > if (rsp) { > - WARN_ON(job->reply->reply_payload_rcv_len > rsp->resid_len); > + WARN_ON(bsg_reply->reply_payload_rcv_len > rsp->resid_len); > > /* set reply (bidi) residual */ > - rsp->resid_len -= min(job->reply->reply_payload_rcv_len, > + rsp->resid_len -= min(bsg_reply->reply_payload_rcv_len, > rsp->resid_len); > } > blk_complete_request(req); > @@ -3701,6 +3702,8 @@ fc_req_to_bsgjob(struct Scsi_Host *shost, struct fc_rport *rport, > struct fc_internal *i = to_fc_internal(shost->transportt); > struct request *rsp = req->next_rq; > struct fc_bsg_job *job; > + struct fc_bsg_request *bsg_request; > + struct fc_bsg_reply *bsg_reply; > int ret; > > BUG_ON(req->special); > @@ -3726,9 +3729,9 @@ fc_req_to_bsgjob(struct Scsi_Host *shost, struct fc_rport *rport, > if (i->f->dd_bsg_size) > job->dd_data = (void *)&job[1]; > spin_lock_init(&job->job_lock); > - job->request = (struct fc_bsg_request *)req->cmd; > + bsg_request = (struct fc_bsg_request *)req->cmd; > job->request_len = req->cmd_len; > - job->reply = req->sense; > + bsg_reply = req->sense; > job->reply_len = SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer > * allocated */ > if (req->bio) { > @@ -3779,11 +3782,13 @@ fc_bsg_host_dispatch(struct request_queue *q, struct Scsi_Host *shost, > struct fc_bsg_job *job) > { > struct fc_internal *i = to_fc_internal(shost->transportt); > + struct fc_bsg_request *bsg_request = job->request; > + struct fc_bsg_reply *bsg_reply = job->reply; > int cmdlen = sizeof(uint32_t); /* start with length of msgcode */ > int ret; > > /* Validate the host command */ > - switch (job->request->msgcode) { > + switch (bsg_request->msgcode) { > case FC_BSG_HST_ADD_RPORT: > cmdlen += sizeof(struct fc_bsg_host_add_rport); > break; > @@ -3815,7 +3820,7 @@ fc_bsg_host_dispatch(struct request_queue *q, struct Scsi_Host *shost, > case FC_BSG_HST_VENDOR: > cmdlen += sizeof(struct fc_bsg_host_vendor); > if ((shost->hostt->vendor_id == 0L) || > - (job->request->rqst_data.h_vendor.vendor_id != > + (bsg_request->rqst_data.h_vendor.vendor_id != > shost->hostt->vendor_id)) { > ret = -ESRCH; > goto fail_host_msg; > @@ -3840,8 +3845,8 @@ fc_bsg_host_dispatch(struct request_queue *q, struct Scsi_Host *shost, > fail_host_msg: > /* return the errno failure code as the only status */ > BUG_ON(job->reply_len < sizeof(uint32_t)); > - job->reply->reply_payload_rcv_len = 0; > - job->reply->result = ret; > + bsg_reply->reply_payload_rcv_len = 0; > + bsg_reply->result = ret; > job->reply_len = sizeof(uint32_t); > fc_bsg_jobdone(job); > return FC_DISPATCH_UNLOCKED; > @@ -3878,11 +3883,13 @@ fc_bsg_rport_dispatch(struct request_queue *q, struct Scsi_Host *shost, > struct fc_rport *rport, struct fc_bsg_job *job) > { > struct fc_internal *i = to_fc_internal(shost->transportt); > + struct fc_bsg_request *bsg_request = job->request; > + struct fc_bsg_reply *bsg_reply = job->reply; > int cmdlen = sizeof(uint32_t); /* start with length of msgcode */ > int ret; > > /* Validate the rport command */ > - switch (job->request->msgcode) { > + switch (bsg_request->msgcode) { > case FC_BSG_RPT_ELS: > cmdlen += sizeof(struct fc_bsg_rport_els); > goto check_bidi; > @@ -3915,8 +3922,8 @@ check_bidi: > fail_rport_msg: > /* return the errno failure code as the only status */ > BUG_ON(job->reply_len < sizeof(uint32_t)); > - job->reply->reply_payload_rcv_len = 0; > - job->reply->result = ret; > + bsg_reply->reply_payload_rcv_len = 0; > + bsg_reply->result = ret; > job->reply_len = sizeof(uint32_t); > fc_bsg_jobdone(job); > return FC_DISPATCH_UNLOCKED; > @@ -3937,6 +3944,7 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost, > struct request *req; > struct fc_bsg_job *job; > enum fc_dispatch_result ret; > + struct fc_bsg_reply *bsg_reply; > > if (!get_device(dev)) > return; > @@ -3973,8 +3981,9 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost, > /* check if we have the msgcode value at least */ > if (job->request_len < sizeof(uint32_t)) { > BUG_ON(job->reply_len < sizeof(uint32_t)); > - job->reply->reply_payload_rcv_len = 0; > - job->reply->result = -ENOMSG; > + bsg_reply = job->reply; > + bsg_reply->reply_payload_rcv_len = 0; > + bsg_reply->result = -ENOMSG; > job->reply_len = sizeof(uint32_t); > fc_bsg_jobdone(job); > spin_lock_irq(q->queue_lock); > -- Mit freundlichen Grüßen / Kind regards Steffen Maier Linux on z Systems Development IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschaeftsfuehrung: Dirk Wittkopp Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steffen Maier Subject: Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly Date: Thu, 13 Oct 2016 17:15:25 +0200 Message-ID: <2ea07f3f-88eb-b795-fa37-a223bf80e581@linux.vnet.ibm.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Johannes Thumshirn , "Martin K . Petersen" Cc: Christoph Hellwig , Hannes Reinecke , Linux Kernel Mailinglist , Linux SCSI Mailinglist , Martin Schwidefsky , Heiko Carstens , Anil Gurumurthy , Sudarsana Kalluru , "James E.J. Bottomley" , Tyrel Datwyler , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Johannes Thumshirn , James Smart , Dick Kennedy , "supporter:QLOGIC QLA2XXX FC-SCSI DRIVER" List-Id: linux-scsi@vger.kernel.org I'm puzzled. $ git bisect start fc_bsg master Bisecting: 8 revisions left to test after this (roughly 3 steps) [005d51510eee6102636d5dbb06310531c5d46151] scsi: fc: implement kref backed reference counting $ git bisect bad Bisecting: 3 revisions left to test after this (roughly 2 steps) [bef6da201de1bb81bb4d9511f9a155862efc251f] scsi: Unify interfaces of fc_bsg_jobdone and bsg_job_done $ git bisect bad Bisecting: 1 revision left to test after this (roughly 1 step) [3087864ce3d7282f59021245d8a5f83ef1caef18] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly $ git bisect bad Bisecting: 0 revisions left to test after this (roughly 0 steps) [81aea44720d22d2e0c4a2613ae8b1c256ef6b0cb] scsi: Get rid of struct fc_bsg_buffer > $ git bisect good > 3087864ce3d7282f59021245d8a5f83ef1caef18 is the first bad commit > commit 3087864ce3d7282f59021245d8a5f83ef1caef18 > Author: Johannes Thumshirn > Date: Wed Oct 12 15:06:28 2016 +0200 > > scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly > > Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but use > helper variables bsg_request and bsg_reply. This will be helpfull when > transitioning to bsg-lib. > > Signed-off-by: Johannes Thumshirn > > :040000 040000 140c4b6829d5cfaec4079716e0795f63f8bc3bd2 0d9fe225615679550be91fbd9f84c09ab1e280fc M drivers From there (on the reverse bisect path) I get the following Oops, except for the full patch set having another stack trace as in my previous mail (dying in zfcp code). > [ 46.942452] Oops: 0004 ilc:2 [#1] [ 46.942460] PREEMPT SMP [ 46.942465] > [ 46.942470] Modules linked in: nf_log_ipv6 xt_pkttype nf_log_ipv4 nf_log_common xt_LOG xt_limit ip6t_REJECT nf_reject_ipv6 xt_tcpudp nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_raw ipt_REJECT nf_reject_ipv4 iptable_raw xt_CT iptable_filter ip6table_mangle nf_conntrack_netbios_ns nf_conntrack_broadcast nf_conntrack_ipv4 nf_defrag_ipv4 ip_tables xt_conntrack nf_conntrack ip6table_filter ip6_tables x_tables qeth_l2 ghash_s390 prng ecb aes_s390 des_s390 des_generic sha512_s390 sha256_s390 sha1_s390 sha_common dm_mod qeth ccwgroup zfcp qdio autofs4 > [ 46.942547] CPU: 1 PID: 1714 Comm: zfcp_ping Not tainted 4.8.0fcbsg+ #9 > [ 46.942550] Hardware name: IBM 2964 N96 702 (z/VM) > [ 46.942556] task: 000000005c988008 task.stack: 000000005d2ec000 > [ 46.942560] Krnl PSW : 0704e00180000000 00000000007c91ec[ 46.942574] (fc_bsg_request_handler+0x404/0x4b0) > [ 46.942579] > [ 46.942583] R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:000: > [ 46.942598] RI:0 EA:3 > [ 46.942601] > [ 46.942601] Krnl GPRS: 0000000000000000 00000000ffffffcb 0000000000000000 0000000080000001 > [ 46.942603] 00000000007c8fe8 0000000064398c68 0000000069f967e8 000000006a3d8008 > [ 46.942605] 000000006a5e02c8 00000000698b5490 0000000000000000 0000000000000000 > [ 46.942607] 000000006a9ef5f8 0000000000a36840 00000000007c8fe8 000000005d2efa00 > [ 46.942619] Krnl Code: 00000000007c91de: e55dc08c0003 clfhsi 140(%r12),3[ 46.942622] > [ 46.942622] 00000000007c91e4: a7240004 brc 2,7c91ec > #00000000007c91e8: a7f40001 brc 15,7c91ea[ 46.942629] > [ 46.942629] >00000000007c91ec: 5010b000 st %r1,0(%r11) > 00000000007c91f0: e54cb0040000 mvhi 4(%r11),0[ 46.942635] > [ 46.942635] 00000000007c91f6: e54cc08c0004 mvhi 140(%r12),4 > 00000000007c91fc: b904002c lgr %r2,%r12[ 46.942643] > [ 46.942643] 00000000007c9200: c0e5ffffe2c0 brasl %r14,7c5780 > [ 46.942646] > [ 46.942647] Call Trace: > [ 46.942650] ([<00000000007c8fe8>] fc_bsg_request_handler+0x200/0x4b0) > [ 46.942656] ([<00000000006b8e0a>] __blk_run_queue+0x52/0x68) > [ 46.942661] ([<00000000006c549a>] blk_execute_rq_nowait+0xf2/0x110) > [ 46.942664] ([<00000000006c557a>] blk_execute_rq+0xa2/0x110) > [ 46.942668] ([<00000000006de0ee>] bsg_ioctl+0x1f6/0x268) > [ 46.942675] ([<000000000036ca20>] do_vfs_ioctl+0x680/0x6d8) > [ 46.942677] ([<000000000036caf4>] SyS_ioctl+0x7c/0xb0) > [ 46.942685] ([<00000000009a541e>] system_call+0xd6/0x270) > [ 46.942687] INFO: lockdep is turned off. > [ 46.942688] Last Breaking-Event-Address: > [ 46.942692] [<00000000007c91e4>] fc_bsg_request_handler+0x3fc/0x4b0 > [ 46.942696] [ 46.942698] Kernel panic - not syncing: Fatal exception: panic_on_oops On 10/12/2016 03:06 PM, Johannes Thumshirn wrote: > Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but use > helper variables bsg_request and bsg_reply. This will be helpfull when > transitioning to bsg-lib. > > Signed-off-by: Johannes Thumshirn > --- > drivers/s390/scsi/zfcp_fc.c | 9 +- > drivers/scsi/bfa/bfad_bsg.c | 40 +++--- > drivers/scsi/ibmvscsi/ibmvfc.c | 22 ++-- > drivers/scsi/libfc/fc_lport.c | 23 ++-- > drivers/scsi/lpfc/lpfc_bsg.c | 194 +++++++++++++++++----------- > drivers/scsi/qla2xxx/qla_bsg.c | 264 ++++++++++++++++++++++----------------- > drivers/scsi/qla2xxx/qla_iocb.c | 5 +- > drivers/scsi/qla2xxx/qla_isr.c | 46 ++++--- > drivers/scsi/qla2xxx/qla_mr.c | 10 +- > drivers/scsi/scsi_transport_fc.c | 37 +++--- > 10 files changed, 387 insertions(+), 263 deletions(-) > diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c > index 8ff2067..eafc7555 100644 > --- a/drivers/scsi/scsi_transport_fc.c > +++ b/drivers/scsi/scsi_transport_fc.c > @@ -3588,9 +3588,10 @@ fc_bsg_jobdone(struct fc_bsg_job *job) > { > struct request *req = job->req; > struct request *rsp = req->next_rq; > + struct fc_bsg_reply *bsg_reply = job->reply; > int err; > > - err = job->req->errors = job->reply->result; > + err = job->req->errors = bsg_reply->result; > > if (err < 0) > /* we're only returning the result field in the reply */ > @@ -3602,10 +3603,10 @@ fc_bsg_jobdone(struct fc_bsg_job *job) > req->resid_len = 0; > > if (rsp) { > - WARN_ON(job->reply->reply_payload_rcv_len > rsp->resid_len); > + WARN_ON(bsg_reply->reply_payload_rcv_len > rsp->resid_len); > > /* set reply (bidi) residual */ > - rsp->resid_len -= min(job->reply->reply_payload_rcv_len, > + rsp->resid_len -= min(bsg_reply->reply_payload_rcv_len, > rsp->resid_len); > } > blk_complete_request(req); > @@ -3701,6 +3702,8 @@ fc_req_to_bsgjob(struct Scsi_Host *shost, struct fc_rport *rport, > struct fc_internal *i = to_fc_internal(shost->transportt); > struct request *rsp = req->next_rq; > struct fc_bsg_job *job; > + struct fc_bsg_request *bsg_request; > + struct fc_bsg_reply *bsg_reply; > int ret; > > BUG_ON(req->special); > @@ -3726,9 +3729,9 @@ fc_req_to_bsgjob(struct Scsi_Host *shost, struct fc_rport *rport, > if (i->f->dd_bsg_size) > job->dd_data = (void *)&job[1]; > spin_lock_init(&job->job_lock); > - job->request = (struct fc_bsg_request *)req->cmd; > + bsg_request = (struct fc_bsg_request *)req->cmd; > job->request_len = req->cmd_len; > - job->reply = req->sense; > + bsg_reply = req->sense; > job->reply_len = SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer > * allocated */ > if (req->bio) { > @@ -3779,11 +3782,13 @@ fc_bsg_host_dispatch(struct request_queue *q, struct Scsi_Host *shost, > struct fc_bsg_job *job) > { > struct fc_internal *i = to_fc_internal(shost->transportt); > + struct fc_bsg_request *bsg_request = job->request; > + struct fc_bsg_reply *bsg_reply = job->reply; > int cmdlen = sizeof(uint32_t); /* start with length of msgcode */ > int ret; > > /* Validate the host command */ > - switch (job->request->msgcode) { > + switch (bsg_request->msgcode) { > case FC_BSG_HST_ADD_RPORT: > cmdlen += sizeof(struct fc_bsg_host_add_rport); > break; > @@ -3815,7 +3820,7 @@ fc_bsg_host_dispatch(struct request_queue *q, struct Scsi_Host *shost, > case FC_BSG_HST_VENDOR: > cmdlen += sizeof(struct fc_bsg_host_vendor); > if ((shost->hostt->vendor_id == 0L) || > - (job->request->rqst_data.h_vendor.vendor_id != > + (bsg_request->rqst_data.h_vendor.vendor_id != > shost->hostt->vendor_id)) { > ret = -ESRCH; > goto fail_host_msg; > @@ -3840,8 +3845,8 @@ fc_bsg_host_dispatch(struct request_queue *q, struct Scsi_Host *shost, > fail_host_msg: > /* return the errno failure code as the only status */ > BUG_ON(job->reply_len < sizeof(uint32_t)); > - job->reply->reply_payload_rcv_len = 0; > - job->reply->result = ret; > + bsg_reply->reply_payload_rcv_len = 0; > + bsg_reply->result = ret; > job->reply_len = sizeof(uint32_t); > fc_bsg_jobdone(job); > return FC_DISPATCH_UNLOCKED; > @@ -3878,11 +3883,13 @@ fc_bsg_rport_dispatch(struct request_queue *q, struct Scsi_Host *shost, > struct fc_rport *rport, struct fc_bsg_job *job) > { > struct fc_internal *i = to_fc_internal(shost->transportt); > + struct fc_bsg_request *bsg_request = job->request; > + struct fc_bsg_reply *bsg_reply = job->reply; > int cmdlen = sizeof(uint32_t); /* start with length of msgcode */ > int ret; > > /* Validate the rport command */ > - switch (job->request->msgcode) { > + switch (bsg_request->msgcode) { > case FC_BSG_RPT_ELS: > cmdlen += sizeof(struct fc_bsg_rport_els); > goto check_bidi; > @@ -3915,8 +3922,8 @@ check_bidi: > fail_rport_msg: > /* return the errno failure code as the only status */ > BUG_ON(job->reply_len < sizeof(uint32_t)); > - job->reply->reply_payload_rcv_len = 0; > - job->reply->result = ret; > + bsg_reply->reply_payload_rcv_len = 0; > + bsg_reply->result = ret; > job->reply_len = sizeof(uint32_t); > fc_bsg_jobdone(job); > return FC_DISPATCH_UNLOCKED; > @@ -3937,6 +3944,7 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost, > struct request *req; > struct fc_bsg_job *job; > enum fc_dispatch_result ret; > + struct fc_bsg_reply *bsg_reply; > > if (!get_device(dev)) > return; > @@ -3973,8 +3981,9 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost, > /* check if we have the msgcode value at least */ > if (job->request_len < sizeof(uint32_t)) { > BUG_ON(job->reply_len < sizeof(uint32_t)); > - job->reply->reply_payload_rcv_len = 0; > - job->reply->result = -ENOMSG; > + bsg_reply = job->reply; > + bsg_reply->reply_payload_rcv_len = 0; > + bsg_reply->result = -ENOMSG; > job->reply_len = sizeof(uint32_t); > fc_bsg_jobdone(job); > spin_lock_irq(q->queue_lock); > -- Mit freundlichen Grüßen / Kind regards Steffen Maier Linux on z Systems Development IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschaeftsfuehrung: Dirk Wittkopp Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3svvS96vqFzDsxh for ; Fri, 14 Oct 2016 02:15:37 +1100 (AEDT) Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u9DFDhLg144854 for ; Thu, 13 Oct 2016 11:15:35 -0400 Received: from e06smtp12.uk.ibm.com (e06smtp12.uk.ibm.com [195.75.94.108]) by mx0a-001b2d01.pphosted.com with ESMTP id 262b6g6ypm-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 13 Oct 2016 11:15:35 -0400 Received: from localhost by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 13 Oct 2016 16:15:32 +0100 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id DF9471B08069 for ; Thu, 13 Oct 2016 16:17:29 +0100 (BST) Received: from d06av11.portsmouth.uk.ibm.com (d06av11.portsmouth.uk.ibm.com [9.149.37.252]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u9DFFSwV29229122 for ; Thu, 13 Oct 2016 15:15:28 GMT Received: from d06av11.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av11.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u9DFFQQ7010956 for ; Thu, 13 Oct 2016 09:15:28 -0600 Subject: Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly To: Johannes Thumshirn , "Martin K . Petersen" References: Cc: Christoph Hellwig , Hannes Reinecke , Linux Kernel Mailinglist , Linux SCSI Mailinglist , Martin Schwidefsky , Heiko Carstens , Anil Gurumurthy , Sudarsana Kalluru , "James E.J. Bottomley" , Tyrel Datwyler , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Johannes Thumshirn , James Smart , Dick Kennedy , "supporter:QLOGIC QLA2XXX FC-SCSI DRIVER" , "open list:S390 ZFCP DRIVER" , "open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)" , "open list:FCOE SUBSYSTEM (libfc, libfcoe, fcoe)" From: Steffen Maier Date: Thu, 13 Oct 2016 17:15:25 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Message-Id: <2ea07f3f-88eb-b795-fa37-a223bf80e581@linux.vnet.ibm.com> List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , I'm puzzled. $ git bisect start fc_bsg master Bisecting: 8 revisions left to test after this (roughly 3 steps) [005d51510eee6102636d5dbb06310531c5d46151] scsi: fc: implement kref=20 backed reference counting $ git bisect bad Bisecting: 3 revisions left to test after this (roughly 2 steps) [bef6da201de1bb81bb4d9511f9a155862efc251f] scsi: Unify interfaces of=20 fc_bsg_jobdone and bsg_job_done $ git bisect bad Bisecting: 1 revision left to test after this (roughly 1 step) [3087864ce3d7282f59021245d8a5f83ef1caef18] scsi: don't use=20 fc_bsg_job::request and fc_bsg_job::reply directly $ git bisect bad Bisecting: 0 revisions left to test after this (roughly 0 steps) [81aea44720d22d2e0c4a2613ae8b1c256ef6b0cb] scsi: Get rid of struct=20 fc_bsg_buffer > $ git bisect good > 3087864ce3d7282f59021245d8a5f83ef1caef18 is the first bad commit > commit 3087864ce3d7282f59021245d8a5f83ef1caef18 > Author: Johannes Thumshirn > Date: Wed Oct 12 15:06:28 2016 +0200 > > scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly > > Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but u= se > helper variables bsg_request and bsg_reply. This will be helpfull = when > transitioning to bsg-lib. > > Signed-off-by: Johannes Thumshirn > > :040000 040000 140c4b6829d5cfaec4079716e0795f63f8bc3bd2 0d9fe2256156795= 50be91fbd9f84c09ab1e280fc M drivers From there (on the reverse bisect path) I get the following Oops, except for the full patch set having another stack trace as in my=20 previous mail (dying in zfcp code). > [ 46.942452] Oops: 0004 ilc:2 [#1] [ 46.942460] PREEMPT SMP [ 46= =2E942465] > [ 46.942470] Modules linked in: nf_log_ipv6 xt_pkttype nf_log_ipv4 nf= _log_common xt_LOG xt_limit ip6t_REJECT nf_reject_ipv6 xt_tcpudp nf_connt= rack_ipv6 nf_defrag_ipv6 ip6table_raw ipt_REJECT nf_reject_ipv4 iptable_r= aw xt_CT iptable_filter ip6table_mangle nf_conntrack_netbios_ns nf_conntr= ack_broadcast nf_conntrack_ipv4 nf_defrag_ipv4 ip_tables xt_conntrack nf_= conntrack ip6table_filter ip6_tables x_tables qeth_l2 ghash_s390 prng ecb= aes_s390 des_s390 des_generic sha512_s390 sha256_s390 sha1_s390 sha_comm= on dm_mod qeth ccwgroup zfcp qdio autofs4 > [ 46.942547] CPU: 1 PID: 1714 Comm: zfcp_ping Not tainted 4.8.0fcbsg+= #9 > [ 46.942550] Hardware name: IBM 2964 N96 70= 2 (z/VM) > [ 46.942556] task: 000000005c988008 task.stack: 000000005d2ec000 > [ 46.942560] Krnl PSW : 0704e00180000000 00000000007c91ec[ 46.94257= 4] (fc_bsg_request_handler+0x404/0x4b0) > [ 46.942579] > [ 46.942583] R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2= PM:000: > [ 46.942598] RI:0 EA:3 > [ 46.942601] > [ 46.942601] Krnl GPRS: 0000000000000000 00000000ffffffcb 00000000000= 00000 0000000080000001 > [ 46.942603] 00000000007c8fe8 0000000064398c68 0000000069f= 967e8 000000006a3d8008 > [ 46.942605] 000000006a5e02c8 00000000698b5490 00000000000= 00000 0000000000000000 > [ 46.942607] 000000006a9ef5f8 0000000000a36840 00000000007= c8fe8 000000005d2efa00 > [ 46.942619] Krnl Code: 00000000007c91de: e55dc08c0003 clfhsi = 140(%r12),3[ 46.942622] > [ 46.942622] 00000000007c91e4: a7240004 brc = 2,7c91ec > #00000000007c91e8: a7f40001 brc = 15,7c91ea[ 46.942629] > [ 46.942629] >00000000007c91ec: 5010b000 st = %r1,0(%r11) > 00000000007c91f0: e54cb0040000 mvhi = 4(%r11),0[ 46.942635] > [ 46.942635] 00000000007c91f6: e54cc08c0004 mvhi = 140(%r12),4 > 00000000007c91fc: b904002c lgr = %r2,%r12[ 46.942643] > [ 46.942643] 00000000007c9200: c0e5ffffe2c0 brasl = %r14,7c5780 > [ 46.942646] > [ 46.942647] Call Trace: > [ 46.942650] ([<00000000007c8fe8>] fc_bsg_request_handler+0x200/0x4b0= ) > [ 46.942656] ([<00000000006b8e0a>] __blk_run_queue+0x52/0x68) > [ 46.942661] ([<00000000006c549a>] blk_execute_rq_nowait+0xf2/0x110) > [ 46.942664] ([<00000000006c557a>] blk_execute_rq+0xa2/0x110) > [ 46.942668] ([<00000000006de0ee>] bsg_ioctl+0x1f6/0x268) > [ 46.942675] ([<000000000036ca20>] do_vfs_ioctl+0x680/0x6d8) > [ 46.942677] ([<000000000036caf4>] SyS_ioctl+0x7c/0xb0) > [ 46.942685] ([<00000000009a541e>] system_call+0xd6/0x270) > [ 46.942687] INFO: lockdep is turned off. > [ 46.942688] Last Breaking-Event-Address: > [ 46.942692] [<00000000007c91e4>] fc_bsg_request_handler+0x3fc/0x4b0= > [ 46.942696] [ 46.942698] Kernel panic - not syncing: Fatal except= ion: panic_on_oops On 10/12/2016 03:06 PM, Johannes Thumshirn wrote: > Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but use > helper variables bsg_request and bsg_reply. This will be helpfull when= > transitioning to bsg-lib. > > Signed-off-by: Johannes Thumshirn > --- > drivers/s390/scsi/zfcp_fc.c | 9 +- > drivers/scsi/bfa/bfad_bsg.c | 40 +++--- > drivers/scsi/ibmvscsi/ibmvfc.c | 22 ++-- > drivers/scsi/libfc/fc_lport.c | 23 ++-- > drivers/scsi/lpfc/lpfc_bsg.c | 194 +++++++++++++++++----------- > drivers/scsi/qla2xxx/qla_bsg.c | 264 ++++++++++++++++++++++---------= -------- > drivers/scsi/qla2xxx/qla_iocb.c | 5 +- > drivers/scsi/qla2xxx/qla_isr.c | 46 ++++--- > drivers/scsi/qla2xxx/qla_mr.c | 10 +- > drivers/scsi/scsi_transport_fc.c | 37 +++--- > 10 files changed, 387 insertions(+), 263 deletions(-) > diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_trans= port_fc.c > index 8ff2067..eafc7555 100644 > --- a/drivers/scsi/scsi_transport_fc.c > +++ b/drivers/scsi/scsi_transport_fc.c > @@ -3588,9 +3588,10 @@ fc_bsg_jobdone(struct fc_bsg_job *job) > { > struct request *req =3D job->req; > struct request *rsp =3D req->next_rq; > + struct fc_bsg_reply *bsg_reply =3D job->reply; > int err; > > - err =3D job->req->errors =3D job->reply->result; > + err =3D job->req->errors =3D bsg_reply->result; > > if (err < 0) > /* we're only returning the result field in the reply */ > @@ -3602,10 +3603,10 @@ fc_bsg_jobdone(struct fc_bsg_job *job) > req->resid_len =3D 0; > > if (rsp) { > - WARN_ON(job->reply->reply_payload_rcv_len > rsp->resid_len); > + WARN_ON(bsg_reply->reply_payload_rcv_len > rsp->resid_len); > > /* set reply (bidi) residual */ > - rsp->resid_len -=3D min(job->reply->reply_payload_rcv_len, > + rsp->resid_len -=3D min(bsg_reply->reply_payload_rcv_len, > rsp->resid_len); > } > blk_complete_request(req); > @@ -3701,6 +3702,8 @@ fc_req_to_bsgjob(struct Scsi_Host *shost, struct = fc_rport *rport, > struct fc_internal *i =3D to_fc_internal(shost->transportt); > struct request *rsp =3D req->next_rq; > struct fc_bsg_job *job; > + struct fc_bsg_request *bsg_request; > + struct fc_bsg_reply *bsg_reply; > int ret; > > BUG_ON(req->special); > @@ -3726,9 +3729,9 @@ fc_req_to_bsgjob(struct Scsi_Host *shost, struct = fc_rport *rport, > if (i->f->dd_bsg_size) > job->dd_data =3D (void *)&job[1]; > spin_lock_init(&job->job_lock); > - job->request =3D (struct fc_bsg_request *)req->cmd; > + bsg_request =3D (struct fc_bsg_request *)req->cmd; > job->request_len =3D req->cmd_len; > - job->reply =3D req->sense; > + bsg_reply =3D req->sense; > job->reply_len =3D SCSI_SENSE_BUFFERSIZE; /* Size of sense buffer > * allocated */ > if (req->bio) { > @@ -3779,11 +3782,13 @@ fc_bsg_host_dispatch(struct request_queue *q, s= truct Scsi_Host *shost, > struct fc_bsg_job *job) > { > struct fc_internal *i =3D to_fc_internal(shost->transportt); > + struct fc_bsg_request *bsg_request =3D job->request; > + struct fc_bsg_reply *bsg_reply =3D job->reply; > int cmdlen =3D sizeof(uint32_t); /* start with length of msgcode */ > int ret; > > /* Validate the host command */ > - switch (job->request->msgcode) { > + switch (bsg_request->msgcode) { > case FC_BSG_HST_ADD_RPORT: > cmdlen +=3D sizeof(struct fc_bsg_host_add_rport); > break; > @@ -3815,7 +3820,7 @@ fc_bsg_host_dispatch(struct request_queue *q, str= uct Scsi_Host *shost, > case FC_BSG_HST_VENDOR: > cmdlen +=3D sizeof(struct fc_bsg_host_vendor); > if ((shost->hostt->vendor_id =3D=3D 0L) || > - (job->request->rqst_data.h_vendor.vendor_id !=3D > + (bsg_request->rqst_data.h_vendor.vendor_id !=3D > shost->hostt->vendor_id)) { > ret =3D -ESRCH; > goto fail_host_msg; > @@ -3840,8 +3845,8 @@ fc_bsg_host_dispatch(struct request_queue *q, str= uct Scsi_Host *shost, > fail_host_msg: > /* return the errno failure code as the only status */ > BUG_ON(job->reply_len < sizeof(uint32_t)); > - job->reply->reply_payload_rcv_len =3D 0; > - job->reply->result =3D ret; > + bsg_reply->reply_payload_rcv_len =3D 0; > + bsg_reply->result =3D ret; > job->reply_len =3D sizeof(uint32_t); > fc_bsg_jobdone(job); > return FC_DISPATCH_UNLOCKED; > @@ -3878,11 +3883,13 @@ fc_bsg_rport_dispatch(struct request_queue *q, = struct Scsi_Host *shost, > struct fc_rport *rport, struct fc_bsg_job *job) > { > struct fc_internal *i =3D to_fc_internal(shost->transportt); > + struct fc_bsg_request *bsg_request =3D job->request; > + struct fc_bsg_reply *bsg_reply =3D job->reply; > int cmdlen =3D sizeof(uint32_t); /* start with length of msgcode */ > int ret; > > /* Validate the rport command */ > - switch (job->request->msgcode) { > + switch (bsg_request->msgcode) { > case FC_BSG_RPT_ELS: > cmdlen +=3D sizeof(struct fc_bsg_rport_els); > goto check_bidi; > @@ -3915,8 +3922,8 @@ check_bidi: > fail_rport_msg: > /* return the errno failure code as the only status */ > BUG_ON(job->reply_len < sizeof(uint32_t)); > - job->reply->reply_payload_rcv_len =3D 0; > - job->reply->result =3D ret; > + bsg_reply->reply_payload_rcv_len =3D 0; > + bsg_reply->result =3D ret; > job->reply_len =3D sizeof(uint32_t); > fc_bsg_jobdone(job); > return FC_DISPATCH_UNLOCKED; > @@ -3937,6 +3944,7 @@ fc_bsg_request_handler(struct request_queue *q, s= truct Scsi_Host *shost, > struct request *req; > struct fc_bsg_job *job; > enum fc_dispatch_result ret; > + struct fc_bsg_reply *bsg_reply; > > if (!get_device(dev)) > return; > @@ -3973,8 +3981,9 @@ fc_bsg_request_handler(struct request_queue *q, s= truct Scsi_Host *shost, > /* check if we have the msgcode value at least */ > if (job->request_len < sizeof(uint32_t)) { > BUG_ON(job->reply_len < sizeof(uint32_t)); > - job->reply->reply_payload_rcv_len =3D 0; > - job->reply->result =3D -ENOMSG; > + bsg_reply =3D job->reply; > + bsg_reply->reply_payload_rcv_len =3D 0; > + bsg_reply->result =3D -ENOMSG; > job->reply_len =3D sizeof(uint32_t); > fc_bsg_jobdone(job); > spin_lock_irq(q->queue_lock); > --=20 Mit freundlichen Gr=FC=DFen / Kind regards Steffen Maier Linux on z Systems Development IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschaeftsfuehrung: Dirk Wittkopp Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294