Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
From: Steffen Maier <hidden>
Date: 2016-10-13 15:16:20
Also in:
linux-scsi, lkml
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 [off-list ref]
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 [off-list ref]
: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 <redacted> --- 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(-)
quoted hunk ↗ jump to hunk
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_trans=
port_fc.c
quoted hunk ↗ jump to hunk
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,
quoted hunk ↗ jump to hunk
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,
quoted hunk ↗ jump to hunk
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,
quoted hunk ↗ jump to hunk
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,
quoted hunk ↗ jump to hunk
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,
quoted hunk ↗ jump to hunk
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,
quoted hunk ↗ jump to hunk
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;
quoted hunk ↗ jump to hunk
@@ -3937,6 +3944,7 @@ fc_bsg_request_handler(struct request_queue *q, s=
truct Scsi_Host *shost,
quoted hunk ↗ jump to hunk
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