Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Tue, Nov 15, 2016 at 04:39:33PM +0100, Johannes Thumshirn wrote: > On Tue, Nov 15, 2016 at 03:31:27PM +0100, Steffen Maier wrote: > > Hi Johannes, > > > > On 11/15/2016 12:56 PM, Johannes Thumshirn wrote: > > > On Tue, Oct 25, 2016 at 09:43:14AM +0200, Johannes Thumshirn wrote: > > > > On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: > > > > > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > > > > > > Hm, still behaves for me like I reported for v2: > > > > > > http://marc.info/?l=linux-scsi&m=147637177902937&w=2 > > > > > > [...] > > > > > > > > > > > > > The rational behind this is, in fc_req_to_bsgjob() we're assigning > > > > > job->request as req->cmd and job->request_len = req->cmd_len. But > > > > > without > > > > > checkinf job->request_len we don't know whether we're save to touch > > > > > job->request (a.k.a. bsg_request). > > > > > > > > Hi Steffen, > > > > Did you have any chance testing this? I hacked fcping to work with > > > > non-FCoE > > > > and rports as well and tested with FCoE and lpfc. No problems seen from > > > > my > > > > side. I've also pused the series (With this change folded in) to my git > > > > tree at [1] if this helps you in any way. > > > > > > > > [1] > > > > https://git.kernel.org/cgit/linux/kernel/git/jth/linux.git/log/?h=scsi-bsg-rewrite-v4 > > > > > > > > > > So I finally have a test system up and running. I have good and bad news. > > > The > > > good news is, I can't get the system crashing with my patches, the bad > > > news is > > > I can't get zfcp_ping and zfcp_show to output something but > > > HBA_STATUS_ERROR > > > with my patches and without. > > Please ignore my last mails, apparently it's a wise idea to check which user > id one has before running zfcp_ping... > > The good news for this is, I can now recreate the crashes you have and thus > have a chance to fix them :-) So JFTR, I was able to fix the 1 problem introduced by this patch, it's the follwoing hunk: @@ -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) { But as fc_req_to_bsgjob() get's deleted in Patch 15/16 the problem is re-introduced. Unfortunately the fix isn't as trivial as for 2/16 so I'm still trying to nail it down. Thanks, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Tue, Nov 15, 2016 at 03:31:27PM +0100, Steffen Maier wrote: > Hi Johannes, > > On 11/15/2016 12:56 PM, Johannes Thumshirn wrote: > > On Tue, Oct 25, 2016 at 09:43:14AM +0200, Johannes Thumshirn wrote: > > > On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: > > > > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > > > > > Hm, still behaves for me like I reported for v2: > > > > > http://marc.info/?l=linux-scsi&m=147637177902937&w=2 > > > > [...] > > > > > > > > > > The rational behind this is, in fc_req_to_bsgjob() we're assigning > > > > job->request as req->cmd and job->request_len = req->cmd_len. But > > > > without > > > > checkinf job->request_len we don't know whether we're save to touch > > > > job->request (a.k.a. bsg_request). > > > > > > Hi Steffen, > > > Did you have any chance testing this? I hacked fcping to work with > > > non-FCoE > > > and rports as well and tested with FCoE and lpfc. No problems seen from my > > > side. I've also pused the series (With this change folded in) to my git > > > tree at [1] if this helps you in any way. > > > > > > [1] > > > https://git.kernel.org/cgit/linux/kernel/git/jth/linux.git/log/?h=scsi-bsg-rewrite-v4 > > > > > > > So I finally have a test system up and running. I have good and bad news. > > The > > good news is, I can't get the system crashing with my patches, the bad news > > is > > I can't get zfcp_ping and zfcp_show to output something but HBA_STATUS_ERROR > > with my patches and without. Please ignore my last mails, apparently it's a wise idea to check which user id one has before running zfcp_ping... The good news for this is, I can now recreate the crashes you have and thus have a chance to fix them :-) Byte, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Tue, Nov 15, 2016 at 03:31:27PM +0100, Steffen Maier wrote: > Hi Johannes, > > On 11/15/2016 12:56 PM, Johannes Thumshirn wrote: > > On Tue, Oct 25, 2016 at 09:43:14AM +0200, Johannes Thumshirn wrote: > > > On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: > > > > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > > > > > Hm, still behaves for me like I reported for v2: > > > > > http://marc.info/?l=linux-scsi&m=147637177902937&w=2 > > > > [...] > > > > > > > > > > The rational behind this is, in fc_req_to_bsgjob() we're assigning > > > > job->request as req->cmd and job->request_len = req->cmd_len. But > > > > without > > > > checkinf job->request_len we don't know whether we're save to touch > > > > job->request (a.k.a. bsg_request). > > > > > > Hi Steffen, > > > Did you have any chance testing this? I hacked fcping to work with > > > non-FCoE > > > and rports as well and tested with FCoE and lpfc. No problems seen from my > > > side. I've also pused the series (With this change folded in) to my git > > > tree at [1] if this helps you in any way. > > > > > > [1] > > > https://git.kernel.org/cgit/linux/kernel/git/jth/linux.git/log/?h=scsi-bsg-rewrite-v4 > > > > > > > So I finally have a test system up and running. I have good and bad news. > > The > > good news is, I can't get the system crashing with my patches, the bad news > > is > > I can't get zfcp_ping and zfcp_show to output something but HBA_STATUS_ERROR > > with my patches and without. > > Assuming you run the latest package version on s390x: > > Do steps 2 and 3 of the procedure in > http://www.ibm.com/support/knowledgecenter/linuxonibm/com.ibm.linux.z.lhdd/lhdd_t_fcp_api_runappl.html > help? No, I do have a correct /etc/hba.conf: $ grep libzfcphbaapi /etc/hba.conf com.ibm.libzfcphbaapi /usr/lib64/libzfcphbaapi.so.0 and bumping the log level via export LIB_ZFCP_HBAAPI_LOG_LEVEL=1 just adds '(vlib.c:105)_initvlib: libzfcphbaapi.so loaded at Nov 15 15:43:23' to the 1st line of zfcp_ping output. Here's the full output: $ zfcp_ping -v -a 0.0.fc00 -d 0x50050763051b473a (vlib.c:105)_initvlib: libzfcphbaapi.so loaded at Nov 15 15:45:33 Sending PNG from BUS_ID=0.0.fc00 WWPN=0xc05076e0f3002344 ID=0x760c1c dev=/dev/bsg/fc_host1 speed=8 GBit/s --- REQUEST cmd = 0x0401 --- 03 00 00 00 fa 01 00 00 04 01 00 01 00 00 00 00 00 00 00 01 00 02 00 08 50 05 07 63 05 1b 47 3a 00 00 00 00 --- RESPONSE rc = 0x1 --- 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 --- REQUEST cmd = 0x0401 --- 03 00 00 00 fa 01 00 00 04 01 00 01 00 00 00 00 00 00 00 01 00 02 00 08 50 05 07 63 05 1b 47 3a 00 00 00 00 --- RESPONSE rc = 0x1 --- 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 --- REQUEST cmd = 0x0401 --- 03 00 00 00 fa 01 00 00 04 01 00 01 00 00 00 00 00 00 00 01 00 02 00 08 50 05 07 63 05 1b 47 3a 00 00 00 00 --- RESPONSE rc = 0x1 --- 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Error received for FPNG request, aborting. -- ping statistics --- min/avg/max = 0.000/0.000/0.000 ms -- I agree this looks a bit suspicious and I'll add some debugs in the kernel to see what's going on. > > The only other thing I can think of from the top of my head is that BSG > ioctls are sensitive regarding ABI and I once had the kernel ioctl return > EINVAL due to unmatching kernel-headers and libzfcphbaapi maps this EINVAL > to HBA_STATUS_ERROR because there is no more specifically suitable HBA > constant [old SUSE bugs 834498 and 834500]. > > > And btw, I renamed the branch to fc-bsg-rewrite-v4 in case you want to clone > > from it (it has patch 2/16 changed to the v3 submission). > > > > Can you please have a look with your setup? > > I'm going to re-test hopefully within the next few days. That'll be great, thanks. Byte, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
Hi Johannes, On 11/15/2016 12:56 PM, Johannes Thumshirn wrote: On Tue, Oct 25, 2016 at 09:43:14AM +0200, Johannes Thumshirn wrote: On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: Hm, still behaves for me like I reported for v2: http://marc.info/?l=linux-scsi&m=147637177902937&w=2 [...] The rational behind this is, in fc_req_to_bsgjob() we're assigning job->request as req->cmd and job->request_len = req->cmd_len. But without checkinf job->request_len we don't know whether we're save to touch job->request (a.k.a. bsg_request). Hi Steffen, Did you have any chance testing this? I hacked fcping to work with non-FCoE and rports as well and tested with FCoE and lpfc. No problems seen from my side. I've also pused the series (With this change folded in) to my git tree at [1] if this helps you in any way. [1] https://git.kernel.org/cgit/linux/kernel/git/jth/linux.git/log/?h=scsi-bsg-rewrite-v4 So I finally have a test system up and running. I have good and bad news. The good news is, I can't get the system crashing with my patches, the bad news is I can't get zfcp_ping and zfcp_show to output something but HBA_STATUS_ERROR with my patches and without. Assuming you run the latest package version on s390x: Do steps 2 and 3 of the procedure in http://www.ibm.com/support/knowledgecenter/linuxonibm/com.ibm.linux.z.lhdd/lhdd_t_fcp_api_runappl.html help? The only other thing I can think of from the top of my head is that BSG ioctls are sensitive regarding ABI and I once had the kernel ioctl return EINVAL due to unmatching kernel-headers and libzfcphbaapi maps this EINVAL to HBA_STATUS_ERROR because there is no more specifically suitable HBA constant [old SUSE bugs 834498 and 834500]. And btw, I renamed the branch to fc-bsg-rewrite-v4 in case you want to clone from it (it has patch 2/16 changed to the v3 submission). Can you please have a look with your setup? I'm going to re-test hopefully within the next few days. -- 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
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Tue, Oct 25, 2016 at 09:43:14AM +0200, Johannes Thumshirn wrote: > On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: > > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > > > Hm, still behaves for me like I reported for v2: > > > http://marc.info/?l=linux-scsi&m=147637177902937&w=2 [...] > > > > The rational behind this is, in fc_req_to_bsgjob() we're assigning > > job->request as req->cmd and job->request_len = req->cmd_len. But without > > checkinf job->request_len we don't know whether we're save to touch > > job->request (a.k.a. bsg_request). > > Hi Steffen, > Did you have any chance testing this? I hacked fcping to work with non-FCoE > and rports as well and tested with FCoE and lpfc. No problems seen from my > side. I've also pused the series (With this change folded in) to my git > tree at [1] if this helps you in any way. > > [1] > https://git.kernel.org/cgit/linux/kernel/git/jth/linux.git/log/?h=scsi-bsg-rewrite-v4 > So I finally have a test system up and running. I have good and bad news. The good news is, I can't get the system crashing with my patches, the bad news is I can't get zfcp_ping and zfcp_show to output something but HBA_STATUS_ERROR with my patches and without. And btw, I renamed the branch to fc-bsg-rewrite-v4 in case you want to clone from it (it has patch 2/16 changed to the v3 submission). Can you please have a look with your setup? Thanks, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Thu, Nov 03, 2016 at 08:17:51AM -0700, Christoph Hellwig wrote: > On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: > > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > > > Hm, still behaves for me like I reported for v2: > > > http://marc.info/?l=linux-scsi&m=147637177902937&w=2 > > > > Hi Steffen, > > > > Can you please try the following on top of 2/16? > > How does this fit into the patch we're replying to? Sorry but I don't quite get you. If you look at a mixed source assembly listing for fc_host_dispatch() [1] we load the the offset of 24 from %r12 into %r1 and then the beginning of %r1 into %r2 and crash. So if we now check if job->request_len is smaller than uint32_t we know we can't have a messagecode in the request and bail out early, instead of accessing a possible NULL pointer. At least that's my analysis, feel free to correct me if I'm wrong. [1] Listing: static int fc_bsg_host_dispatch(struct Scsi_Host *shost, struct bsg_job *job) { struct fc_internal *i = to_fc_internal(shost->transportt); struct fc_bsg_request *bsg_request = job->request; 5d54: e3 10 c0 18 00 04 lg %r1,24(%r12) struct fc_bsg_reply *bsg_reply = job->reply; 5d5a: e3 b0 c0 20 00 04 lg %r11,32(%r12) int cmdlen = sizeof(uint32_t); /* start with length of msgcode */ int ret; /* check if we really have all the request data needed */ if (job->request_len < cmdlen) { 5d60: a7 c4 00 48 jle 5df0 ret = -ENOMSG; goto fail_host_msg; } /* Validate the host command */ switch (bsg_request->msgcode) { 5d64: 58 20 10 00 l %r2,0(%r1) 5d68: c2 2f 80 00 00 03 clfi%r2,2147483651 5d6e: a7 84 00 1a je 5da2 5d72: a7 24 00 0a jh 5d86 5d76: c0 19 80 00 00 01 iilf%r1,2147483649 5d7c: ec 21 00 2b a0 77 clrj%r2,%r1,10,5dd2 5d82: a7 f4 00 3b j 5df8 5d86: c0 59 80 00 00 04 iilf%r5,2147483652 5d8c: ec 25 00 0b 80 76 crj %r2,%r5,8,5da2 5d92: c0 59 80 00 00 ff iilf%r5,2147483903 5d98: ec 25 00 13 80 76 crj %r2,%r5,8,5dbe 5d9e: a7 f4 00 2d j 5df8 Thanks, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > > Hm, still behaves for me like I reported for v2: > > http://marc.info/?l=linux-scsi&m=147637177902937&w=2 > > Hi Steffen, > > Can you please try the following on top of 2/16? How does this fit into the patch we're replying to?
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > > Hm, still behaves for me like I reported for v2: > > http://marc.info/?l=linux-scsi&m=147637177902937&w=2 > > Hi Steffen, > > Can you please try the following on top of 2/16? > > diff --git a/drivers/scsi/scsi_transport_fc.c > b/drivers/scsi/scsi_transport_fc.c > index 4149dac..baebaab 100644 > --- a/drivers/scsi/scsi_transport_fc.c > +++ b/drivers/scsi/scsi_transport_fc.c > @@ -3786,6 +3786,12 @@ enum fc_dispatch_result { > int cmdlen = sizeof(uint32_t); /* start with length of msgcode */ > int ret; > > + /* check if we really have all the request data needed */ > + if (job->request_len < cmdlen) { > + ret = -ENOMSG; > + goto fail_host_msg; > + } > + > /* Validate the host command */ > switch (bsg_request->msgcode) { > case FC_BSG_HST_ADD_RPORT: > @@ -3831,12 +3837,6 @@ enum fc_dispatch_result { > goto fail_host_msg; > } > > - /* check if we really have all the request data needed */ > - if (job->request_len < cmdlen) { > - ret = -ENOMSG; > - goto fail_host_msg; > - } > - > ret = i->f->bsg_request(job); > if (!ret) > return FC_DISPATCH_UNLOCKED; > @@ -3887,6 +3887,12 @@ enum fc_dispatch_result { > int cmdlen = sizeof(uint32_t); /* start with length of msgcode */ > int ret; > > + /* check if we really have all the request data needed */ > + if (job->request_len < cmdlen) { > + ret = -ENOMSG; > + goto fail_rport_msg; > + } > + > /* Validate the rport command */ > switch (bsg_request->msgcode) { > case FC_BSG_RPT_ELS: > > > > The rational behind this is, in fc_req_to_bsgjob() we're assigning > job->request as req->cmd and job->request_len = req->cmd_len. But without > checkinf job->request_len we don't know whether we're save to touch > job->request (a.k.a. bsg_request). Hi Steffen, Did you have any chance testing this? I hacked fcping to work with non-FCoE and rports as well and tested with FCoE and lpfc. No problems seen from my side. I've also pused the series (With this change folded in) to my git tree at [1] if this helps you in any way. [1] https://git.kernel.org/cgit/linux/kernel/git/jth/linux.git/log/?h=scsi-bsg-rewrite-v4 Thanks a lot, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > Hm, still behaves for me like I reported for v2: > http://marc.info/?l=linux-scsi&m=147637177902937&w=2 Hi Steffen, Can you please try the following on top of 2/16? diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c index 4149dac..baebaab 100644 --- a/drivers/scsi/scsi_transport_fc.c +++ b/drivers/scsi/scsi_transport_fc.c @@ -3786,6 +3786,12 @@ enum fc_dispatch_result { int cmdlen = sizeof(uint32_t); /* start with length of msgcode */ int ret; + /* check if we really have all the request data needed */ + if (job->request_len < cmdlen) { + ret = -ENOMSG; + goto fail_host_msg; + } + /* Validate the host command */ switch (bsg_request->msgcode) { case FC_BSG_HST_ADD_RPORT: @@ -3831,12 +3837,6 @@ enum fc_dispatch_result { goto fail_host_msg; } - /* check if we really have all the request data needed */ - if (job->request_len < cmdlen) { - ret = -ENOMSG; - goto fail_host_msg; - } - ret = i->f->bsg_request(job); if (!ret) return FC_DISPATCH_UNLOCKED; @@ -3887,6 +3887,12 @@ enum fc_dispatch_result { int cmdlen = sizeof(uint32_t); /* start with length of msgcode */ int ret; + /* check if we really have all the request data needed */ + if (job->request_len < cmdlen) { + ret = -ENOMSG; + goto fail_rport_msg; + } + /* Validate the rport command */ switch (bsg_request->msgcode) { case FC_BSG_RPT_ELS: The rational behind this is, in fc_req_to_bsgjob() we're assigning job->request as req->cmd and job->request_len = req->cmd_len. But without checkinf job->request_len we don't know whether we're save to touch job->request (a.k.a. bsg_request). In the meanwhile I try to reproduce your report here. Thanks, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
Hm, still behaves for me like I reported for v2: http://marc.info/?l=linux-scsi&m=147637177902937&w=2 On 10/13/2016 05:00 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 Reviewed-by: Hannes Reinecke --- 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 @@ -3973,8 +3981,9 @@ enum fc_dispatch_result { /* 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
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > Hm, still behaves for me like I reported for v2: > http://marc.info/?l=linux-scsi&m=147637177902937&w=2 > Well given what you've wrote for v2 it's kinda expected. Byte, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
[PATCH v3 02/16] 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 Reviewed-by: Hannes Reinecke --- 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/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c index 237688a..4c4023f 100644 --- a/drivers/s390/scsi/zfcp_fc.c +++ b/drivers/s390/scsi/zfcp_fc.c @@ -900,8 +900,9 @@ static struct zfcp_fc_wka_port *zfcp_fc_job_wka_port(struct fc_bsg_job *job) u32 preamble_word1; u8 gs_type; struct zfcp_adapter *adapter; + struct fc_bsg_request *bsg_request = job->request; - preamble_word1 = job->request->rqst_data.r_ct.preamble_word1; + preamble_word1 = bsg_request->rqst_data.r_ct.preamble_word1; gs_type = (preamble_word1 & 0xff00) >> 24; adapter = (struct zfcp_adapter *) job->shost->hostdata[0]; @@ -938,6 +939,7 @@ static int zfcp_fc_exec_els_job(struct fc_bsg_job *job, { struct zfcp_fsf_ct_els *els = job->dd_data; struct fc_rport *rport = job->rport; + struct fc_bsg_request *bsg_request = job->request; struct zfcp_port *port; u32 d_id; @@ -949,7 +951,7 @@ static int zfcp_fc_exec_els_job(struct fc_bsg_job *job, d_id = port->d_id; put_device(&port->dev); } else - d_id = ntoh24(job->request->rqst_data.h_els.port_id); + d_id = ntoh24(bsg_request->rqst_data.h_els.port_id); els->handler = zfcp_fc_ct_els_job_handler; return zfcp_fsf_send_els(adapter, d_id, els, job->req->timeout / HZ); @@ -983,6 +985,7 @@ int zfcp_fc_exec_bsg_job(struct fc_bsg_job *job) struct Scsi_Host *shost; struct zfcp_adapter *adapter; struct zfcp_fsf_ct_els *ct_els = job->dd_data; + struct fc_bsg_request *bsg_request = job->request; shost = job->rport ? rport_to_shost(job->rport) : job->shost; adapter = (struct zfcp_adapter *)shost->hostdata[0]; @@ -994,7 +997,7 @@ int zfcp_fc_exec_bsg_job(struct fc_bsg_job *job) ct_els->resp = job->reply_payload.sg_list; ct_els->handler_data = job; - switch (job->request->msgcode) { + switch (bsg_request->msgcode) { case FC_BSG_RPT_ELS: case FC_BSG_HST_ELS_NOLOGIN: return zfcp_fc_exec_els_job(job, adapter); diff --git a/drivers/scsi/bfa/bfad_bsg.c b/drivers/scsi/bfa/bfad_bsg.c index d1ad020..48366d8 100644 --- a/drivers/scsi/bfa/bfad_bsg.c +++ b/drivers/scsi/bfa/bfad_bsg.c @@ -3132,7 +3132,9 @@ static int bfad_im_bsg_vendor_request(struct fc_bsg_job *job) { - uint32_t vendor_cmd = job->request->rqst_data.h_vendor.vendor_cmd[0]; + struct fc_bsg_request *bsg_request = job->request; + struct fc_bsg_reply *bsg_reply = job->reply; + uint32_t vendor_cmd = bsg_request->rqst_data.h_vendor.vendor_cmd[0]; struct bfad_im_port_s *im_port = (struct bfad_im_port_s *) job->shost->hostdata[0]; struct bfad_s *bfad = im_port->bfad; @@ -3175,8 +3177,8 @@ /* Fill the BSG job reply data */ job->reply_len = job->reply_payload.payload_len; - job->reply->reply_payload_rcv_len = job->reply_payload.payload_len; - job->reply->result = rc; + bsg_reply->reply_payload_rcv_len = job->reply_payload.payload_len; + bsg_reply->result = rc; job->job_done(job); return rc; @@ -3184,9 +3186,9 @@ /* free the command buffer */ kfree(payload_kbuf); out: - job->reply->result = rc; + bsg_reply->result = rc; job->reply_len = sizeof(uint32_t); - job->reply->reply_payload_rcv_len = 0; + bsg_reply->reply_payload_rcv_len = 0; return rc; } @@ -3362,18 +3364,20 @@ struct bfad_buf_info * struct bfad_fcxp*drv_fcxp; struct bfa_fcs_lport_s *fcs_port; struct bfa_fcs_rport_s *fcs_rport; - uint32_t command_type = job->request->msgcode; + struct fc_bsg_request *bsg_request = bsg_request; + struct fc_bsg_reply *bsg_reply = job->reply; + uint32_t command_type = bsg_request->msgcode; unsigned long flags; struct bfad_buf_info *rsp_buf_info; void *req_kbuf = NULL, *rsp_kbuf = NULL; int rc = -EINVAL; job->reply_len = sizeof(uint32_t); /* Atleast uint32_t reply_len */ - job->reply->reply_pa