diff mbox series

bsg: convert to use blk-mq

Message ID d8dec904-341e-6a13-7b87-f5e360d86ff8@kernel.dk (mailing list archive)
State Not Applicable
Headers show
Series bsg: convert to use blk-mq | expand

Commit Message

Jens Axboe Oct. 16, 2018, 2:43 p.m. UTC 
Requires a few changes to the FC transport class as well.

Cc: Johannes Thumshirn <jthumshirn@suse.de>
Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 block/bsg-lib.c                  | 102 +++++++++++++++++--------------
 drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
 2 files changed, 91 insertions(+), 72 deletions(-)

Comments

Benjamin Block Oct. 17, 2018, 3:55 p.m. UTC | #1 
On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
> Requires a few changes to the FC transport class as well.
> 
> Cc: Johannes Thumshirn <jthumshirn@suse.de>
> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
> Cc: linux-scsi@vger.kernel.org
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>  block/bsg-lib.c                  | 102 +++++++++++++++++--------------
>  drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
>  2 files changed, 91 insertions(+), 72 deletions(-)
> 

Hey Jens,

I haven't had time to look into this in any deep way - but I did plan to
-, but just to see whether it starts and runs some I/O I tried giving it
a spin and came up with nothing (see line 3 and 5):

[   14.082079] scsi host0: scsi_eh_0: sleeping
[   14.082288] scsi host0: zfcp
[   14.086823] scsi host0: fc_host0: bsg interface failed to initialize - setup queue
[   14.089198] qdio: 0.0.1900 ZFCP on SC 107 using AI:1 QEBSM:0 PRI:1 TDD:1 SIGA: W AP
[   15.228583]  rport-0:0-0: failed to setup bsg queue
[   15.229886] scsi 0:0:0:0: scsi scan: INQUIRY pass 1 length 36
[   15.230801] scsi 0:0:0:0: scsi scan: INQUIRY successful with code 0x0
[   15.230860] scsi 0:0:0:0: scsi scan: INQUIRY pass 2 length 164
[   15.231171] scsi 0:0:0:0: scsi scan: INQUIRY successful with code 0x0
[   15.231190] scsi 0:0:0:0: scsi scan: peripheral device type of 31, no device added
[   15.233171] scsi 0:0:0:0: scsi scan: Sending REPORT LUNS to (try 0)
[   15.234144] scsi 0:0:0:0: scsi scan: REPORT LUNS successful (try 0) result 0x0
[   15.234156] scsi 0:0:0:0: scsi scan: REPORT LUN scan
[   15.235040] scsi 0:0:0:1082671104: scsi scan: INQUIRY pass 1 length 36
[   15.235220] scsi host1: scsi_eh_1: sleeping
[   15.235336] scsi 0:0:0:1082671104: scsi scan: INQUIRY successful with code 0x0
[   15.235355] scsi 0:0:0:1082671104: scsi scan: INQUIRY pass 2 length 164
[   15.235434] scsi host1: zfcp
[   15.235667] scsi 0:0:0:1082671104: scsi scan: INQUIRY successful with code 0x0
[   15.235709] scsi 0:0:0:1082671104: Direct-Access     IBM      2107900          3230 PQ: 0 ANSI: 5
[   15.238468] scsi host1: fc_host1: bsg interface failed to initialize - setup queue
....

Seems to already fail at setting up the bsg interfaces for zFCP - at
loading time of the module. This is just your patch on top of
4.19.0-rc8.
Jens Axboe Oct. 17, 2018, 4:01 p.m. UTC | #2 
On 10/17/18 9:55 AM, Benjamin Block wrote:
> On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
>> Requires a few changes to the FC transport class as well.
>>
>> Cc: Johannes Thumshirn <jthumshirn@suse.de>
>> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
>> Cc: linux-scsi@vger.kernel.org
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>  block/bsg-lib.c                  | 102 +++++++++++++++++--------------
>>  drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
>>  2 files changed, 91 insertions(+), 72 deletions(-)
>>
> 
> Hey Jens,
> 
> I haven't had time to look into this in any deep way - but I did plan to
> -, but just to see whether it starts and runs some I/O I tried giving it
> a spin and came up with nothing (see line 3 and 5):

I'm an idiot, can you try this on top?


diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index 1aa0ed3fc339..95e12b635225 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -311,7 +311,7 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
 	int ret = -ENOMEM;
 
 	set = kzalloc(sizeof(*set), GFP_KERNEL);
-	if (set)
+	if (!set)
 		return ERR_PTR(-ENOMEM);
 
 	set->ops = &bsg_mq_ops;
Douglas Gilbert Oct. 17, 2018, 6:08 p.m. UTC | #3 
On 2018-10-17 11:55 a.m., Benjamin Block wrote:
> On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
>> Requires a few changes to the FC transport class as well.
>>
>> Cc: Johannes Thumshirn <jthumshirn@suse.de>
>> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
>> Cc: linux-scsi@vger.kernel.org
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>   block/bsg-lib.c                  | 102 +++++++++++++++++--------------
>>   drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
>>   2 files changed, 91 insertions(+), 72 deletions(-)
>>
> 
> Hey Jens,
> 
> I haven't had time to look into this in any deep way - but I did plan to
> -, but just to see whether it starts and runs some I/O I tried giving it
> a spin and came up with nothing (see line 3 and 5):
> 
> [   14.082079] scsi host0: scsi_eh_0: sleeping
> [   14.082288] scsi host0: zfcp
> [   14.086823] scsi host0: fc_host0: bsg interface failed to initialize - setup queue
> [   14.089198] qdio: 0.0.1900 ZFCP on SC 107 using AI:1 QEBSM:0 PRI:1 TDD:1 SIGA: W AP
> [   15.228583]  rport-0:0-0: failed to setup bsg queue
> [   15.229886] scsi 0:0:0:0: scsi scan: INQUIRY pass 1 length 36
> [   15.230801] scsi 0:0:0:0: scsi scan: INQUIRY successful with code 0x0
> [   15.230860] scsi 0:0:0:0: scsi scan: INQUIRY pass 2 length 164
> [   15.231171] scsi 0:0:0:0: scsi scan: INQUIRY successful with code 0x0
> [   15.231190] scsi 0:0:0:0: scsi scan: peripheral device type of 31, no device added
> [   15.233171] scsi 0:0:0:0: scsi scan: Sending REPORT LUNS to (try 0)
> [   15.234144] scsi 0:0:0:0: scsi scan: REPORT LUNS successful (try 0) result 0x0
> [   15.234156] scsi 0:0:0:0: scsi scan: REPORT LUN scan
> [   15.235040] scsi 0:0:0:1082671104: scsi scan: INQUIRY pass 1 length 36
> [   15.235220] scsi host1: scsi_eh_1: sleeping
> [   15.235336] scsi 0:0:0:1082671104: scsi scan: INQUIRY successful with code 0x0
> [   15.235355] scsi 0:0:0:1082671104: scsi scan: INQUIRY pass 2 length 164
> [   15.235434] scsi host1: zfcp
> [   15.235667] scsi 0:0:0:1082671104: scsi scan: INQUIRY successful with code 0x0
> [   15.235709] scsi 0:0:0:1082671104: Direct-Access     IBM      2107900          3230 PQ: 0 ANSI: 5
> [   15.238468] scsi host1: fc_host1: bsg interface failed to initialize - setup queue
> ....
> 
> Seems to already fail at setting up the bsg interfaces for zFCP - at
> loading time of the module. This is just your patch on top of
> 4.19.0-rc8.

Hi,
Almost all of the utilities in the sg3_utils package (a few exceptions:
sg_reset, sgm_dd, sgp_dd), when given a bsg file node (e.g.
'sg_inq /dev/bsg/0:0:0:1'), will use the bsg sg_v4 interface (i.e. as
defined in <uapi/linux/bsg.h>). Dito with the sdparm and ddpt packages.
So there is a lot of test code for any changes to the bsg driver.

Doug Gilbert


P.S. that above is true from sg3_utils version 1.27 released on
      20090411. The current version is 1.44
Jens Axboe Oct. 17, 2018, 6:15 p.m. UTC | #4 
On 10/17/18 12:08 PM, Douglas Gilbert wrote:
> On 2018-10-17 11:55 a.m., Benjamin Block wrote:
>> On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
>>> Requires a few changes to the FC transport class as well.
>>>
>>> Cc: Johannes Thumshirn <jthumshirn@suse.de>
>>> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
>>> Cc: linux-scsi@vger.kernel.org
>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>> ---
>>>   block/bsg-lib.c                  | 102 +++++++++++++++++--------------
>>>   drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
>>>   2 files changed, 91 insertions(+), 72 deletions(-)
>>>
>>
>> Hey Jens,
>>
>> I haven't had time to look into this in any deep way - but I did plan to
>> -, but just to see whether it starts and runs some I/O I tried giving it
>> a spin and came up with nothing (see line 3 and 5):
>>
>> [   14.082079] scsi host0: scsi_eh_0: sleeping
>> [   14.082288] scsi host0: zfcp
>> [   14.086823] scsi host0: fc_host0: bsg interface failed to initialize - setup queue
>> [   14.089198] qdio: 0.0.1900 ZFCP on SC 107 using AI:1 QEBSM:0 PRI:1 TDD:1 SIGA: W AP
>> [   15.228583]  rport-0:0-0: failed to setup bsg queue
>> [   15.229886] scsi 0:0:0:0: scsi scan: INQUIRY pass 1 length 36
>> [   15.230801] scsi 0:0:0:0: scsi scan: INQUIRY successful with code 0x0
>> [   15.230860] scsi 0:0:0:0: scsi scan: INQUIRY pass 2 length 164
>> [   15.231171] scsi 0:0:0:0: scsi scan: INQUIRY successful with code 0x0
>> [   15.231190] scsi 0:0:0:0: scsi scan: peripheral device type of 31, no device added
>> [   15.233171] scsi 0:0:0:0: scsi scan: Sending REPORT LUNS to (try 0)
>> [   15.234144] scsi 0:0:0:0: scsi scan: REPORT LUNS successful (try 0) result 0x0
>> [   15.234156] scsi 0:0:0:0: scsi scan: REPORT LUN scan
>> [   15.235040] scsi 0:0:0:1082671104: scsi scan: INQUIRY pass 1 length 36
>> [   15.235220] scsi host1: scsi_eh_1: sleeping
>> [   15.235336] scsi 0:0:0:1082671104: scsi scan: INQUIRY successful with code 0x0
>> [   15.235355] scsi 0:0:0:1082671104: scsi scan: INQUIRY pass 2 length 164
>> [   15.235434] scsi host1: zfcp
>> [   15.235667] scsi 0:0:0:1082671104: scsi scan: INQUIRY successful with code 0x0
>> [   15.235709] scsi 0:0:0:1082671104: Direct-Access     IBM      2107900          3230 PQ: 0 ANSI: 5
>> [   15.238468] scsi host1: fc_host1: bsg interface failed to initialize - setup queue
>> ....
>>
>> Seems to already fail at setting up the bsg interfaces for zFCP - at
>> loading time of the module. This is just your patch on top of
>> 4.19.0-rc8.
> 
> Hi,
> Almost all of the utilities in the sg3_utils package (a few exceptions:
> sg_reset, sgm_dd, sgp_dd), when given a bsg file node (e.g.
> 'sg_inq /dev/bsg/0:0:0:1'), will use the bsg sg_v4 interface (i.e. as
> defined in <uapi/linux/bsg.h>). Dito with the sdparm and ddpt packages.
> So there is a lot of test code for any changes to the bsg driver.

That's good info, I did in fact run test tools on the bsg nodes and
verified that it worked fine. But I could not test the embedded queues
like Benjamin is doing above - and those were broken due to a silly
wrong pointer check. So hopefully with that fixed, it'll work just
fine.
Benjamin Block Oct. 19, 2018, 3:01 p.m. UTC | #5 
On Wed, Oct 17, 2018 at 10:01:16AM -0600, Jens Axboe wrote:
> On 10/17/18 9:55 AM, Benjamin Block wrote:
> > On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
> >> Requires a few changes to the FC transport class as well.
> >>
> >> Cc: Johannes Thumshirn <jthumshirn@suse.de>
> >> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
> >> Cc: linux-scsi@vger.kernel.org
> >> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> >> ---
> >>  block/bsg-lib.c                  | 102 +++++++++++++++++--------------
> >>  drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
> >>  2 files changed, 91 insertions(+), 72 deletions(-)
> >>
> > 
> > Hey Jens,
> > 
> > I haven't had time to look into this in any deep way - but I did plan to
> > -, but just to see whether it starts and runs some I/O I tried giving it
> > a spin and came up with nothing (see line 3 and 5):
> 
> I'm an idiot, can you try this on top?
> 
> 
> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
> index 1aa0ed3fc339..95e12b635225 100644
> --- a/block/bsg-lib.c
> +++ b/block/bsg-lib.c
> @@ -311,7 +311,7 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
>  	int ret = -ENOMEM;
>  
>  	set = kzalloc(sizeof(*set), GFP_KERNEL);
> -	if (set)
> +	if (!set)
>  		return ERR_PTR(-ENOMEM);
>  
>  	set->ops = &bsg_mq_ops;
> 

Well, yes, that would be wrong. But it still doesn't fly (s390x stack
trace):

[   36.271953] scsi host0: scsi_eh_0: sleeping
[   36.272571] scsi host0: zfcp
[   36.298065] WARNING: CPU: 7 PID: 856 at block/blk-settings.c:71 blk_queue_rq_timed_out+0x44/0x60
[   36.298315] Modules linked in: zfcp scsi_transport_fc dm_multipath ....
[   36.299015] CPU: 7 PID: 856 Comm: systemd-udevd Tainted: G        W         4.19.0-rc8-bb-next+ #1
[   36.299059] Hardware name: IBM 3906 M03 704 (LPAR)
[   36.299101] Krnl PSW : 0704e00180000000 0000000000ced494 (blk_queue_rq_timed_out+0x44/0x60)
[   36.299192]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
[   36.299259] Krnl GPRS: 0000000000000000 00000000015cee60 00000000a0ce0180 0000030000000000
[   36.299304]            0000030000000000 0000000000ced486 00000000a5738000 000003ff8069eba0
[   36.299349]            00000000a11ec6a8 00000000a0ce0000 00000000a11ec400 000003ff805ee438
[   36.299394]            00000000a0ce0000 000003ff805f6b00 0000000000ced486 00000000a28af0b0
[   36.299460] Krnl Code: 0000000000ced486: e310c1800002        ltg     %r1,384(%r12)
                          0000000000ced48c: a7840004            brc     8,ced494
                         #0000000000ced490: a7f40001            brc     15,ced492
                         >0000000000ced494: 4120c150            la      %r2,336(%r12)
                          0000000000ced498: c0e5ffc76290        brasl   %r14,5d99b8
                          0000000000ced49e: e3b0c1500024        stg     %r11,336(%r12)
                          0000000000ced4a4: ebbff0a00004        lmg     %r11,%r15,160(%r15)
                          0000000000ced4aa: 07fe                bcr     15,%r14
[   36.299879] Call Trace:
[   36.299922] ([<00000000a11ec6a8>] 0xa11ec6a8)
[   36.299979]  [<000003ff805ee3fa>] fc_host_setup+0x622/0x660 [scsi_transport_fc] 
[   36.300029]  [<0000000000f0baca>] transport_setup_classdev+0x62/0x70 
[   36.300075]  [<0000000000f0b592>] attribute_container_add_device+0x182/0x1d0 
[   36.300163]  [<000003ff80503cae>] scsi_sysfs_add_host+0x5e/0x100 [scsi_mod] 
[   36.300245]  [<000003ff804e6d7c>] scsi_add_host_with_dma+0x2dc/0x468 [scsi_mod] 
[   36.300310]  [<000003ff806835f2>] zfcp_scsi_adapter_register+0x222/0x260 [zfcp] 
[   36.300373]  [<000003ff8066a49a>] zfcp_adapter_enqueue+0xae2/0xb20 [zfcp] 
[   36.300435]  [<000003ff8066b436>] zfcp_ccw_set_online+0xb6/0x208 [zfcp] 
[   36.300482]  [<0000000000f8ad14>] ccw_device_set_online+0x41c/0x878 
[   36.300527]  [<0000000000f8b374>] online_store_recog_and_online+0x204/0x230 
[   36.300572]  [<0000000000f8de20>] online_store+0x290/0x3e8 
[   36.300619]  [<00000000007842c0>] kernfs_fop_write+0x1b0/0x288 
[   36.300663]  [<000000000064217e>] __vfs_write+0xee/0x398 
[   36.300705]  [<00000000006426b4>] vfs_write+0xec/0x238 
[   36.300754]  [<0000000000642abe>] ksys_write+0xd6/0x148 
[   36.300800]  [<000000000137b668>] system_call+0x2b0/0x2d0 
[   36.300843] 5 locks held by systemd-udevd/856:
[   36.300882]  #0: 00000000da3c74e2 (sb_writers#4){.+.+}, at: vfs_write+0xd6/0x238
[   36.301053]  #1: 000000002a1f461f (&of->mutex){+.+.}, at: kernfs_fop_write+0x258/0x288
[   36.301202]  #2: 00000000deb28615 (kn->count#52){.+.+}, at: kernfs_fop_write+0x26e/0x288
[   36.301374]  #3: 000000002ddb9663 (&dev->mutex){....}, at: online_store+0x19c/0x3e8
[   36.301523]  #4: 00000000982b5ed9 (attribute_container_mutex){+.+.}, at: attribute_container_add_device+0x3c/0x1d0
[   36.301675] Last Breaking-Event-Address:
[   36.301717]  [<0000000000ced490>] blk_queue_rq_timed_out+0x40/0x60
[   36.301758] irq event stamp: 9073
[   36.301804] hardirqs last  enabled at (9081): [<000000000028bd7e>] console_unlock+0xa46/0xb20
[   36.301851] hardirqs last disabled at (9088): [<000000000028b5b6>] console_unlock+0x27e/0xb20
[   36.301900] softirqs last  enabled at (8334): [<00000000010853c0>] peernet2id+0xd8/0xf0
[   36.301947] softirqs last disabled at (8332): [<0000000001085380>] peernet2id+0x98/0xf0
[   36.301989] ---[ end trace 32ec744f85308e9e ]---
[   36.307495] qdio: 0.0.1900 ZFCP on SC 107 using AI:1 QEBSM:0 PRI:1 TDD:1 SIGA: W AP

This should be here:

	static int
	fc_bsg_hostadd(struct Scsi_Host *shost, struct fc_host_attrs *fc_host)
	{
		struct device *dev = &shost->shost_gendev;
		struct fc_internal *i = to_fc_internal(shost->transportt);
		struct request_queue *q;
		char bsg_name[20];

		fc_host->rqst_q = NULL;

		if (!i->f->bsg_request)
			return -ENOTSUPP;

		snprintf(bsg_name, sizeof(bsg_name),
			 "fc_host%d", shost->host_no);

		q = bsg_setup_queue(dev, bsg_name, fc_bsg_dispatch, i->f->dd_bsg_size);
		if (IS_ERR(q)) {
			dev_err(dev,
				"fc_host%d: bsg interface failed to initialize - setup queue\n",
				shost->host_no);
			return PTR_ERR(q);
		}
		__scsi_init_queue(shost, q);
====>		blk_queue_rq_timed_out(q, fc_bsg_job_timeout);
		fc_host->rqst_q = q;
		return 0;
	}

Guessing it has also to do with the series.
Jens Axboe Oct. 19, 2018, 3:50 p.m. UTC | #6 
On 10/19/18 9:01 AM, Benjamin Block wrote:
> On Wed, Oct 17, 2018 at 10:01:16AM -0600, Jens Axboe wrote:
>> On 10/17/18 9:55 AM, Benjamin Block wrote:
>>> On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
>>>> Requires a few changes to the FC transport class as well.
>>>>
>>>> Cc: Johannes Thumshirn <jthumshirn@suse.de>
>>>> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
>>>> Cc: linux-scsi@vger.kernel.org
>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>> ---
>>>>  block/bsg-lib.c                  | 102 +++++++++++++++++--------------
>>>>  drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
>>>>  2 files changed, 91 insertions(+), 72 deletions(-)
>>>>
>>>
>>> Hey Jens,
>>>
>>> I haven't had time to look into this in any deep way - but I did plan to
>>> -, but just to see whether it starts and runs some I/O I tried giving it
>>> a spin and came up with nothing (see line 3 and 5):
>>
>> I'm an idiot, can you try this on top?
>>
>>
>> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
>> index 1aa0ed3fc339..95e12b635225 100644
>> --- a/block/bsg-lib.c
>> +++ b/block/bsg-lib.c
>> @@ -311,7 +311,7 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
>>  	int ret = -ENOMEM;
>>  
>>  	set = kzalloc(sizeof(*set), GFP_KERNEL);
>> -	if (set)
>> +	if (!set)
>>  		return ERR_PTR(-ENOMEM);
>>  
>>  	set->ops = &bsg_mq_ops;
>>
> 
> Well, yes, that would be wrong. But it still doesn't fly (s390x stack
> trace):
> 
> [   36.271953] scsi host0: scsi_eh_0: sleeping
> [   36.272571] scsi host0: zfcp
> [   36.298065] WARNING: CPU: 7 PID: 856 at block/blk-settings.c:71 blk_queue_rq_timed_out+0x44/0x60
> [   36.298315] Modules linked in: zfcp scsi_transport_fc dm_multipath ....
> [   36.299015] CPU: 7 PID: 856 Comm: systemd-udevd Tainted: G        W         4.19.0-rc8-bb-next+ #1
> [   36.299059] Hardware name: IBM 3906 M03 704 (LPAR)
> [   36.299101] Krnl PSW : 0704e00180000000 0000000000ced494 (blk_queue_rq_timed_out+0x44/0x60)
> [   36.299192]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
> [   36.299259] Krnl GPRS: 0000000000000000 00000000015cee60 00000000a0ce0180 0000030000000000
> [   36.299304]            0000030000000000 0000000000ced486 00000000a5738000 000003ff8069eba0
> [   36.299349]            00000000a11ec6a8 00000000a0ce0000 00000000a11ec400 000003ff805ee438
> [   36.299394]            00000000a0ce0000 000003ff805f6b00 0000000000ced486 00000000a28af0b0
> [   36.299460] Krnl Code: 0000000000ced486: e310c1800002        ltg     %r1,384(%r12)
>                           0000000000ced48c: a7840004            brc     8,ced494
>                          #0000000000ced490: a7f40001            brc     15,ced492
>                          >0000000000ced494: 4120c150            la      %r2,336(%r12)
>                           0000000000ced498: c0e5ffc76290        brasl   %r14,5d99b8
>                           0000000000ced49e: e3b0c1500024        stg     %r11,336(%r12)
>                           0000000000ced4a4: ebbff0a00004        lmg     %r11,%r15,160(%r15)
>                           0000000000ced4aa: 07fe                bcr     15,%r14
> [   36.299879] Call Trace:
> [   36.299922] ([<00000000a11ec6a8>] 0xa11ec6a8)
> [   36.299979]  [<000003ff805ee3fa>] fc_host_setup+0x622/0x660 [scsi_transport_fc] 
> [   36.300029]  [<0000000000f0baca>] transport_setup_classdev+0x62/0x70 
> [   36.300075]  [<0000000000f0b592>] attribute_container_add_device+0x182/0x1d0 
> [   36.300163]  [<000003ff80503cae>] scsi_sysfs_add_host+0x5e/0x100 [scsi_mod] 
> [   36.300245]  [<000003ff804e6d7c>] scsi_add_host_with_dma+0x2dc/0x468 [scsi_mod] 
> [   36.300310]  [<000003ff806835f2>] zfcp_scsi_adapter_register+0x222/0x260 [zfcp] 
> [   36.300373]  [<000003ff8066a49a>] zfcp_adapter_enqueue+0xae2/0xb20 [zfcp] 
> [   36.300435]  [<000003ff8066b436>] zfcp_ccw_set_online+0xb6/0x208 [zfcp] 
> [   36.300482]  [<0000000000f8ad14>] ccw_device_set_online+0x41c/0x878 
> [   36.300527]  [<0000000000f8b374>] online_store_recog_and_online+0x204/0x230 
> [   36.300572]  [<0000000000f8de20>] online_store+0x290/0x3e8 
> [   36.300619]  [<00000000007842c0>] kernfs_fop_write+0x1b0/0x288 
> [   36.300663]  [<000000000064217e>] __vfs_write+0xee/0x398 
> [   36.300705]  [<00000000006426b4>] vfs_write+0xec/0x238 
> [   36.300754]  [<0000000000642abe>] ksys_write+0xd6/0x148 
> [   36.300800]  [<000000000137b668>] system_call+0x2b0/0x2d0 
> [   36.300843] 5 locks held by systemd-udevd/856:
> [   36.300882]  #0: 00000000da3c74e2 (sb_writers#4){.+.+}, at: vfs_write+0xd6/0x238
> [   36.301053]  #1: 000000002a1f461f (&of->mutex){+.+.}, at: kernfs_fop_write+0x258/0x288
> [   36.301202]  #2: 00000000deb28615 (kn->count#52){.+.+}, at: kernfs_fop_write+0x26e/0x288
> [   36.301374]  #3: 000000002ddb9663 (&dev->mutex){....}, at: online_store+0x19c/0x3e8
> [   36.301523]  #4: 00000000982b5ed9 (attribute_container_mutex){+.+.}, at: attribute_container_add_device+0x3c/0x1d0
> [   36.301675] Last Breaking-Event-Address:
> [   36.301717]  [<0000000000ced490>] blk_queue_rq_timed_out+0x40/0x60
> [   36.301758] irq event stamp: 9073
> [   36.301804] hardirqs last  enabled at (9081): [<000000000028bd7e>] console_unlock+0xa46/0xb20
> [   36.301851] hardirqs last disabled at (9088): [<000000000028b5b6>] console_unlock+0x27e/0xb20
> [   36.301900] softirqs last  enabled at (8334): [<00000000010853c0>] peernet2id+0xd8/0xf0
> [   36.301947] softirqs last disabled at (8332): [<0000000001085380>] peernet2id+0x98/0xf0
> [   36.301989] ---[ end trace 32ec744f85308e9e ]---
> [   36.307495] qdio: 0.0.1900 ZFCP on SC 107 using AI:1 QEBSM:0 PRI:1 TDD:1 SIGA: W AP
> 
> This should be here:
> 
> 	static int
> 	fc_bsg_hostadd(struct Scsi_Host *shost, struct fc_host_attrs *fc_host)
> 	{
> 		struct device *dev = &shost->shost_gendev;
> 		struct fc_internal *i = to_fc_internal(shost->transportt);
> 		struct request_queue *q;
> 		char bsg_name[20];
> 
> 		fc_host->rqst_q = NULL;
> 
> 		if (!i->f->bsg_request)
> 			return -ENOTSUPP;
> 
> 		snprintf(bsg_name, sizeof(bsg_name),
> 			 "fc_host%d", shost->host_no);
> 
> 		q = bsg_setup_queue(dev, bsg_name, fc_bsg_dispatch, i->f->dd_bsg_size);
> 		if (IS_ERR(q)) {
> 			dev_err(dev,
> 				"fc_host%d: bsg interface failed to initialize - setup queue\n",
> 				shost->host_no);
> 			return PTR_ERR(q);
> 		}
> 		__scsi_init_queue(shost, q);
> ====>		blk_queue_rq_timed_out(q, fc_bsg_job_timeout);
> 		fc_host->rqst_q = q;
> 		return 0;
> 	}
> 
> Guessing it has also to do with the series.

It does, you'll want this one as well:

http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=e930a73f3112c9c617303253fba4e754eb5d0491

but that's not going to apply cleanly... Can you just try and run my
mq-conversions branch? That has everything, and it also has that
alloc failure fixed.

git://git.kernel.dk/linux-block mq-conversions
Benjamin Block Oct. 19, 2018, 3:57 p.m. UTC | #7 
On Fri, Oct 19, 2018 at 09:50:53AM -0600, Jens Axboe wrote:
> On 10/19/18 9:01 AM, Benjamin Block wrote:
> > On Wed, Oct 17, 2018 at 10:01:16AM -0600, Jens Axboe wrote:
> >> On 10/17/18 9:55 AM, Benjamin Block wrote:
> >>> On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
> >>>> Requires a few changes to the FC transport class as well.
> >>>>
> >>>> Cc: Johannes Thumshirn <jthumshirn@suse.de>
> >>>> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
> >>>> Cc: linux-scsi@vger.kernel.org
> >>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> >>>> ---
> >>>>  block/bsg-lib.c                  | 102 +++++++++++++++++--------------
> >>>>  drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
> >>>>  2 files changed, 91 insertions(+), 72 deletions(-)
> >>>>
> >>>
> >>> Hey Jens,
> >>>
> >>> I haven't had time to look into this in any deep way - but I did plan to
> >>> -, but just to see whether it starts and runs some I/O I tried giving it
> >>> a spin and came up with nothing (see line 3 and 5):
> >>
> >> I'm an idiot, can you try this on top?
> >>
> >>
> >> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
> >> index 1aa0ed3fc339..95e12b635225 100644
> >> --- a/block/bsg-lib.c
> >> +++ b/block/bsg-lib.c
> >> @@ -311,7 +311,7 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
> >>  	int ret = -ENOMEM;
> >>  
> >>  	set = kzalloc(sizeof(*set), GFP_KERNEL);
> >> -	if (set)
> >> +	if (!set)
> >>  		return ERR_PTR(-ENOMEM);
> >>  
> >>  	set->ops = &bsg_mq_ops;
> >>
> > 
> > Well, yes, that would be wrong. But it still doesn't fly (s390x stack
> > trace):
> > 
> > [   36.271953] scsi host0: scsi_eh_0: sleeping
> > [   36.272571] scsi host0: zfcp
> > [   36.298065] WARNING: CPU: 7 PID: 856 at block/blk-settings.c:71 blk_queue_rq_timed_out+0x44/0x60
> > [   36.298315] Modules linked in: zfcp scsi_transport_fc dm_multipath ....
> > [   36.299015] CPU: 7 PID: 856 Comm: systemd-udevd Tainted: G        W         4.19.0-rc8-bb-next+ #1
> > [   36.299059] Hardware name: IBM 3906 M03 704 (LPAR)
> > [   36.299101] Krnl PSW : 0704e00180000000 0000000000ced494 (blk_queue_rq_timed_out+0x44/0x60)
> > [   36.299192]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
> > [   36.299259] Krnl GPRS: 0000000000000000 00000000015cee60 00000000a0ce0180 0000030000000000
> > [   36.299304]            0000030000000000 0000000000ced486 00000000a5738000 000003ff8069eba0
> > [   36.299349]            00000000a11ec6a8 00000000a0ce0000 00000000a11ec400 000003ff805ee438
> > [   36.299394]            00000000a0ce0000 000003ff805f6b00 0000000000ced486 00000000a28af0b0
> > [   36.299460] Krnl Code: 0000000000ced486: e310c1800002        ltg     %r1,384(%r12)
> >                           0000000000ced48c: a7840004            brc     8,ced494
> >                          #0000000000ced490: a7f40001            brc     15,ced492
> >                          >0000000000ced494: 4120c150            la      %r2,336(%r12)
> >                           0000000000ced498: c0e5ffc76290        brasl   %r14,5d99b8
> >                           0000000000ced49e: e3b0c1500024        stg     %r11,336(%r12)
> >                           0000000000ced4a4: ebbff0a00004        lmg     %r11,%r15,160(%r15)
> >                           0000000000ced4aa: 07fe                bcr     15,%r14
> > [   36.299879] Call Trace:
> > [   36.299922] ([<00000000a11ec6a8>] 0xa11ec6a8)
> > [   36.299979]  [<000003ff805ee3fa>] fc_host_setup+0x622/0x660 [scsi_transport_fc] 
> > [   36.300029]  [<0000000000f0baca>] transport_setup_classdev+0x62/0x70 
> > [   36.300075]  [<0000000000f0b592>] attribute_container_add_device+0x182/0x1d0 
> > [   36.300163]  [<000003ff80503cae>] scsi_sysfs_add_host+0x5e/0x100 [scsi_mod] 
> > [   36.300245]  [<000003ff804e6d7c>] scsi_add_host_with_dma+0x2dc/0x468 [scsi_mod] 
> > [   36.300310]  [<000003ff806835f2>] zfcp_scsi_adapter_register+0x222/0x260 [zfcp] 
> > [   36.300373]  [<000003ff8066a49a>] zfcp_adapter_enqueue+0xae2/0xb20 [zfcp] 
> > [   36.300435]  [<000003ff8066b436>] zfcp_ccw_set_online+0xb6/0x208 [zfcp] 
> > [   36.300482]  [<0000000000f8ad14>] ccw_device_set_online+0x41c/0x878 
> > [   36.300527]  [<0000000000f8b374>] online_store_recog_and_online+0x204/0x230 
> > [   36.300572]  [<0000000000f8de20>] online_store+0x290/0x3e8 
> > [   36.300619]  [<00000000007842c0>] kernfs_fop_write+0x1b0/0x288 
> > [   36.300663]  [<000000000064217e>] __vfs_write+0xee/0x398 
> > [   36.300705]  [<00000000006426b4>] vfs_write+0xec/0x238 
> > [   36.300754]  [<0000000000642abe>] ksys_write+0xd6/0x148 
> > [   36.300800]  [<000000000137b668>] system_call+0x2b0/0x2d0 
> > [   36.300843] 5 locks held by systemd-udevd/856:
> > [   36.300882]  #0: 00000000da3c74e2 (sb_writers#4){.+.+}, at: vfs_write+0xd6/0x238
> > [   36.301053]  #1: 000000002a1f461f (&of->mutex){+.+.}, at: kernfs_fop_write+0x258/0x288
> > [   36.301202]  #2: 00000000deb28615 (kn->count#52){.+.+}, at: kernfs_fop_write+0x26e/0x288
> > [   36.301374]  #3: 000000002ddb9663 (&dev->mutex){....}, at: online_store+0x19c/0x3e8
> > [   36.301523]  #4: 00000000982b5ed9 (attribute_container_mutex){+.+.}, at: attribute_container_add_device+0x3c/0x1d0
> > [   36.301675] Last Breaking-Event-Address:
> > [   36.301717]  [<0000000000ced490>] blk_queue_rq_timed_out+0x40/0x60
> > [   36.301758] irq event stamp: 9073
> > [   36.301804] hardirqs last  enabled at (9081): [<000000000028bd7e>] console_unlock+0xa46/0xb20
> > [   36.301851] hardirqs last disabled at (9088): [<000000000028b5b6>] console_unlock+0x27e/0xb20
> > [   36.301900] softirqs last  enabled at (8334): [<00000000010853c0>] peernet2id+0xd8/0xf0
> > [   36.301947] softirqs last disabled at (8332): [<0000000001085380>] peernet2id+0x98/0xf0
> > [   36.301989] ---[ end trace 32ec744f85308e9e ]---
> > [   36.307495] qdio: 0.0.1900 ZFCP on SC 107 using AI:1 QEBSM:0 PRI:1 TDD:1 SIGA: W AP
> > 
> > This should be here:
> > 
> > 	static int
> > 	fc_bsg_hostadd(struct Scsi_Host *shost, struct fc_host_attrs *fc_host)
> > 	{
> > 		struct device *dev = &shost->shost_gendev;
> > 		struct fc_internal *i = to_fc_internal(shost->transportt);
> > 		struct request_queue *q;
> > 		char bsg_name[20];
> > 
> > 		fc_host->rqst_q = NULL;
> > 
> > 		if (!i->f->bsg_request)
> > 			return -ENOTSUPP;
> > 
> > 		snprintf(bsg_name, sizeof(bsg_name),
> > 			 "fc_host%d", shost->host_no);
> > 
> > 		q = bsg_setup_queue(dev, bsg_name, fc_bsg_dispatch, i->f->dd_bsg_size);
> > 		if (IS_ERR(q)) {
> > 			dev_err(dev,
> > 				"fc_host%d: bsg interface failed to initialize - setup queue\n",
> > 				shost->host_no);
> > 			return PTR_ERR(q);
> > 		}
> > 		__scsi_init_queue(shost, q);
> > ====>		blk_queue_rq_timed_out(q, fc_bsg_job_timeout);
> > 		fc_host->rqst_q = q;
> > 		return 0;
> > 	}
> > 
> > Guessing it has also to do with the series.
> 
> It does, you'll want this one as well:
> 
> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=e930a73f3112c9c617303253fba4e754eb5d0491
> 
> but that's not going to apply cleanly... Can you just try and run my
> mq-conversions branch? That has everything, and it also has that
> alloc failure fixed.
> 
> git://git.kernel.dk/linux-block mq-conversions
> 

Yeah sure, can give that a spin.
Jens Axboe Oct. 22, 2018, 9:23 a.m. UTC | #8 
On 10/19/18 9:57 AM, Benjamin Block wrote:
> On Fri, Oct 19, 2018 at 09:50:53AM -0600, Jens Axboe wrote:
>> On 10/19/18 9:01 AM, Benjamin Block wrote:
>>> On Wed, Oct 17, 2018 at 10:01:16AM -0600, Jens Axboe wrote:
>>>> On 10/17/18 9:55 AM, Benjamin Block wrote:
>>>>> On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
>>>>>> Requires a few changes to the FC transport class as well.
>>>>>>
>>>>>> Cc: Johannes Thumshirn <jthumshirn@suse.de>
>>>>>> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
>>>>>> Cc: linux-scsi@vger.kernel.org
>>>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>>>> ---
>>>>>>  block/bsg-lib.c                  | 102 +++++++++++++++++--------------
>>>>>>  drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
>>>>>>  2 files changed, 91 insertions(+), 72 deletions(-)
>>>>>>
>>>>>
>>>>> Hey Jens,
>>>>>
>>>>> I haven't had time to look into this in any deep way - but I did plan to
>>>>> -, but just to see whether it starts and runs some I/O I tried giving it
>>>>> a spin and came up with nothing (see line 3 and 5):
>>>>
>>>> I'm an idiot, can you try this on top?
>>>>
>>>>
>>>> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
>>>> index 1aa0ed3fc339..95e12b635225 100644
>>>> --- a/block/bsg-lib.c
>>>> +++ b/block/bsg-lib.c
>>>> @@ -311,7 +311,7 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
>>>>  	int ret = -ENOMEM;
>>>>  
>>>>  	set = kzalloc(sizeof(*set), GFP_KERNEL);
>>>> -	if (set)
>>>> +	if (!set)
>>>>  		return ERR_PTR(-ENOMEM);
>>>>  
>>>>  	set->ops = &bsg_mq_ops;
>>>>
>>>
>>> Well, yes, that would be wrong. But it still doesn't fly (s390x stack
>>> trace):
>>>
>>> [   36.271953] scsi host0: scsi_eh_0: sleeping
>>> [   36.272571] scsi host0: zfcp
>>> [   36.298065] WARNING: CPU: 7 PID: 856 at block/blk-settings.c:71 blk_queue_rq_timed_out+0x44/0x60
>>> [   36.298315] Modules linked in: zfcp scsi_transport_fc dm_multipath ....
>>> [   36.299015] CPU: 7 PID: 856 Comm: systemd-udevd Tainted: G        W         4.19.0-rc8-bb-next+ #1
>>> [   36.299059] Hardware name: IBM 3906 M03 704 (LPAR)
>>> [   36.299101] Krnl PSW : 0704e00180000000 0000000000ced494 (blk_queue_rq_timed_out+0x44/0x60)
>>> [   36.299192]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
>>> [   36.299259] Krnl GPRS: 0000000000000000 00000000015cee60 00000000a0ce0180 0000030000000000
>>> [   36.299304]            0000030000000000 0000000000ced486 00000000a5738000 000003ff8069eba0
>>> [   36.299349]            00000000a11ec6a8 00000000a0ce0000 00000000a11ec400 000003ff805ee438
>>> [   36.299394]            00000000a0ce0000 000003ff805f6b00 0000000000ced486 00000000a28af0b0
>>> [   36.299460] Krnl Code: 0000000000ced486: e310c1800002        ltg     %r1,384(%r12)
>>>                           0000000000ced48c: a7840004            brc     8,ced494
>>>                          #0000000000ced490: a7f40001            brc     15,ced492
>>>                          >0000000000ced494: 4120c150            la      %r2,336(%r12)
>>>                           0000000000ced498: c0e5ffc76290        brasl   %r14,5d99b8
>>>                           0000000000ced49e: e3b0c1500024        stg     %r11,336(%r12)
>>>                           0000000000ced4a4: ebbff0a00004        lmg     %r11,%r15,160(%r15)
>>>                           0000000000ced4aa: 07fe                bcr     15,%r14
>>> [   36.299879] Call Trace:
>>> [   36.299922] ([<00000000a11ec6a8>] 0xa11ec6a8)
>>> [   36.299979]  [<000003ff805ee3fa>] fc_host_setup+0x622/0x660 [scsi_transport_fc] 
>>> [   36.300029]  [<0000000000f0baca>] transport_setup_classdev+0x62/0x70 
>>> [   36.300075]  [<0000000000f0b592>] attribute_container_add_device+0x182/0x1d0 
>>> [   36.300163]  [<000003ff80503cae>] scsi_sysfs_add_host+0x5e/0x100 [scsi_mod] 
>>> [   36.300245]  [<000003ff804e6d7c>] scsi_add_host_with_dma+0x2dc/0x468 [scsi_mod] 
>>> [   36.300310]  [<000003ff806835f2>] zfcp_scsi_adapter_register+0x222/0x260 [zfcp] 
>>> [   36.300373]  [<000003ff8066a49a>] zfcp_adapter_enqueue+0xae2/0xb20 [zfcp] 
>>> [   36.300435]  [<000003ff8066b436>] zfcp_ccw_set_online+0xb6/0x208 [zfcp] 
>>> [   36.300482]  [<0000000000f8ad14>] ccw_device_set_online+0x41c/0x878 
>>> [   36.300527]  [<0000000000f8b374>] online_store_recog_and_online+0x204/0x230 
>>> [   36.300572]  [<0000000000f8de20>] online_store+0x290/0x3e8 
>>> [   36.300619]  [<00000000007842c0>] kernfs_fop_write+0x1b0/0x288 
>>> [   36.300663]  [<000000000064217e>] __vfs_write+0xee/0x398 
>>> [   36.300705]  [<00000000006426b4>] vfs_write+0xec/0x238 
>>> [   36.300754]  [<0000000000642abe>] ksys_write+0xd6/0x148 
>>> [   36.300800]  [<000000000137b668>] system_call+0x2b0/0x2d0 
>>> [   36.300843] 5 locks held by systemd-udevd/856:
>>> [   36.300882]  #0: 00000000da3c74e2 (sb_writers#4){.+.+}, at: vfs_write+0xd6/0x238
>>> [   36.301053]  #1: 000000002a1f461f (&of->mutex){+.+.}, at: kernfs_fop_write+0x258/0x288
>>> [   36.301202]  #2: 00000000deb28615 (kn->count#52){.+.+}, at: kernfs_fop_write+0x26e/0x288
>>> [   36.301374]  #3: 000000002ddb9663 (&dev->mutex){....}, at: online_store+0x19c/0x3e8
>>> [   36.301523]  #4: 00000000982b5ed9 (attribute_container_mutex){+.+.}, at: attribute_container_add_device+0x3c/0x1d0
>>> [   36.301675] Last Breaking-Event-Address:
>>> [   36.301717]  [<0000000000ced490>] blk_queue_rq_timed_out+0x40/0x60
>>> [   36.301758] irq event stamp: 9073
>>> [   36.301804] hardirqs last  enabled at (9081): [<000000000028bd7e>] console_unlock+0xa46/0xb20
>>> [   36.301851] hardirqs last disabled at (9088): [<000000000028b5b6>] console_unlock+0x27e/0xb20
>>> [   36.301900] softirqs last  enabled at (8334): [<00000000010853c0>] peernet2id+0xd8/0xf0
>>> [   36.301947] softirqs last disabled at (8332): [<0000000001085380>] peernet2id+0x98/0xf0
>>> [   36.301989] ---[ end trace 32ec744f85308e9e ]---
>>> [   36.307495] qdio: 0.0.1900 ZFCP on SC 107 using AI:1 QEBSM:0 PRI:1 TDD:1 SIGA: W AP
>>>
>>> This should be here:
>>>
>>> 	static int
>>> 	fc_bsg_hostadd(struct Scsi_Host *shost, struct fc_host_attrs *fc_host)
>>> 	{
>>> 		struct device *dev = &shost->shost_gendev;
>>> 		struct fc_internal *i = to_fc_internal(shost->transportt);
>>> 		struct request_queue *q;
>>> 		char bsg_name[20];
>>>
>>> 		fc_host->rqst_q = NULL;
>>>
>>> 		if (!i->f->bsg_request)
>>> 			return -ENOTSUPP;
>>>
>>> 		snprintf(bsg_name, sizeof(bsg_name),
>>> 			 "fc_host%d", shost->host_no);
>>>
>>> 		q = bsg_setup_queue(dev, bsg_name, fc_bsg_dispatch, i->f->dd_bsg_size);
>>> 		if (IS_ERR(q)) {
>>> 			dev_err(dev,
>>> 				"fc_host%d: bsg interface failed to initialize - setup queue\n",
>>> 				shost->host_no);
>>> 			return PTR_ERR(q);
>>> 		}
>>> 		__scsi_init_queue(shost, q);
>>> ====>		blk_queue_rq_timed_out(q, fc_bsg_job_timeout);
>>> 		fc_host->rqst_q = q;
>>> 		return 0;
>>> 	}
>>>
>>> Guessing it has also to do with the series.
>>
>> It does, you'll want this one as well:
>>
>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=e930a73f3112c9c617303253fba4e754eb5d0491
>>
>> but that's not going to apply cleanly... Can you just try and run my
>> mq-conversions branch? That has everything, and it also has that
>> alloc failure fixed.
>>
>> git://git.kernel.dk/linux-block mq-conversions
>>
> 
> Yeah sure, can give that a spin.

JFYI, I also reordered the series to make it correct. You can apply
this one:

http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa

before the bsg patch, and it should be fine. Or just use the above branch,
of course.
Benjamin Block Oct. 22, 2018, 10:03 a.m. UTC | #9 
On Fri, Oct 19, 2018 at 09:50:53AM -0600, Jens Axboe wrote:
> On 10/19/18 9:01 AM, Benjamin Block wrote:
> > On Wed, Oct 17, 2018 at 10:01:16AM -0600, Jens Axboe wrote:
> >> On 10/17/18 9:55 AM, Benjamin Block wrote:
> >>> On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
> >>>> Requires a few changes to the FC transport class as well.
> >>>>
> >>>> Cc: Johannes Thumshirn <jthumshirn@suse.de>
> >>>> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
> >>>> Cc: linux-scsi@vger.kernel.org
> >>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> >>>> ---
> >>>>  block/bsg-lib.c                  | 102 +++++++++++++++++--------------
> >>>>  drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
> >>>>  2 files changed, 91 insertions(+), 72 deletions(-)
> >>>>
> 
> but that's not going to apply cleanly... Can you just try and run my
> mq-conversions branch? That has everything, and it also has that
> alloc failure fixed.
> 
> git://git.kernel.dk/linux-block mq-conversions
> 

Ok so, that gets past the stage where we initialize the queues. Simple
SCSI-I/O also seems to work, that is for example an INQUIRY(10), but
transport commands that get passed to the driver break. Tried to send
a FibreChannel GPN_FT (remote port discovery).

As the BSG interface goes. This is a bidirectional command, that has
both a buffer for the request and for the reply. AFAIR BSG will create a
struct request for each of them. Protocol is BSG_PROTOCOL_SCSI,
Subprotocol BSG_SUB_PROTOCOL_SCSI_TRANSPORT. The rest should be
transparent till we get into the driver.

First got this:

[  566.531100] BUG: sleeping function called from invalid context at mm/slab.h:421
[  566.531452] in_atomic(): 1, irqs_disabled(): 0, pid: 3104, name: bsg_api_test
[  566.531460] 1 lock held by bsg_api_test/3104:
[  566.531466]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
[  566.531498] Preemption disabled at:
[  566.531503] [<00000000008175d0>] __blk_mq_delay_run_hw_queue+0x50/0x218
[  566.531519] CPU: 3 PID: 3104 Comm: bsg_api_test Tainted: G        W         4.19.0-rc6-bb-next+ #1
[  566.531527] Hardware name: IBM 3906 M03 704 (LPAR)
[  566.531533] Call Trace:
[  566.531544] ([<00000000001167fa>] show_stack+0x8a/0xd8)
[  566.531555]  [<0000000000bcc6d2>] dump_stack+0x9a/0xd8
[  566.531565]  [<0000000000196410>] ___might_sleep+0x280/0x298
[  566.531576]  [<00000000003e528c>] __kmalloc+0xbc/0x560
[  566.531584]  [<000000000083186a>] bsg_map_buffer+0x5a/0xb0
[  566.531591]  [<0000000000831948>] bsg_queue_rq+0x88/0x118
[  566.531599]  [<000000000081ab56>] blk_mq_dispatch_rq_list+0x37e/0x670
[  566.531607]  [<000000000082050e>] blk_mq_do_dispatch_sched+0x11e/0x130
[  566.531615]  [<0000000000820dfe>] blk_mq_sched_dispatch_requests+0x156/0x1a0
[  566.531622]  [<0000000000817564>] __blk_mq_run_hw_queue+0x144/0x160
[  566.531630]  [<0000000000817614>] __blk_mq_delay_run_hw_queue+0x94/0x218
[  566.531638]  [<00000000008178b2>] blk_mq_run_hw_queue+0xda/0xf0
[  566.531645]  [<00000000008211d8>] blk_mq_sched_insert_request+0x1a8/0x1e8
[  566.531653]  [<0000000000811ee2>] blk_execute_rq_nowait+0x72/0x80
[  566.531660]  [<0000000000811f66>] blk_execute_rq+0x76/0xb8
[  566.531778]  [<0000000000830d0e>] bsg_ioctl+0x426/0x500
[  566.531787]  [<0000000000440cb4>] do_vfs_ioctl+0x68c/0x710
[  566.531794]  [<0000000000440dac>] ksys_ioctl+0x74/0xa0
[  566.531801]  [<0000000000440e0a>] sys_ioctl+0x32/0x40
[  566.531808]  [<0000000000bf1dd0>] system_call+0xd8/0x2d0
[  566.531815] 1 lock held by bsg_api_test/3104:
[  566.531821]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118

And then it dies completely:

[  566.531854] Unable to handle kernel pointer dereference in virtual kernel address space
[  566.531861] Failing address: 0000000000000000 TEID: 0000000000000483
[  566.531867] Fault in home space mode while using kernel ASCE.
[  566.531885] AS:00000000013ec007 R3:00000000effc8007 S:00000000effce000 P:000000000000013d
[  566.531927] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[  566.531938] Modules linked in: ...
[  566.532042] CPU: 3 PID: 3104 Comm: bsg_api_test Tainted: G        W         4.19.0-rc6-bb-next+ #1
[  566.532047] Hardware name: IBM 3906 M03 704 (LPAR)
[  566.532051] Krnl PSW : 00000000d16c67b2 00000000e4a74b5c (zfcp_fc_exec_bsg_job+0x116/0x2c0 [zfcp])
[  566.532071]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:3 PM:0 RI:0 EA:3
[  566.532077] Krnl GPRS: 0000000000001000 00000000bfb84178 0000000000000001 0000000080000004
[  566.532082]            0000000000001000 00000000a6625108 0000000000000000 0000000000000001
[  566.532086]            00000000bfb870e8 0000000000000000 00000000b6276930 00000000bb3a6fc8
[  566.532091]            00000000b6276800 000003ff80306228 000003ff802fc048 00000000a7313830
[  566.532104] Krnl Code: 000003ff802fc090: a7740004            brc     7,3ff802fc098
[  566.532104]            000003ff802fc094: a7f4002e            brc     15,3ff802fc0f0
[  566.532104]           #000003ff802fc098: e310a0300004        lg      %r1,48(%r10)
[  566.532104]           >000003ff802fc09e: e31090000024        stg     %r1,0(%r9)
[  566.532104]            000003ff802fc0a4: e310a0400004        lg      %r1,64(%r10)
[  566.532104]            000003ff802fc0aa: e3a090180024        stg     %r10,24(%r9)
[  566.532104]            000003ff802fc0b0: e31090080024        stg     %r1,8(%r9)
[  566.532104]            000003ff802fc0b6: 58108000            l       %r1,0(%r8)
[  566.532143] Call Trace:
[  566.532149] ([<00000000be459dd8>] 0xbe459dd8)
[  566.532160]  [<000003ff802bba00>] fc_bsg_dispatch+0x1d0/0x248 [scsi_transport_fc]
[  566.532164]  [<00000000008319a4>] bsg_queue_rq+0xe4/0x118
[  566.532169]  [<000000000081ab56>] blk_mq_dispatch_rq_list+0x37e/0x670
[  566.532174]  [<000000000082050e>] blk_mq_do_dispatch_sched+0x11e/0x130
[  566.532178]  [<0000000000820dfe>] blk_mq_sched_dispatch_requests+0x156/0x1a0
[  566.532183]  [<0000000000817564>] __blk_mq_run_hw_queue+0x144/0x160
[  566.532188]  [<0000000000817614>] __blk_mq_delay_run_hw_queue+0x94/0x218
[  566.532193]  [<00000000008178b2>] blk_mq_run_hw_queue+0xda/0xf0
[  566.532197]  [<00000000008211d8>] blk_mq_sched_insert_request+0x1a8/0x1e8
[  566.532202]  [<0000000000811ee2>] blk_execute_rq_nowait+0x72/0x80
[  566.532207]  [<0000000000811f66>] blk_execute_rq+0x76/0xb8
[  566.532211]  [<0000000000830d0e>] bsg_ioctl+0x426/0x500
[  566.532215]  [<0000000000440cb4>] do_vfs_ioctl+0x68c/0x710
[  566.532220]  [<0000000000440dac>] ksys_ioctl+0x74/0xa0
[  566.532224]  [<0000000000440e0a>] sys_ioctl+0x32/0x40
[  566.532228]  [<0000000000bf1dd0>] system_call+0xd8/0x2d0
[  566.532231] INFO: lockdep is turned off.
[  566.532234] Last Breaking-Event-Address:
[  566.532243]  [<000003ff802fc090>] zfcp_fc_exec_bsg_job+0x108/0x2c0 [zfcp]
[  566.532247]
[  566.532250] Kernel panic - not syncing: Fatal exception: panic_on_oops

This is the state of your branch from an hour ago or so.
Jens Axboe Oct. 22, 2018, 12:38 p.m. UTC | #10 
On 10/22/18 4:03 AM, Benjamin Block wrote:
> On Fri, Oct 19, 2018 at 09:50:53AM -0600, Jens Axboe wrote:
>> On 10/19/18 9:01 AM, Benjamin Block wrote:
>>> On Wed, Oct 17, 2018 at 10:01:16AM -0600, Jens Axboe wrote:
>>>> On 10/17/18 9:55 AM, Benjamin Block wrote:
>>>>> On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
>>>>>> Requires a few changes to the FC transport class as well.
>>>>>>
>>>>>> Cc: Johannes Thumshirn <jthumshirn@suse.de>
>>>>>> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
>>>>>> Cc: linux-scsi@vger.kernel.org
>>>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>>>> ---
>>>>>>  block/bsg-lib.c                  | 102 +++++++++++++++++--------------
>>>>>>  drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
>>>>>>  2 files changed, 91 insertions(+), 72 deletions(-)
>>>>>>
>>
>> but that's not going to apply cleanly... Can you just try and run my
>> mq-conversions branch? That has everything, and it also has that
>> alloc failure fixed.
>>
>> git://git.kernel.dk/linux-block mq-conversions
>>
> 
> Ok so, that gets past the stage where we initialize the queues. Simple
> SCSI-I/O also seems to work, that is for example an INQUIRY(10), but
> transport commands that get passed to the driver break. Tried to send
> a FibreChannel GPN_FT (remote port discovery).
> 
> As the BSG interface goes. This is a bidirectional command, that has
> both a buffer for the request and for the reply. AFAIR BSG will create a
> struct request for each of them. Protocol is BSG_PROTOCOL_SCSI,
> Subprotocol BSG_SUB_PROTOCOL_SCSI_TRANSPORT. The rest should be
> transparent till we get into the driver.
> 
> First got this:
> 
> [  566.531100] BUG: sleeping function called from invalid context at mm/slab.h:421
> [  566.531452] in_atomic(): 1, irqs_disabled(): 0, pid: 3104, name: bsg_api_test
> [  566.531460] 1 lock held by bsg_api_test/3104:
> [  566.531466]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
> [  566.531498] Preemption disabled at:
> [  566.531503] [<00000000008175d0>] __blk_mq_delay_run_hw_queue+0x50/0x218
> [  566.531519] CPU: 3 PID: 3104 Comm: bsg_api_test Tainted: G        W         4.19.0-rc6-bb-next+ #1
> [  566.531527] Hardware name: IBM 3906 M03 704 (LPAR)
> [  566.531533] Call Trace:
> [  566.531544] ([<00000000001167fa>] show_stack+0x8a/0xd8)
> [  566.531555]  [<0000000000bcc6d2>] dump_stack+0x9a/0xd8
> [  566.531565]  [<0000000000196410>] ___might_sleep+0x280/0x298
> [  566.531576]  [<00000000003e528c>] __kmalloc+0xbc/0x560
> [  566.531584]  [<000000000083186a>] bsg_map_buffer+0x5a/0xb0
> [  566.531591]  [<0000000000831948>] bsg_queue_rq+0x88/0x118
> [  566.531599]  [<000000000081ab56>] blk_mq_dispatch_rq_list+0x37e/0x670
> [  566.531607]  [<000000000082050e>] blk_mq_do_dispatch_sched+0x11e/0x130
> [  566.531615]  [<0000000000820dfe>] blk_mq_sched_dispatch_requests+0x156/0x1a0
> [  566.531622]  [<0000000000817564>] __blk_mq_run_hw_queue+0x144/0x160
> [  566.531630]  [<0000000000817614>] __blk_mq_delay_run_hw_queue+0x94/0x218
> [  566.531638]  [<00000000008178b2>] blk_mq_run_hw_queue+0xda/0xf0
> [  566.531645]  [<00000000008211d8>] blk_mq_sched_insert_request+0x1a8/0x1e8
> [  566.531653]  [<0000000000811ee2>] blk_execute_rq_nowait+0x72/0x80
> [  566.531660]  [<0000000000811f66>] blk_execute_rq+0x76/0xb8
> [  566.531778]  [<0000000000830d0e>] bsg_ioctl+0x426/0x500
> [  566.531787]  [<0000000000440cb4>] do_vfs_ioctl+0x68c/0x710
> [  566.531794]  [<0000000000440dac>] ksys_ioctl+0x74/0xa0
> [  566.531801]  [<0000000000440e0a>] sys_ioctl+0x32/0x40
> [  566.531808]  [<0000000000bf1dd0>] system_call+0xd8/0x2d0
> [  566.531815] 1 lock held by bsg_api_test/3104:
> [  566.531821]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
> 
> And then it dies completely:
> 
> [  566.531854] Unable to handle kernel pointer dereference in virtual kernel address space
> [  566.531861] Failing address: 0000000000000000 TEID: 0000000000000483
> [  566.531867] Fault in home space mode while using kernel ASCE.
> [  566.531885] AS:00000000013ec007 R3:00000000effc8007 S:00000000effce000 P:000000000000013d
> [  566.531927] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> [  566.531938] Modules linked in: ...
> [  566.532042] CPU: 3 PID: 3104 Comm: bsg_api_test Tainted: G        W         4.19.0-rc6-bb-next+ #1
> [  566.532047] Hardware name: IBM 3906 M03 704 (LPAR)
> [  566.532051] Krnl PSW : 00000000d16c67b2 00000000e4a74b5c (zfcp_fc_exec_bsg_job+0x116/0x2c0 [zfcp])
> [  566.532071]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:3 PM:0 RI:0 EA:3
> [  566.532077] Krnl GPRS: 0000000000001000 00000000bfb84178 0000000000000001 0000000080000004
> [  566.532082]            0000000000001000 00000000a6625108 0000000000000000 0000000000000001
> [  566.532086]            00000000bfb870e8 0000000000000000 00000000b6276930 00000000bb3a6fc8
> [  566.532091]            00000000b6276800 000003ff80306228 000003ff802fc048 00000000a7313830
> [  566.532104] Krnl Code: 000003ff802fc090: a7740004            brc     7,3ff802fc098
> [  566.532104]            000003ff802fc094: a7f4002e            brc     15,3ff802fc0f0
> [  566.532104]           #000003ff802fc098: e310a0300004        lg      %r1,48(%r10)
> [  566.532104]           >000003ff802fc09e: e31090000024        stg     %r1,0(%r9)
> [  566.532104]            000003ff802fc0a4: e310a0400004        lg      %r1,64(%r10)
> [  566.532104]            000003ff802fc0aa: e3a090180024        stg     %r10,24(%r9)
> [  566.532104]            000003ff802fc0b0: e31090080024        stg     %r1,8(%r9)
> [  566.532104]            000003ff802fc0b6: 58108000            l       %r1,0(%r8)
> [  566.532143] Call Trace:
> [  566.532149] ([<00000000be459dd8>] 0xbe459dd8)
> [  566.532160]  [<000003ff802bba00>] fc_bsg_dispatch+0x1d0/0x248 [scsi_transport_fc]
> [  566.532164]  [<00000000008319a4>] bsg_queue_rq+0xe4/0x118
> [  566.532169]  [<000000000081ab56>] blk_mq_dispatch_rq_list+0x37e/0x670
> [  566.532174]  [<000000000082050e>] blk_mq_do_dispatch_sched+0x11e/0x130
> [  566.532178]  [<0000000000820dfe>] blk_mq_sched_dispatch_requests+0x156/0x1a0
> [  566.532183]  [<0000000000817564>] __blk_mq_run_hw_queue+0x144/0x160
> [  566.532188]  [<0000000000817614>] __blk_mq_delay_run_hw_queue+0x94/0x218
> [  566.532193]  [<00000000008178b2>] blk_mq_run_hw_queue+0xda/0xf0
> [  566.532197]  [<00000000008211d8>] blk_mq_sched_insert_request+0x1a8/0x1e8
> [  566.532202]  [<0000000000811ee2>] blk_execute_rq_nowait+0x72/0x80
> [  566.532207]  [<0000000000811f66>] blk_execute_rq+0x76/0xb8
> [  566.532211]  [<0000000000830d0e>] bsg_ioctl+0x426/0x500
> [  566.532215]  [<0000000000440cb4>] do_vfs_ioctl+0x68c/0x710
> [  566.532220]  [<0000000000440dac>] ksys_ioctl+0x74/0xa0
> [  566.532224]  [<0000000000440e0a>] sys_ioctl+0x32/0x40
> [  566.532228]  [<0000000000bf1dd0>] system_call+0xd8/0x2d0
> [  566.532231] INFO: lockdep is turned off.
> [  566.532234] Last Breaking-Event-Address:
> [  566.532243]  [<000003ff802fc090>] zfcp_fc_exec_bsg_job+0x108/0x2c0 [zfcp]
> [  566.532247]
> [  566.532250] Kernel panic - not syncing: Fatal exception: panic_on_oops
> 
> This is the state of your branch from an hour ago or so.

The first one is an easy fix, not sure how I missed that. The other
one I have no idea, any chance you could try with this one:

http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c

which fixes the first one, and also corrects a wrong end_io call,
but I don't think that's the cause of the above.

If it crashes, can you figure out where in the source that is?
Basically just do

gdb vmlinux
l *zfcp_fc_exec_bsg_job+0x116

assuming that works fine on s390 :-)
Benjamin Block Oct. 22, 2018, 3:21 p.m. UTC | #11 
On Mon, Oct 22, 2018 at 06:38:36AM -0600, Jens Axboe wrote:
> On 10/22/18 4:03 AM, Benjamin Block wrote:
> > On Fri, Oct 19, 2018 at 09:50:53AM -0600, Jens Axboe wrote:
> >> On 10/19/18 9:01 AM, Benjamin Block wrote:
> >>> On Wed, Oct 17, 2018 at 10:01:16AM -0600, Jens Axboe wrote:
> >>>> On 10/17/18 9:55 AM, Benjamin Block wrote:
> >>>>> On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
> >>>>>> Requires a few changes to the FC transport class as well.
> >>>>>>
> >>>>>> Cc: Johannes Thumshirn <jthumshirn@suse.de>
> >>>>>> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
> >>>>>> Cc: linux-scsi@vger.kernel.org
> >>>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> >>>>>> ---
> >>>>>>  block/bsg-lib.c                  | 102 +++++++++++++++++--------------
> >>>>>>  drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
> >>>>>>  2 files changed, 91 insertions(+), 72 deletions(-)
> >>>>>>
> >>
> >> but that's not going to apply cleanly... Can you just try and run my
> >> mq-conversions branch? That has everything, and it also has that
> >> alloc failure fixed.
> >>
> >> git://git.kernel.dk/linux-block mq-conversions
> >>
> > 
> > Ok so, that gets past the stage where we initialize the queues. Simple
> > SCSI-I/O also seems to work, that is for example an INQUIRY(10), but
> > transport commands that get passed to the driver break. Tried to send
> > a FibreChannel GPN_FT (remote port discovery).
> > 
> > As the BSG interface goes. This is a bidirectional command, that has
> > both a buffer for the request and for the reply. AFAIR BSG will create a
> > struct request for each of them. Protocol is BSG_PROTOCOL_SCSI,
> > Subprotocol BSG_SUB_PROTOCOL_SCSI_TRANSPORT. The rest should be
> > transparent till we get into the driver.
> > 
> > First got this:
> > 
> > [  566.531100] BUG: sleeping function called from invalid context at mm/slab.h:421
> > [  566.531452] in_atomic(): 1, irqs_disabled(): 0, pid: 3104, name: bsg_api_test
> > [  566.531460] 1 lock held by bsg_api_test/3104:
> > [  566.531466]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
> > [  566.531498] Preemption disabled at:
> > [  566.531503] [<00000000008175d0>] __blk_mq_delay_run_hw_queue+0x50/0x218
> > [  566.531519] CPU: 3 PID: 3104 Comm: bsg_api_test Tainted: G        W         4.19.0-rc6-bb-next+ #1
> > [  566.531527] Hardware name: IBM 3906 M03 704 (LPAR)
> > [  566.531533] Call Trace:
> > [  566.531544] ([<00000000001167fa>] show_stack+0x8a/0xd8)
> > [  566.531555]  [<0000000000bcc6d2>] dump_stack+0x9a/0xd8
> > [  566.531565]  [<0000000000196410>] ___might_sleep+0x280/0x298
> > [  566.531576]  [<00000000003e528c>] __kmalloc+0xbc/0x560
> > [  566.531584]  [<000000000083186a>] bsg_map_buffer+0x5a/0xb0
> > [  566.531591]  [<0000000000831948>] bsg_queue_rq+0x88/0x118
> > [  566.531599]  [<000000000081ab56>] blk_mq_dispatch_rq_list+0x37e/0x670
> > [  566.531607]  [<000000000082050e>] blk_mq_do_dispatch_sched+0x11e/0x130
> > [  566.531615]  [<0000000000820dfe>] blk_mq_sched_dispatch_requests+0x156/0x1a0
> > [  566.531622]  [<0000000000817564>] __blk_mq_run_hw_queue+0x144/0x160
> > [  566.531630]  [<0000000000817614>] __blk_mq_delay_run_hw_queue+0x94/0x218
> > [  566.531638]  [<00000000008178b2>] blk_mq_run_hw_queue+0xda/0xf0
> > [  566.531645]  [<00000000008211d8>] blk_mq_sched_insert_request+0x1a8/0x1e8
> > [  566.531653]  [<0000000000811ee2>] blk_execute_rq_nowait+0x72/0x80
> > [  566.531660]  [<0000000000811f66>] blk_execute_rq+0x76/0xb8
> > [  566.531778]  [<0000000000830d0e>] bsg_ioctl+0x426/0x500
> > [  566.531787]  [<0000000000440cb4>] do_vfs_ioctl+0x68c/0x710
> > [  566.531794]  [<0000000000440dac>] ksys_ioctl+0x74/0xa0
> > [  566.531801]  [<0000000000440e0a>] sys_ioctl+0x32/0x40
> > [  566.531808]  [<0000000000bf1dd0>] system_call+0xd8/0x2d0
> > [  566.531815] 1 lock held by bsg_api_test/3104:
> > [  566.531821]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
> > 
> > And then it dies completely:
> > 
> > [  566.531854] Unable to handle kernel pointer dereference in virtual kernel address space
> > [  566.531861] Failing address: 0000000000000000 TEID: 0000000000000483
> > [  566.531867] Fault in home space mode while using kernel ASCE.
> > [  566.531885] AS:00000000013ec007 R3:00000000effc8007 S:00000000effce000 P:000000000000013d
> > [  566.531927] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> > [  566.531938] Modules linked in: ...
> > [  566.532042] CPU: 3 PID: 3104 Comm: bsg_api_test Tainted: G        W         4.19.0-rc6-bb-next+ #1
> > [  566.532047] Hardware name: IBM 3906 M03 704 (LPAR)
> > [  566.532051] Krnl PSW : 00000000d16c67b2 00000000e4a74b5c (zfcp_fc_exec_bsg_job+0x116/0x2c0 [zfcp])
> > [  566.532071]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:3 PM:0 RI:0 EA:3
> > [  566.532077] Krnl GPRS: 0000000000001000 00000000bfb84178 0000000000000001 0000000080000004
> > [  566.532082]            0000000000001000 00000000a6625108 0000000000000000 0000000000000001
> > [  566.532086]            00000000bfb870e8 0000000000000000 00000000b6276930 00000000bb3a6fc8
> > [  566.532091]            00000000b6276800 000003ff80306228 000003ff802fc048 00000000a7313830
> > [  566.532104] Krnl Code: 000003ff802fc090: a7740004            brc     7,3ff802fc098
> > [  566.532104]            000003ff802fc094: a7f4002e            brc     15,3ff802fc0f0
> > [  566.532104]           #000003ff802fc098: e310a0300004        lg      %r1,48(%r10)
> > [  566.532104]           >000003ff802fc09e: e31090000024        stg     %r1,0(%r9)
> > [  566.532104]            000003ff802fc0a4: e310a0400004        lg      %r1,64(%r10)
> > [  566.532104]            000003ff802fc0aa: e3a090180024        stg     %r10,24(%r9)
> > [  566.532104]            000003ff802fc0b0: e31090080024        stg     %r1,8(%r9)
> > [  566.532104]            000003ff802fc0b6: 58108000            l       %r1,0(%r8)
> > [  566.532143] Call Trace:
> > [  566.532149] ([<00000000be459dd8>] 0xbe459dd8)
> > [  566.532160]  [<000003ff802bba00>] fc_bsg_dispatch+0x1d0/0x248 [scsi_transport_fc]
> > [  566.532164]  [<00000000008319a4>] bsg_queue_rq+0xe4/0x118
> > [  566.532169]  [<000000000081ab56>] blk_mq_dispatch_rq_list+0x37e/0x670
> > [  566.532174]  [<000000000082050e>] blk_mq_do_dispatch_sched+0x11e/0x130
> > [  566.532178]  [<0000000000820dfe>] blk_mq_sched_dispatch_requests+0x156/0x1a0
> > [  566.532183]  [<0000000000817564>] __blk_mq_run_hw_queue+0x144/0x160
> > [  566.532188]  [<0000000000817614>] __blk_mq_delay_run_hw_queue+0x94/0x218
> > [  566.532193]  [<00000000008178b2>] blk_mq_run_hw_queue+0xda/0xf0
> > [  566.532197]  [<00000000008211d8>] blk_mq_sched_insert_request+0x1a8/0x1e8
> > [  566.532202]  [<0000000000811ee2>] blk_execute_rq_nowait+0x72/0x80
> > [  566.532207]  [<0000000000811f66>] blk_execute_rq+0x76/0xb8
> > [  566.532211]  [<0000000000830d0e>] bsg_ioctl+0x426/0x500
> > [  566.532215]  [<0000000000440cb4>] do_vfs_ioctl+0x68c/0x710
> > [  566.532220]  [<0000000000440dac>] ksys_ioctl+0x74/0xa0
> > [  566.532224]  [<0000000000440e0a>] sys_ioctl+0x32/0x40
> > [  566.532228]  [<0000000000bf1dd0>] system_call+0xd8/0x2d0
> > [  566.532231] INFO: lockdep is turned off.
> > [  566.532234] Last Breaking-Event-Address:
> > [  566.532243]  [<000003ff802fc090>] zfcp_fc_exec_bsg_job+0x108/0x2c0 [zfcp]
> > [  566.532247]
> > [  566.532250] Kernel panic - not syncing: Fatal exception: panic_on_oops
> > 
> > This is the state of your branch from an hour ago or so.
> 
> The first one is an easy fix, not sure how I missed that. The other
> one I have no idea, any chance you could try with this one:
> 
> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
> 
> which fixes the first one, and also corrects a wrong end_io call,
> but I don't think that's the cause of the above.
> 
> If it crashes, can you figure out where in the source that is?
> Basically just do
> 
> gdb vmlinux
> l *zfcp_fc_exec_bsg_job+0x116
> 
> assuming that works fine on s390 :-)
> 

Sry, I am a bit split between several things I should do right now
(nothing new, right? :) ), I'll continue on this tomorrow.
Jens Axboe Oct. 22, 2018, 9:58 p.m. UTC | #12 
On 10/22/18 9:21 AM, Benjamin Block wrote:
> On Mon, Oct 22, 2018 at 06:38:36AM -0600, Jens Axboe wrote:
>> On 10/22/18 4:03 AM, Benjamin Block wrote:
>>> On Fri, Oct 19, 2018 at 09:50:53AM -0600, Jens Axboe wrote:
>>>> On 10/19/18 9:01 AM, Benjamin Block wrote:
>>>>> On Wed, Oct 17, 2018 at 10:01:16AM -0600, Jens Axboe wrote:
>>>>>> On 10/17/18 9:55 AM, Benjamin Block wrote:
>>>>>>> On Tue, Oct 16, 2018 at 08:43:01AM -0600, Jens Axboe wrote:
>>>>>>>> Requires a few changes to the FC transport class as well.
>>>>>>>>
>>>>>>>> Cc: Johannes Thumshirn <jthumshirn@suse.de>
>>>>>>>> Cc: Benjamin Block <bblock@linux.vnet.ibm.com>
>>>>>>>> Cc: linux-scsi@vger.kernel.org
>>>>>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>>>>>> ---
>>>>>>>>  block/bsg-lib.c                  | 102 +++++++++++++++++--------------
>>>>>>>>  drivers/scsi/scsi_transport_fc.c |  61 ++++++++++--------
>>>>>>>>  2 files changed, 91 insertions(+), 72 deletions(-)
>>>>>>>>
>>>>
>>>> but that's not going to apply cleanly... Can you just try and run my
>>>> mq-conversions branch? That has everything, and it also has that
>>>> alloc failure fixed.
>>>>
>>>> git://git.kernel.dk/linux-block mq-conversions
>>>>
>>>
>>> Ok so, that gets past the stage where we initialize the queues. Simple
>>> SCSI-I/O also seems to work, that is for example an INQUIRY(10), but
>>> transport commands that get passed to the driver break. Tried to send
>>> a FibreChannel GPN_FT (remote port discovery).
>>>
>>> As the BSG interface goes. This is a bidirectional command, that has
>>> both a buffer for the request and for the reply. AFAIR BSG will create a
>>> struct request for each of them. Protocol is BSG_PROTOCOL_SCSI,
>>> Subprotocol BSG_SUB_PROTOCOL_SCSI_TRANSPORT. The rest should be
>>> transparent till we get into the driver.
>>>
>>> First got this:
>>>
>>> [  566.531100] BUG: sleeping function called from invalid context at mm/slab.h:421
>>> [  566.531452] in_atomic(): 1, irqs_disabled(): 0, pid: 3104, name: bsg_api_test
>>> [  566.531460] 1 lock held by bsg_api_test/3104:
>>> [  566.531466]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
>>> [  566.531498] Preemption disabled at:
>>> [  566.531503] [<00000000008175d0>] __blk_mq_delay_run_hw_queue+0x50/0x218
>>> [  566.531519] CPU: 3 PID: 3104 Comm: bsg_api_test Tainted: G        W         4.19.0-rc6-bb-next+ #1
>>> [  566.531527] Hardware name: IBM 3906 M03 704 (LPAR)
>>> [  566.531533] Call Trace:
>>> [  566.531544] ([<00000000001167fa>] show_stack+0x8a/0xd8)
>>> [  566.531555]  [<0000000000bcc6d2>] dump_stack+0x9a/0xd8
>>> [  566.531565]  [<0000000000196410>] ___might_sleep+0x280/0x298
>>> [  566.531576]  [<00000000003e528c>] __kmalloc+0xbc/0x560
>>> [  566.531584]  [<000000000083186a>] bsg_map_buffer+0x5a/0xb0
>>> [  566.531591]  [<0000000000831948>] bsg_queue_rq+0x88/0x118
>>> [  566.531599]  [<000000000081ab56>] blk_mq_dispatch_rq_list+0x37e/0x670
>>> [  566.531607]  [<000000000082050e>] blk_mq_do_dispatch_sched+0x11e/0x130
>>> [  566.531615]  [<0000000000820dfe>] blk_mq_sched_dispatch_requests+0x156/0x1a0
>>> [  566.531622]  [<0000000000817564>] __blk_mq_run_hw_queue+0x144/0x160
>>> [  566.531630]  [<0000000000817614>] __blk_mq_delay_run_hw_queue+0x94/0x218
>>> [  566.531638]  [<00000000008178b2>] blk_mq_run_hw_queue+0xda/0xf0
>>> [  566.531645]  [<00000000008211d8>] blk_mq_sched_insert_request+0x1a8/0x1e8
>>> [  566.531653]  [<0000000000811ee2>] blk_execute_rq_nowait+0x72/0x80
>>> [  566.531660]  [<0000000000811f66>] blk_execute_rq+0x76/0xb8
>>> [  566.531778]  [<0000000000830d0e>] bsg_ioctl+0x426/0x500
>>> [  566.531787]  [<0000000000440cb4>] do_vfs_ioctl+0x68c/0x710
>>> [  566.531794]  [<0000000000440dac>] ksys_ioctl+0x74/0xa0
>>> [  566.531801]  [<0000000000440e0a>] sys_ioctl+0x32/0x40
>>> [  566.531808]  [<0000000000bf1dd0>] system_call+0xd8/0x2d0
>>> [  566.531815] 1 lock held by bsg_api_test/3104:
>>> [  566.531821]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
>>>
>>> And then it dies completely:
>>>
>>> [  566.531854] Unable to handle kernel pointer dereference in virtual kernel address space
>>> [  566.531861] Failing address: 0000000000000000 TEID: 0000000000000483
>>> [  566.531867] Fault in home space mode while using kernel ASCE.
>>> [  566.531885] AS:00000000013ec007 R3:00000000effc8007 S:00000000effce000 P:000000000000013d
>>> [  566.531927] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
>>> [  566.531938] Modules linked in: ...
>>> [  566.532042] CPU: 3 PID: 3104 Comm: bsg_api_test Tainted: G        W         4.19.0-rc6-bb-next+ #1
>>> [  566.532047] Hardware name: IBM 3906 M03 704 (LPAR)
>>> [  566.532051] Krnl PSW : 00000000d16c67b2 00000000e4a74b5c (zfcp_fc_exec_bsg_job+0x116/0x2c0 [zfcp])
>>> [  566.532071]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:3 PM:0 RI:0 EA:3
>>> [  566.532077] Krnl GPRS: 0000000000001000 00000000bfb84178 0000000000000001 0000000080000004
>>> [  566.532082]            0000000000001000 00000000a6625108 0000000000000000 0000000000000001
>>> [  566.532086]            00000000bfb870e8 0000000000000000 00000000b6276930 00000000bb3a6fc8
>>> [  566.532091]            00000000b6276800 000003ff80306228 000003ff802fc048 00000000a7313830
>>> [  566.532104] Krnl Code: 000003ff802fc090: a7740004            brc     7,3ff802fc098
>>> [  566.532104]            000003ff802fc094: a7f4002e            brc     15,3ff802fc0f0
>>> [  566.532104]           #000003ff802fc098: e310a0300004        lg      %r1,48(%r10)
>>> [  566.532104]           >000003ff802fc09e: e31090000024        stg     %r1,0(%r9)
>>> [  566.532104]            000003ff802fc0a4: e310a0400004        lg      %r1,64(%r10)
>>> [  566.532104]            000003ff802fc0aa: e3a090180024        stg     %r10,24(%r9)
>>> [  566.532104]            000003ff802fc0b0: e31090080024        stg     %r1,8(%r9)
>>> [  566.532104]            000003ff802fc0b6: 58108000            l       %r1,0(%r8)
>>> [  566.532143] Call Trace:
>>> [  566.532149] ([<00000000be459dd8>] 0xbe459dd8)
>>> [  566.532160]  [<000003ff802bba00>] fc_bsg_dispatch+0x1d0/0x248 [scsi_transport_fc]
>>> [  566.532164]  [<00000000008319a4>] bsg_queue_rq+0xe4/0x118
>>> [  566.532169]  [<000000000081ab56>] blk_mq_dispatch_rq_list+0x37e/0x670
>>> [  566.532174]  [<000000000082050e>] blk_mq_do_dispatch_sched+0x11e/0x130
>>> [  566.532178]  [<0000000000820dfe>] blk_mq_sched_dispatch_requests+0x156/0x1a0
>>> [  566.532183]  [<0000000000817564>] __blk_mq_run_hw_queue+0x144/0x160
>>> [  566.532188]  [<0000000000817614>] __blk_mq_delay_run_hw_queue+0x94/0x218
>>> [  566.532193]  [<00000000008178b2>] blk_mq_run_hw_queue+0xda/0xf0
>>> [  566.532197]  [<00000000008211d8>] blk_mq_sched_insert_request+0x1a8/0x1e8
>>> [  566.532202]  [<0000000000811ee2>] blk_execute_rq_nowait+0x72/0x80
>>> [  566.532207]  [<0000000000811f66>] blk_execute_rq+0x76/0xb8
>>> [  566.532211]  [<0000000000830d0e>] bsg_ioctl+0x426/0x500
>>> [  566.532215]  [<0000000000440cb4>] do_vfs_ioctl+0x68c/0x710
>>> [  566.532220]  [<0000000000440dac>] ksys_ioctl+0x74/0xa0
>>> [  566.532224]  [<0000000000440e0a>] sys_ioctl+0x32/0x40
>>> [  566.532228]  [<0000000000bf1dd0>] system_call+0xd8/0x2d0
>>> [  566.532231] INFO: lockdep is turned off.
>>> [  566.532234] Last Breaking-Event-Address:
>>> [  566.532243]  [<000003ff802fc090>] zfcp_fc_exec_bsg_job+0x108/0x2c0 [zfcp]
>>> [  566.532247]
>>> [  566.532250] Kernel panic - not syncing: Fatal exception: panic_on_oops
>>>
>>> This is the state of your branch from an hour ago or so.
>>
>> The first one is an easy fix, not sure how I missed that. The other
>> one I have no idea, any chance you could try with this one:
>>
>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
>>
>> which fixes the first one, and also corrects a wrong end_io call,
>> but I don't think that's the cause of the above.
>>
>> If it crashes, can you figure out where in the source that is?
>> Basically just do
>>
>> gdb vmlinux
>> l *zfcp_fc_exec_bsg_job+0x116
>>
>> assuming that works fine on s390 :-)
>>
> 
> Sry, I am a bit split between several things I should do right now
> (nothing new, right? :) ), I'll continue on this tomorrow.

I'm just happy that you're able to test, since this is something I
cannot test on my own. FWIW, I did run the latest and did bidirectional
commands with scsi_debug through the BSG interface, and it works for me.
But the private bsg setup queues are a bit different, so... Let me know
how I can help you.

For the record, the old bsg interface that dropped the queue lock (and
enabled IRQs) from the request_fn was buggy, that's not a valid thing to
do. I'm hopeful that once we work through the kinks of this issue, we'll
be better off for it in the long run.
Benjamin Block Oct. 23, 2018, 5:40 p.m. UTC | #13 
On Mon, Oct 22, 2018 at 06:38:36AM -0600, Jens Axboe wrote:
> On 10/22/18 4:03 AM, Benjamin Block wrote:
> > On Fri, Oct 19, 2018 at 09:50:53AM -0600, Jens Axboe wrote:
> > 
> > Ok so, that gets past the stage where we initialize the queues. Simple
> > SCSI-I/O also seems to work, that is for example an INQUIRY(10), but
> > transport commands that get passed to the driver break. Tried to send
> > a FibreChannel GPN_FT (remote port discovery).
> > 
> > As the BSG interface goes. This is a bidirectional command, that has
> > both a buffer for the request and for the reply. AFAIR BSG will create a
> > struct request for each of them. Protocol is BSG_PROTOCOL_SCSI,
> > Subprotocol BSG_SUB_PROTOCOL_SCSI_TRANSPORT. The rest should be
> > transparent till we get into the driver.
> > 
> > First got this:
> > 
> > [  566.531100] BUG: sleeping function called from invalid context at mm/slab.h:421
> > [  566.531452] in_atomic(): 1, irqs_disabled(): 0, pid: 3104, name: bsg_api_test
> > [  566.531460] 1 lock held by bsg_api_test/3104:
> > [  566.531466]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
> > [  566.531498] Preemption disabled at:
> > [  566.531503] [<00000000008175d0>] __blk_mq_delay_run_hw_queue+0x50/0x218
> > [  566.531519] CPU: 3 PID: 3104 Comm: bsg_api_test Tainted: G        W         4.19.0-rc6-bb-next+ #1
> > [  566.531527] Hardware name: IBM 3906 M03 704 (LPAR)
> > [  566.531533] Call Trace:
> > [  566.531544] ([<00000000001167fa>] show_stack+0x8a/0xd8)
> > [  566.531555]  [<0000000000bcc6d2>] dump_stack+0x9a/0xd8
> > [  566.531565]  [<0000000000196410>] ___might_sleep+0x280/0x298
> > [  566.531576]  [<00000000003e528c>] __kmalloc+0xbc/0x560
> > [  566.531584]  [<000000000083186a>] bsg_map_buffer+0x5a/0xb0
> > [  566.531591]  [<0000000000831948>] bsg_queue_rq+0x88/0x118
> > [  566.531599]  [<000000000081ab56>] blk_mq_dispatch_rq_list+0x37e/0x670
> > [  566.531607]  [<000000000082050e>] blk_mq_do_dispatch_sched+0x11e/0x130
> > [  566.531615]  [<0000000000820dfe>] blk_mq_sched_dispatch_requests+0x156/0x1a0
> > [  566.531622]  [<0000000000817564>] __blk_mq_run_hw_queue+0x144/0x160
> > [  566.531630]  [<0000000000817614>] __blk_mq_delay_run_hw_queue+0x94/0x218
> > [  566.531638]  [<00000000008178b2>] blk_mq_run_hw_queue+0xda/0xf0
> > [  566.531645]  [<00000000008211d8>] blk_mq_sched_insert_request+0x1a8/0x1e8
> > [  566.531653]  [<0000000000811ee2>] blk_execute_rq_nowait+0x72/0x80
> > [  566.531660]  [<0000000000811f66>] blk_execute_rq+0x76/0xb8
> > [  566.531778]  [<0000000000830d0e>] bsg_ioctl+0x426/0x500
> > [  566.531787]  [<0000000000440cb4>] do_vfs_ioctl+0x68c/0x710
> > [  566.531794]  [<0000000000440dac>] ksys_ioctl+0x74/0xa0
> > [  566.531801]  [<0000000000440e0a>] sys_ioctl+0x32/0x40
> > [  566.531808]  [<0000000000bf1dd0>] system_call+0xd8/0x2d0
> > [  566.531815] 1 lock held by bsg_api_test/3104:
> > [  566.531821]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
> > 
> 
> The first one is an easy fix, not sure how I missed that. The other
> one I have no idea, any chance you could try with this one:
> 
> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
> 
> which fixes the first one, and also corrects a wrong end_io call,
> but I don't think that's the cause of the above.
> 
> If it crashes, can you figure out where in the source that is?
> Basically just do
> 
> gdb vmlinux
> l *zfcp_fc_exec_bsg_job+0x116
> 
> assuming that works fine on s390 :-)
> 

So I tried 4.19.0 with only the two patches from you:
http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c

This fixed the first warning from before, as you suggested, but it still
crash like this:

[ ] Unable to handle kernel pointer dereference in virtual kernel address space
[ ] Failing address: 0000000000000000 TEID: 0000000000000483
[ ] Fault in home space mode while using kernel ASCE.
[ ] AS:00000000025f0007 R3:00000000dffb8007 S:00000000dffbf000 P:000000000000013d
[ ] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[ ] Modules linked in: ....
[ ] CPU: 2 PID: 609 Comm: kworker/2:1H Kdump: loaded Tainted: G        W         4.19.0-bb-next+ #1
[ ] Hardware name: IBM 3906 M03 704 (LPAR)
[ ] Workqueue: kblockd blk_mq_run_work_fn
[ ] Krnl PSW : 0704e00180000000 000003ff806a6b40 (zfcp_fc_exec_bsg_job+0x1c0/0x440 [zfcp])
[ ]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
[ ] Krnl GPRS: 0000000000000000 0000000083e0f3c0 0000000000000000 0000030000000000
[ ]            0000030000000000 000003ff806a6b3a 00000000a86b5948 00000000a86b5988
[ ]            0000000083e0f3f0 0000000000000000 00000000a86b5938 00000000984aee80
[ ]            00000000a86b5800 000003ff806ba950 000003ff806a6b3a 0000000098a5ed88
[ ] Krnl Code: 000003ff806a6b30: b9040029            lgr     %r2,%r9
               000003ff806a6b34: c0e5ffff8b7e        brasl   %r14,3ff80698230
              #000003ff806a6b3a: e310f0a00004        lg      %r1,160(%r15)
              >000003ff806a6b40: e31090000024        stg     %r1,0(%r9)
               000003ff806a6b46: 4120a040            la      %r2,64(%r10)
               000003ff806a6b4a: c0e5ffff8ae7        brasl   %r14,3ff80698118
               000003ff806a6b50: e310a0400004        lg      %r1,64(%r10)
               000003ff806a6b56: e310f0a00024        stg     %r1,160(%r15)
[ ] Call Trace:
[ ] ([<000003ff806a6aa2>] zfcp_fc_exec_bsg_job+0x122/0x440 [zfcp])
[ ]  [<000003ff8061e928>] fc_bsg_dispatch+0x310/0x398 [scsi_transport_fc]
[ ]  [<0000000000d2995a>] bsg_queue_rq+0x15a/0x198
[ ]  [<0000000000d03566>] blk_mq_dispatch_rq_list+0x966/0xf90
[ ]  [<0000000000d0e37a>] blk_mq_sched_dispatch_requests+0x3fa/0x410
[ ]  [<0000000000cfc230>] __blk_mq_run_hw_queue+0x218/0x248
[ ]  [<00000000001cb124>] process_one_work+0x8c4/0xe90
[ ]  [<00000000001cbe58>] worker_thread+0x768/0xbb0
[ ]  [<00000000001dc67a>] kthread+0x22a/0x248
[ ]  [<000000000137b812>] kernel_thread_starter+0x6/0xc
[ ]  [<000000000137b80c>] kernel_thread_starter+0x0/0xc
[ ] INFO: lockdep is turned off.
[ ] Last Breaking-Event-Address:
[ ]  [<00000000005d9b08>] __asan_store8+0x98/0xa0
[ ]
[ ] Kernel panic - not syncing: Fatal exception: panic_on_oops

zfcp_fc_exec_bsg_job+0x1c0 is here:

    int zfcp_fc_exec_bsg_job(struct 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;
            struct fc_rport *rport = fc_bsg_to_rport(job);

            shost = rport ? rport_to_shost(rport) : fc_bsg_to_shost(job);
            adapter = (struct zfcp_adapter *)shost->hostdata[0];

            if (!(atomic_read(&adapter->status) & ZFCP_STATUS_COMMON_OPEN))
                    return -EINVAL;

==>         ct_els->req = job->request_payload.sg_list;
            ct_els->resp = job->reply_payload.sg_list;
            ct_els->handler_data = job;

            switch (bsg_request->msgcode) {
            case FC_BSG_RPT_ELS:
            case FC_BSG_HST_ELS_NOLOGIN:
                    return zfcp_fc_exec_els_job(job, adapter);
            case FC_BSG_RPT_CT:
            case FC_BSG_HST_CT:
                    return zfcp_fc_exec_ct_job(job, adapter);
            default:
                    return -EINVAL;
            }
    }

I took a dump to find out how struct bsg_job looks like when it crashes
(register clobbering isn't as bad here and I verified that job->dev is valid).

    crash> print *((struct bsg_job *)0x00000000a86b5938)
    $5 = {
      dev = 0x9af10358,
      kref = {
        refcount = {
          refs = {
            counter = 0x1
          }
        }
      },
      timeout = 0x384,
      request = 0x83e0f3f0,
      reply = 0x9559d500,
      request_len = 0x14,
      reply_len = 0x0,
      request_payload = {
        payload_len = 0x14,
        sg_cnt = 0x1,
        sg_list = 0x83e0f3c0
      },
      reply_payload = {
        payload_len = 0x1000,
        sg_cnt = 0x1,
        sg_list = 0x83e0f1e0
      },
      result = 0x0,
      reply_payload_rcv_len = 0x0,
==>   dd_data = 0x0
    }

This is where it breaks, job->dd_data is 0x0, so when it wants to write
to it, it fails.

So We set

    struct fc_function_template zfcp_transport_functions = {
            ....
            .dd_bsg_size = sizeof(struct zfcp_fsf_ct_els);
            ....
    }

We get to zfcp_fc_exec_bsg_job() via fc_bsg_host_dispatch(). Previously
this was initially allocated in bsg_setup_queue() with:

            q->cmd_size = sizeof(struct bsg_job) + dd_job_size;

And then in bsg_initialize_rq() with:

            job->dd_data = job + 1;

Sry, I haven't check the rest of your patch yet. But just to
double-check, I ran some tests against 4.18.15, and there stuff still
works, so it didn't break w/o me noticing in between (always possible
:)).
Jens Axboe Oct. 23, 2018, 6:07 p.m. UTC | #14 
On 10/23/18 11:40 AM, Benjamin Block wrote:
> On Mon, Oct 22, 2018 at 06:38:36AM -0600, Jens Axboe wrote:
>> On 10/22/18 4:03 AM, Benjamin Block wrote:
>>> On Fri, Oct 19, 2018 at 09:50:53AM -0600, Jens Axboe wrote:
>>>
>>> Ok so, that gets past the stage where we initialize the queues. Simple
>>> SCSI-I/O also seems to work, that is for example an INQUIRY(10), but
>>> transport commands that get passed to the driver break. Tried to send
>>> a FibreChannel GPN_FT (remote port discovery).
>>>
>>> As the BSG interface goes. This is a bidirectional command, that has
>>> both a buffer for the request and for the reply. AFAIR BSG will create a
>>> struct request for each of them. Protocol is BSG_PROTOCOL_SCSI,
>>> Subprotocol BSG_SUB_PROTOCOL_SCSI_TRANSPORT. The rest should be
>>> transparent till we get into the driver.
>>>
>>> First got this:
>>>
>>> [  566.531100] BUG: sleeping function called from invalid context at mm/slab.h:421
>>> [  566.531452] in_atomic(): 1, irqs_disabled(): 0, pid: 3104, name: bsg_api_test
>>> [  566.531460] 1 lock held by bsg_api_test/3104:
>>> [  566.531466]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
>>> [  566.531498] Preemption disabled at:
>>> [  566.531503] [<00000000008175d0>] __blk_mq_delay_run_hw_queue+0x50/0x218
>>> [  566.531519] CPU: 3 PID: 3104 Comm: bsg_api_test Tainted: G        W         4.19.0-rc6-bb-next+ #1
>>> [  566.531527] Hardware name: IBM 3906 M03 704 (LPAR)
>>> [  566.531533] Call Trace:
>>> [  566.531544] ([<00000000001167fa>] show_stack+0x8a/0xd8)
>>> [  566.531555]  [<0000000000bcc6d2>] dump_stack+0x9a/0xd8
>>> [  566.531565]  [<0000000000196410>] ___might_sleep+0x280/0x298
>>> [  566.531576]  [<00000000003e528c>] __kmalloc+0xbc/0x560
>>> [  566.531584]  [<000000000083186a>] bsg_map_buffer+0x5a/0xb0
>>> [  566.531591]  [<0000000000831948>] bsg_queue_rq+0x88/0x118
>>> [  566.531599]  [<000000000081ab56>] blk_mq_dispatch_rq_list+0x37e/0x670
>>> [  566.531607]  [<000000000082050e>] blk_mq_do_dispatch_sched+0x11e/0x130
>>> [  566.531615]  [<0000000000820dfe>] blk_mq_sched_dispatch_requests+0x156/0x1a0
>>> [  566.531622]  [<0000000000817564>] __blk_mq_run_hw_queue+0x144/0x160
>>> [  566.531630]  [<0000000000817614>] __blk_mq_delay_run_hw_queue+0x94/0x218
>>> [  566.531638]  [<00000000008178b2>] blk_mq_run_hw_queue+0xda/0xf0
>>> [  566.531645]  [<00000000008211d8>] blk_mq_sched_insert_request+0x1a8/0x1e8
>>> [  566.531653]  [<0000000000811ee2>] blk_execute_rq_nowait+0x72/0x80
>>> [  566.531660]  [<0000000000811f66>] blk_execute_rq+0x76/0xb8
>>> [  566.531778]  [<0000000000830d0e>] bsg_ioctl+0x426/0x500
>>> [  566.531787]  [<0000000000440cb4>] do_vfs_ioctl+0x68c/0x710
>>> [  566.531794]  [<0000000000440dac>] ksys_ioctl+0x74/0xa0
>>> [  566.531801]  [<0000000000440e0a>] sys_ioctl+0x32/0x40
>>> [  566.531808]  [<0000000000bf1dd0>] system_call+0xd8/0x2d0
>>> [  566.531815] 1 lock held by bsg_api_test/3104:
>>> [  566.531821]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
>>>
>>
>> The first one is an easy fix, not sure how I missed that. The other
>> one I have no idea, any chance you could try with this one:
>>
>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
>>
>> which fixes the first one, and also corrects a wrong end_io call,
>> but I don't think that's the cause of the above.
>>
>> If it crashes, can you figure out where in the source that is?
>> Basically just do
>>
>> gdb vmlinux
>> l *zfcp_fc_exec_bsg_job+0x116
>>
>> assuming that works fine on s390 :-)
>>
> 
> So I tried 4.19.0 with only the two patches from you:
> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
> 
> This fixed the first warning from before, as you suggested, but it still
> crash like this:
> 
> [ ] Unable to handle kernel pointer dereference in virtual kernel address space
> [ ] Failing address: 0000000000000000 TEID: 0000000000000483
> [ ] Fault in home space mode while using kernel ASCE.
> [ ] AS:00000000025f0007 R3:00000000dffb8007 S:00000000dffbf000 P:000000000000013d
> [ ] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> [ ] Modules linked in: ....
> [ ] CPU: 2 PID: 609 Comm: kworker/2:1H Kdump: loaded Tainted: G        W         4.19.0-bb-next+ #1
> [ ] Hardware name: IBM 3906 M03 704 (LPAR)
> [ ] Workqueue: kblockd blk_mq_run_work_fn
> [ ] Krnl PSW : 0704e00180000000 000003ff806a6b40 (zfcp_fc_exec_bsg_job+0x1c0/0x440 [zfcp])
> [ ]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
> [ ] Krnl GPRS: 0000000000000000 0000000083e0f3c0 0000000000000000 0000030000000000
> [ ]            0000030000000000 000003ff806a6b3a 00000000a86b5948 00000000a86b5988
> [ ]            0000000083e0f3f0 0000000000000000 00000000a86b5938 00000000984aee80
> [ ]            00000000a86b5800 000003ff806ba950 000003ff806a6b3a 0000000098a5ed88
> [ ] Krnl Code: 000003ff806a6b30: b9040029            lgr     %r2,%r9
>                000003ff806a6b34: c0e5ffff8b7e        brasl   %r14,3ff80698230
>               #000003ff806a6b3a: e310f0a00004        lg      %r1,160(%r15)
>               >000003ff806a6b40: e31090000024        stg     %r1,0(%r9)
>                000003ff806a6b46: 4120a040            la      %r2,64(%r10)
>                000003ff806a6b4a: c0e5ffff8ae7        brasl   %r14,3ff80698118
>                000003ff806a6b50: e310a0400004        lg      %r1,64(%r10)
>                000003ff806a6b56: e310f0a00024        stg     %r1,160(%r15)
> [ ] Call Trace:
> [ ] ([<000003ff806a6aa2>] zfcp_fc_exec_bsg_job+0x122/0x440 [zfcp])
> [ ]  [<000003ff8061e928>] fc_bsg_dispatch+0x310/0x398 [scsi_transport_fc]
> [ ]  [<0000000000d2995a>] bsg_queue_rq+0x15a/0x198
> [ ]  [<0000000000d03566>] blk_mq_dispatch_rq_list+0x966/0xf90
> [ ]  [<0000000000d0e37a>] blk_mq_sched_dispatch_requests+0x3fa/0x410
> [ ]  [<0000000000cfc230>] __blk_mq_run_hw_queue+0x218/0x248
> [ ]  [<00000000001cb124>] process_one_work+0x8c4/0xe90
> [ ]  [<00000000001cbe58>] worker_thread+0x768/0xbb0
> [ ]  [<00000000001dc67a>] kthread+0x22a/0x248
> [ ]  [<000000000137b812>] kernel_thread_starter+0x6/0xc
> [ ]  [<000000000137b80c>] kernel_thread_starter+0x0/0xc
> [ ] INFO: lockdep is turned off.
> [ ] Last Breaking-Event-Address:
> [ ]  [<00000000005d9b08>] __asan_store8+0x98/0xa0
> [ ]
> [ ] Kernel panic - not syncing: Fatal exception: panic_on_oops
> 
> zfcp_fc_exec_bsg_job+0x1c0 is here:
> 
>     int zfcp_fc_exec_bsg_job(struct 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;
>             struct fc_rport *rport = fc_bsg_to_rport(job);
> 
>             shost = rport ? rport_to_shost(rport) : fc_bsg_to_shost(job);
>             adapter = (struct zfcp_adapter *)shost->hostdata[0];
> 
>             if (!(atomic_read(&adapter->status) & ZFCP_STATUS_COMMON_OPEN))
>                     return -EINVAL;
> 
> ==>         ct_els->req = job->request_payload.sg_list;
>             ct_els->resp = job->reply_payload.sg_list;
>             ct_els->handler_data = job;
> 
>             switch (bsg_request->msgcode) {
>             case FC_BSG_RPT_ELS:
>             case FC_BSG_HST_ELS_NOLOGIN:
>                     return zfcp_fc_exec_els_job(job, adapter);
>             case FC_BSG_RPT_CT:
>             case FC_BSG_HST_CT:
>                     return zfcp_fc_exec_ct_job(job, adapter);
>             default:
>                     return -EINVAL;
>             }
>     }
> 
> I took a dump to find out how struct bsg_job looks like when it crashes
> (register clobbering isn't as bad here and I verified that job->dev is valid).
> 
>     crash> print *((struct bsg_job *)0x00000000a86b5938)
>     $5 = {
>       dev = 0x9af10358,
>       kref = {
>         refcount = {
>           refs = {
>             counter = 0x1
>           }
>         }
>       },
>       timeout = 0x384,
>       request = 0x83e0f3f0,
>       reply = 0x9559d500,
>       request_len = 0x14,
>       reply_len = 0x0,
>       request_payload = {
>         payload_len = 0x14,
>         sg_cnt = 0x1,
>         sg_list = 0x83e0f3c0
>       },
>       reply_payload = {
>         payload_len = 0x1000,
>         sg_cnt = 0x1,
>         sg_list = 0x83e0f1e0
>       },
>       result = 0x0,
>       reply_payload_rcv_len = 0x0,
> ==>   dd_data = 0x0
>     }
> 
> This is where it breaks, job->dd_data is 0x0, so when it wants to write
> to it, it fails.
> 
> So We set
> 
>     struct fc_function_template zfcp_transport_functions = {
>             ....
>             .dd_bsg_size = sizeof(struct zfcp_fsf_ct_els);
>             ....
>     }
> 
> We get to zfcp_fc_exec_bsg_job() via fc_bsg_host_dispatch(). Previously
> this was initially allocated in bsg_setup_queue() with:
> 
>             q->cmd_size = sizeof(struct bsg_job) + dd_job_size;
> 
> And then in bsg_initialize_rq() with:
> 
>             job->dd_data = job + 1;
> 
> Sry, I haven't check the rest of your patch yet. But just to
> double-check, I ran some tests against 4.18.15, and there stuff still
> works, so it didn't break w/o me noticing in between (always possible

Doh, yeah it's obvious now. Try this one on top. I missed it due to the
similar naming, I'll kill off q->initalize_rq_fn as well.


diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index 58c9886394d8..198cf42b74a3 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -293,6 +293,7 @@ static const struct blk_mq_ops bsg_mq_ops = {
 	.queue_rq	= bsg_queue_rq,
 	.init_request	= bsg_init_rq,
 	.exit_request	= bsg_exit_rq,
+	.initialize_rq_fn = bsg_initialize_rq,
 	.complete	= bsg_complete,
 };
 
@@ -329,7 +330,6 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
 		goto out_queue;
 	}
 
-	q->initialize_rq_fn = bsg_initialize_rq;
 	q->queuedata = dev;
 	q->bsg_job_fn = job_fn;
 	blk_queue_flag_set(QUEUE_FLAG_BIDI, q);
Christoph Hellwig Oct. 24, 2018, 11:19 a.m. UTC | #15 
On Mon, Oct 22, 2018 at 03:23:30AM -0600, Jens Axboe wrote:
> JFYI, I also reordered the series to make it correct. You can apply
> this one:
> 
> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
> 
> before the bsg patch, and it should be fine. Or just use the above branch,
> of course.

Hell no on that one.  The behavior of having methods right on the
request_queue which can be changed any time is something we absolutely
must not introduce into blk-mq.

Just add pass a timeout hander to bsg_register_queue which is called
from the bsg ->timeout handler is a much better way to sort our your
problem.  It can also easily be turned into an independent prep patch.
Jens Axboe Oct. 24, 2018, 11:30 a.m. UTC | #16 
On 10/24/18 5:19 AM, Christoph Hellwig wrote:
> On Mon, Oct 22, 2018 at 03:23:30AM -0600, Jens Axboe wrote:
>> JFYI, I also reordered the series to make it correct. You can apply
>> this one:
>>
>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
>>
>> before the bsg patch, and it should be fine. Or just use the above branch,
>> of course.
> 
> Hell no on that one.  The behavior of having methods right on the
> request_queue which can be changed any time is something we absolutely
> must not introduce into blk-mq.

I agree it's not the prettiest, but it's not like it's something
that is changed at runtime.

> Just add pass a timeout hander to bsg_register_queue which is called
> from the bsg ->timeout handler is a much better way to sort our your
> problem.  It can also easily be turned into an independent prep patch.

Good idea, that is cleaner.
Jens Axboe Oct. 24, 2018, 11:52 a.m. UTC | #17 
On 10/24/18 5:30 AM, Jens Axboe wrote:
> On 10/24/18 5:19 AM, Christoph Hellwig wrote:
>> On Mon, Oct 22, 2018 at 03:23:30AM -0600, Jens Axboe wrote:
>>> JFYI, I also reordered the series to make it correct. You can apply
>>> this one:
>>>
>>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
>>>
>>> before the bsg patch, and it should be fine. Or just use the above branch,
>>> of course.
>>
>> Hell no on that one.  The behavior of having methods right on the
>> request_queue which can be changed any time is something we absolutely
>> must not introduce into blk-mq.
> 
> I agree it's not the prettiest, but it's not like it's something
> that is changed at runtime.
> 
>> Just add pass a timeout hander to bsg_register_queue which is called
>> from the bsg ->timeout handler is a much better way to sort our your
>> problem.  It can also easily be turned into an independent prep patch.
> 
> Good idea, that is cleaner.

Except that STILL doesn't work with mq_ops being a constant, I'd have
to allocate it.
Jens Axboe Oct. 24, 2018, 12:02 p.m. UTC | #18 
On 10/24/18 5:52 AM, Jens Axboe wrote:
> On 10/24/18 5:30 AM, Jens Axboe wrote:
>> On 10/24/18 5:19 AM, Christoph Hellwig wrote:
>>> On Mon, Oct 22, 2018 at 03:23:30AM -0600, Jens Axboe wrote:
>>>> JFYI, I also reordered the series to make it correct. You can apply
>>>> this one:
>>>>
>>>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
>>>>
>>>> before the bsg patch, and it should be fine. Or just use the above branch,
>>>> of course.
>>>
>>> Hell no on that one.  The behavior of having methods right on the
>>> request_queue which can be changed any time is something we absolutely
>>> must not introduce into blk-mq.
>>
>> I agree it's not the prettiest, but it's not like it's something
>> that is changed at runtime.
>>
>>> Just add pass a timeout hander to bsg_register_queue which is called
>>> from the bsg ->timeout handler is a much better way to sort our your
>>> problem.  It can also easily be turned into an independent prep patch.
>>
>> Good idea, that is cleaner.
> 
> Except that STILL doesn't work with mq_ops being a constant, I'd have
> to allocate it.

Which obviously won't work. I don't see a good way out of this, since
I can't store the private timeout handler anywhere. Open to suggestions,
but until something better comes up, I'm keeping q->timeout and
removing the API to set it. bsg-lib can just manually set it in the
queue.
Jens Axboe Oct. 24, 2018, 12:23 p.m. UTC | #19 
On 10/24/18 6:02 AM, Jens Axboe wrote:
> On 10/24/18 5:52 AM, Jens Axboe wrote:
>> On 10/24/18 5:30 AM, Jens Axboe wrote:
>>> On 10/24/18 5:19 AM, Christoph Hellwig wrote:
>>>> On Mon, Oct 22, 2018 at 03:23:30AM -0600, Jens Axboe wrote:
>>>>> JFYI, I also reordered the series to make it correct. You can apply
>>>>> this one:
>>>>>
>>>>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
>>>>>
>>>>> before the bsg patch, and it should be fine. Or just use the above branch,
>>>>> of course.
>>>>
>>>> Hell no on that one.  The behavior of having methods right on the
>>>> request_queue which can be changed any time is something we absolutely
>>>> must not introduce into blk-mq.
>>>
>>> I agree it's not the prettiest, but it's not like it's something
>>> that is changed at runtime.
>>>
>>>> Just add pass a timeout hander to bsg_register_queue which is called
>>>> from the bsg ->timeout handler is a much better way to sort our your
>>>> problem.  It can also easily be turned into an independent prep patch.
>>>
>>> Good idea, that is cleaner.
>>
>> Except that STILL doesn't work with mq_ops being a constant, I'd have
>> to allocate it.
> 
> Which obviously won't work. I don't see a good way out of this, since
> I can't store the private timeout handler anywhere. Open to suggestions,
> but until something better comes up, I'm keeping q->timeout and
> removing the API to set it. bsg-lib can just manually set it in the
> queue.

Pushed that out - it's now ->bsg_job_timeout_fn, and nobody sets it but
bsg when it initializes the queue. bsg sets up a default timeout
handler, and calls ->bsg_job_timeout_fn, if defined.  Not that that is
any different than from before, but at least it's obvious what it's for
now.
Jens Axboe Oct. 25, 2018, 11:12 p.m. UTC | #20 
On 10/23/18 12:07 PM, Jens Axboe wrote:
> On 10/23/18 11:40 AM, Benjamin Block wrote:
>> On Mon, Oct 22, 2018 at 06:38:36AM -0600, Jens Axboe wrote:
>>> On 10/22/18 4:03 AM, Benjamin Block wrote:
>>>> On Fri, Oct 19, 2018 at 09:50:53AM -0600, Jens Axboe wrote:
>>>>
>>>> Ok so, that gets past the stage where we initialize the queues. Simple
>>>> SCSI-I/O also seems to work, that is for example an INQUIRY(10), but
>>>> transport commands that get passed to the driver break. Tried to send
>>>> a FibreChannel GPN_FT (remote port discovery).
>>>>
>>>> As the BSG interface goes. This is a bidirectional command, that has
>>>> both a buffer for the request and for the reply. AFAIR BSG will create a
>>>> struct request for each of them. Protocol is BSG_PROTOCOL_SCSI,
>>>> Subprotocol BSG_SUB_PROTOCOL_SCSI_TRANSPORT. The rest should be
>>>> transparent till we get into the driver.
>>>>
>>>> First got this:
>>>>
>>>> [  566.531100] BUG: sleeping function called from invalid context at mm/slab.h:421
>>>> [  566.531452] in_atomic(): 1, irqs_disabled(): 0, pid: 3104, name: bsg_api_test
>>>> [  566.531460] 1 lock held by bsg_api_test/3104:
>>>> [  566.531466]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
>>>> [  566.531498] Preemption disabled at:
>>>> [  566.531503] [<00000000008175d0>] __blk_mq_delay_run_hw_queue+0x50/0x218
>>>> [  566.531519] CPU: 3 PID: 3104 Comm: bsg_api_test Tainted: G        W         4.19.0-rc6-bb-next+ #1
>>>> [  566.531527] Hardware name: IBM 3906 M03 704 (LPAR)
>>>> [  566.531533] Call Trace:
>>>> [  566.531544] ([<00000000001167fa>] show_stack+0x8a/0xd8)
>>>> [  566.531555]  [<0000000000bcc6d2>] dump_stack+0x9a/0xd8
>>>> [  566.531565]  [<0000000000196410>] ___might_sleep+0x280/0x298
>>>> [  566.531576]  [<00000000003e528c>] __kmalloc+0xbc/0x560
>>>> [  566.531584]  [<000000000083186a>] bsg_map_buffer+0x5a/0xb0
>>>> [  566.531591]  [<0000000000831948>] bsg_queue_rq+0x88/0x118
>>>> [  566.531599]  [<000000000081ab56>] blk_mq_dispatch_rq_list+0x37e/0x670
>>>> [  566.531607]  [<000000000082050e>] blk_mq_do_dispatch_sched+0x11e/0x130
>>>> [  566.531615]  [<0000000000820dfe>] blk_mq_sched_dispatch_requests+0x156/0x1a0
>>>> [  566.531622]  [<0000000000817564>] __blk_mq_run_hw_queue+0x144/0x160
>>>> [  566.531630]  [<0000000000817614>] __blk_mq_delay_run_hw_queue+0x94/0x218
>>>> [  566.531638]  [<00000000008178b2>] blk_mq_run_hw_queue+0xda/0xf0
>>>> [  566.531645]  [<00000000008211d8>] blk_mq_sched_insert_request+0x1a8/0x1e8
>>>> [  566.531653]  [<0000000000811ee2>] blk_execute_rq_nowait+0x72/0x80
>>>> [  566.531660]  [<0000000000811f66>] blk_execute_rq+0x76/0xb8
>>>> [  566.531778]  [<0000000000830d0e>] bsg_ioctl+0x426/0x500
>>>> [  566.531787]  [<0000000000440cb4>] do_vfs_ioctl+0x68c/0x710
>>>> [  566.531794]  [<0000000000440dac>] ksys_ioctl+0x74/0xa0
>>>> [  566.531801]  [<0000000000440e0a>] sys_ioctl+0x32/0x40
>>>> [  566.531808]  [<0000000000bf1dd0>] system_call+0xd8/0x2d0
>>>> [  566.531815] 1 lock held by bsg_api_test/3104:
>>>> [  566.531821]  #0: 00000000cb4b58e8 (rcu_read_lock){....}, at: hctx_lock+0x30/0x118
>>>>
>>>
>>> The first one is an easy fix, not sure how I missed that. The other
>>> one I have no idea, any chance you could try with this one:
>>>
>>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
>>>
>>> which fixes the first one, and also corrects a wrong end_io call,
>>> but I don't think that's the cause of the above.
>>>
>>> If it crashes, can you figure out where in the source that is?
>>> Basically just do
>>>
>>> gdb vmlinux
>>> l *zfcp_fc_exec_bsg_job+0x116
>>>
>>> assuming that works fine on s390 :-)
>>>
>>
>> So I tried 4.19.0 with only the two patches from you:
>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
>>
>> This fixed the first warning from before, as you suggested, but it still
>> crash like this:
>>
>> [ ] Unable to handle kernel pointer dereference in virtual kernel address space
>> [ ] Failing address: 0000000000000000 TEID: 0000000000000483
>> [ ] Fault in home space mode while using kernel ASCE.
>> [ ] AS:00000000025f0007 R3:00000000dffb8007 S:00000000dffbf000 P:000000000000013d
>> [ ] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
>> [ ] Modules linked in: ....
>> [ ] CPU: 2 PID: 609 Comm: kworker/2:1H Kdump: loaded Tainted: G        W         4.19.0-bb-next+ #1
>> [ ] Hardware name: IBM 3906 M03 704 (LPAR)
>> [ ] Workqueue: kblockd blk_mq_run_work_fn
>> [ ] Krnl PSW : 0704e00180000000 000003ff806a6b40 (zfcp_fc_exec_bsg_job+0x1c0/0x440 [zfcp])
>> [ ]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
>> [ ] Krnl GPRS: 0000000000000000 0000000083e0f3c0 0000000000000000 0000030000000000
>> [ ]            0000030000000000 000003ff806a6b3a 00000000a86b5948 00000000a86b5988
>> [ ]            0000000083e0f3f0 0000000000000000 00000000a86b5938 00000000984aee80
>> [ ]            00000000a86b5800 000003ff806ba950 000003ff806a6b3a 0000000098a5ed88
>> [ ] Krnl Code: 000003ff806a6b30: b9040029            lgr     %r2,%r9
>>                000003ff806a6b34: c0e5ffff8b7e        brasl   %r14,3ff80698230
>>               #000003ff806a6b3a: e310f0a00004        lg      %r1,160(%r15)
>>               >000003ff806a6b40: e31090000024        stg     %r1,0(%r9)
>>                000003ff806a6b46: 4120a040            la      %r2,64(%r10)
>>                000003ff806a6b4a: c0e5ffff8ae7        brasl   %r14,3ff80698118
>>                000003ff806a6b50: e310a0400004        lg      %r1,64(%r10)
>>                000003ff806a6b56: e310f0a00024        stg     %r1,160(%r15)
>> [ ] Call Trace:
>> [ ] ([<000003ff806a6aa2>] zfcp_fc_exec_bsg_job+0x122/0x440 [zfcp])
>> [ ]  [<000003ff8061e928>] fc_bsg_dispatch+0x310/0x398 [scsi_transport_fc]
>> [ ]  [<0000000000d2995a>] bsg_queue_rq+0x15a/0x198
>> [ ]  [<0000000000d03566>] blk_mq_dispatch_rq_list+0x966/0xf90
>> [ ]  [<0000000000d0e37a>] blk_mq_sched_dispatch_requests+0x3fa/0x410
>> [ ]  [<0000000000cfc230>] __blk_mq_run_hw_queue+0x218/0x248
>> [ ]  [<00000000001cb124>] process_one_work+0x8c4/0xe90
>> [ ]  [<00000000001cbe58>] worker_thread+0x768/0xbb0
>> [ ]  [<00000000001dc67a>] kthread+0x22a/0x248
>> [ ]  [<000000000137b812>] kernel_thread_starter+0x6/0xc
>> [ ]  [<000000000137b80c>] kernel_thread_starter+0x0/0xc
>> [ ] INFO: lockdep is turned off.
>> [ ] Last Breaking-Event-Address:
>> [ ]  [<00000000005d9b08>] __asan_store8+0x98/0xa0
>> [ ]
>> [ ] Kernel panic - not syncing: Fatal exception: panic_on_oops
>>
>> zfcp_fc_exec_bsg_job+0x1c0 is here:
>>
>>     int zfcp_fc_exec_bsg_job(struct 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;
>>             struct fc_rport *rport = fc_bsg_to_rport(job);
>>
>>             shost = rport ? rport_to_shost(rport) : fc_bsg_to_shost(job);
>>             adapter = (struct zfcp_adapter *)shost->hostdata[0];
>>
>>             if (!(atomic_read(&adapter->status) & ZFCP_STATUS_COMMON_OPEN))
>>                     return -EINVAL;
>>
>> ==>         ct_els->req = job->request_payload.sg_list;
>>             ct_els->resp = job->reply_payload.sg_list;
>>             ct_els->handler_data = job;
>>
>>             switch (bsg_request->msgcode) {
>>             case FC_BSG_RPT_ELS:
>>             case FC_BSG_HST_ELS_NOLOGIN:
>>                     return zfcp_fc_exec_els_job(job, adapter);
>>             case FC_BSG_RPT_CT:
>>             case FC_BSG_HST_CT:
>>                     return zfcp_fc_exec_ct_job(job, adapter);
>>             default:
>>                     return -EINVAL;
>>             }
>>     }
>>
>> I took a dump to find out how struct bsg_job looks like when it crashes
>> (register clobbering isn't as bad here and I verified that job->dev is valid).
>>
>>     crash> print *((struct bsg_job *)0x00000000a86b5938)
>>     $5 = {
>>       dev = 0x9af10358,
>>       kref = {
>>         refcount = {
>>           refs = {
>>             counter = 0x1
>>           }
>>         }
>>       },
>>       timeout = 0x384,
>>       request = 0x83e0f3f0,
>>       reply = 0x9559d500,
>>       request_len = 0x14,
>>       reply_len = 0x0,
>>       request_payload = {
>>         payload_len = 0x14,
>>         sg_cnt = 0x1,
>>         sg_list = 0x83e0f3c0
>>       },
>>       reply_payload = {
>>         payload_len = 0x1000,
>>         sg_cnt = 0x1,
>>         sg_list = 0x83e0f1e0
>>       },
>>       result = 0x0,
>>       reply_payload_rcv_len = 0x0,
>> ==>   dd_data = 0x0
>>     }
>>
>> This is where it breaks, job->dd_data is 0x0, so when it wants to write
>> to it, it fails.
>>
>> So We set
>>
>>     struct fc_function_template zfcp_transport_functions = {
>>             ....
>>             .dd_bsg_size = sizeof(struct zfcp_fsf_ct_els);
>>             ....
>>     }
>>
>> We get to zfcp_fc_exec_bsg_job() via fc_bsg_host_dispatch(). Previously
>> this was initially allocated in bsg_setup_queue() with:
>>
>>             q->cmd_size = sizeof(struct bsg_job) + dd_job_size;
>>
>> And then in bsg_initialize_rq() with:
>>
>>             job->dd_data = job + 1;
>>
>> Sry, I haven't check the rest of your patch yet. But just to
>> double-check, I ran some tests against 4.18.15, and there stuff still
>> works, so it didn't break w/o me noticing in between (always possible
> 
> Doh, yeah it's obvious now. Try this one on top. I missed it due to the
> similar naming, I'll kill off q->initalize_rq_fn as well.
> 
> 
> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
> index 58c9886394d8..198cf42b74a3 100644
> --- a/block/bsg-lib.c
> +++ b/block/bsg-lib.c
> @@ -293,6 +293,7 @@ static const struct blk_mq_ops bsg_mq_ops = {
>  	.queue_rq	= bsg_queue_rq,
>  	.init_request	= bsg_init_rq,
>  	.exit_request	= bsg_exit_rq,
> +	.initialize_rq_fn = bsg_initialize_rq,
>  	.complete	= bsg_complete,
>  };
>  
> @@ -329,7 +330,6 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
>  		goto out_queue;
>  	}
>  
> -	q->initialize_rq_fn = bsg_initialize_rq;
>  	q->queuedata = dev;
>  	q->bsg_job_fn = job_fn;
>  	blk_queue_flag_set(QUEUE_FLAG_BIDI, q);

Did you get a chance to test with the above?
Benjamin Block Oct. 26, 2018, 4:06 p.m. UTC | #21 
On Thu, Oct 25, 2018 at 05:12:59PM -0600, Jens Axboe wrote:
> On 10/23/18 12:07 PM, Jens Axboe wrote:
> > On 10/23/18 11:40 AM, Benjamin Block wrote:
> >>
> >> So I tried 4.19.0 with only the two patches from you:
> >> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
> >> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
> >>
> >> This fixed the first warning from before, as you suggested, but it still
> >> crash like this:
> >>
> >> [ ] Unable to handle kernel pointer dereference in virtual kernel address space
> >> [ ] Failing address: 0000000000000000 TEID: 0000000000000483
> >> [ ] Fault in home space mode while using kernel ASCE.
> >> [ ] AS:00000000025f0007 R3:00000000dffb8007 S:00000000dffbf000 P:000000000000013d
> >> [ ] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> >> [ ] Modules linked in: ....
> >> [ ] CPU: 2 PID: 609 Comm: kworker/2:1H Kdump: loaded Tainted: G        W         4.19.0-bb-next+ #1
> >> [ ] Hardware name: IBM 3906 M03 704 (LPAR)
> >> [ ] Workqueue: kblockd blk_mq_run_work_fn
> >> [ ] Krnl PSW : 0704e00180000000 000003ff806a6b40 (zfcp_fc_exec_bsg_job+0x1c0/0x440 [zfcp])
> >> [ ]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
> >> [ ] Krnl GPRS: 0000000000000000 0000000083e0f3c0 0000000000000000 0000030000000000
> >> [ ]            0000030000000000 000003ff806a6b3a 00000000a86b5948 00000000a86b5988
> >> [ ]            0000000083e0f3f0 0000000000000000 00000000a86b5938 00000000984aee80
> >> [ ]            00000000a86b5800 000003ff806ba950 000003ff806a6b3a 0000000098a5ed88
> >> [ ] Krnl Code: 000003ff806a6b30: b9040029            lgr     %r2,%r9
> >>                000003ff806a6b34: c0e5ffff8b7e        brasl   %r14,3ff80698230
> >>               #000003ff806a6b3a: e310f0a00004        lg      %r1,160(%r15)
> >>               >000003ff806a6b40: e31090000024        stg     %r1,0(%r9)
> >>                000003ff806a6b46: 4120a040            la      %r2,64(%r10)
> >>                000003ff806a6b4a: c0e5ffff8ae7        brasl   %r14,3ff80698118
> >>                000003ff806a6b50: e310a0400004        lg      %r1,64(%r10)
> >>                000003ff806a6b56: e310f0a00024        stg     %r1,160(%r15)
> >> [ ] Call Trace:
> >> [ ] ([<000003ff806a6aa2>] zfcp_fc_exec_bsg_job+0x122/0x440 [zfcp])
> >> [ ]  [<000003ff8061e928>] fc_bsg_dispatch+0x310/0x398 [scsi_transport_fc]
> >> [ ]  [<0000000000d2995a>] bsg_queue_rq+0x15a/0x198
> >> [ ]  [<0000000000d03566>] blk_mq_dispatch_rq_list+0x966/0xf90
> >> [ ]  [<0000000000d0e37a>] blk_mq_sched_dispatch_requests+0x3fa/0x410
> >> [ ]  [<0000000000cfc230>] __blk_mq_run_hw_queue+0x218/0x248
> >> [ ]  [<00000000001cb124>] process_one_work+0x8c4/0xe90
> >> [ ]  [<00000000001cbe58>] worker_thread+0x768/0xbb0
> >> [ ]  [<00000000001dc67a>] kthread+0x22a/0x248
> >> [ ]  [<000000000137b812>] kernel_thread_starter+0x6/0xc
> >> [ ]  [<000000000137b80c>] kernel_thread_starter+0x0/0xc
> >> [ ] INFO: lockdep is turned off.
> >> [ ] Last Breaking-Event-Address:
> >> [ ]  [<00000000005d9b08>] __asan_store8+0x98/0xa0
> >> [ ]
> >> [ ] Kernel panic - not syncing: Fatal exception: panic_on_oops
> >>
> >> zfcp_fc_exec_bsg_job+0x1c0 is here:
> >>
> >>     int zfcp_fc_exec_bsg_job(struct 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;
> >>             struct fc_rport *rport = fc_bsg_to_rport(job);
> >>
> >>             shost = rport ? rport_to_shost(rport) : fc_bsg_to_shost(job);
> >>             adapter = (struct zfcp_adapter *)shost->hostdata[0];
> >>
> >>             if (!(atomic_read(&adapter->status) & ZFCP_STATUS_COMMON_OPEN))
> >>                     return -EINVAL;
> >>
> >> ==>         ct_els->req = job->request_payload.sg_list;
> >>             ct_els->resp = job->reply_payload.sg_list;
> >>             ct_els->handler_data = job;
> >>
> >>             switch (bsg_request->msgcode) {
> >>             case FC_BSG_RPT_ELS:
> >>             case FC_BSG_HST_ELS_NOLOGIN:
> >>                     return zfcp_fc_exec_els_job(job, adapter);
> >>             case FC_BSG_RPT_CT:
> >>             case FC_BSG_HST_CT:
> >>                     return zfcp_fc_exec_ct_job(job, adapter);
> >>             default:
> >>                     return -EINVAL;
> >>             }
> >>     }
> >>
> >> I took a dump to find out how struct bsg_job looks like when it crashes
> >> (register clobbering isn't as bad here and I verified that job->dev is valid).
> >>
> >>     crash> print *((struct bsg_job *)0x00000000a86b5938)
> >>     $5 = {
> >>       dev = 0x9af10358,
> >>       kref = {
> >>         refcount = {
> >>           refs = {
> >>             counter = 0x1
> >>           }
> >>         }
> >>       },
> >>       timeout = 0x384,
> >>       request = 0x83e0f3f0,
> >>       reply = 0x9559d500,
> >>       request_len = 0x14,
> >>       reply_len = 0x0,
> >>       request_payload = {
> >>         payload_len = 0x14,
> >>         sg_cnt = 0x1,
> >>         sg_list = 0x83e0f3c0
> >>       },
> >>       reply_payload = {
> >>         payload_len = 0x1000,
> >>         sg_cnt = 0x1,
> >>         sg_list = 0x83e0f1e0
> >>       },
> >>       result = 0x0,
> >>       reply_payload_rcv_len = 0x0,
> >> ==>   dd_data = 0x0
> >>     }
> >>
> >> This is where it breaks, job->dd_data is 0x0, so when it wants to write
> >> to it, it fails.
> >>
> >> So We set
> >>
> >>     struct fc_function_template zfcp_transport_functions = {
> >>             ....
> >>             .dd_bsg_size = sizeof(struct zfcp_fsf_ct_els);
> >>             ....
> >>     }
> >>
> >> We get to zfcp_fc_exec_bsg_job() via fc_bsg_host_dispatch(). Previously
> >> this was initially allocated in bsg_setup_queue() with:
> >>
> >>             q->cmd_size = sizeof(struct bsg_job) + dd_job_size;
> >>
> >> And then in bsg_initialize_rq() with:
> >>
> >>             job->dd_data = job + 1;
> >>
> >> Sry, I haven't check the rest of your patch yet. But just to
> >> double-check, I ran some tests against 4.18.15, and there stuff still
> >> works, so it didn't break w/o me noticing in between (always possible
> > 
> > Doh, yeah it's obvious now. Try this one on top. I missed it due to the
> > similar naming, I'll kill off q->initalize_rq_fn as well.
> > 
> > 
> > diff --git a/block/bsg-lib.c b/block/bsg-lib.c
> > index 58c9886394d8..198cf42b74a3 100644
> > --- a/block/bsg-lib.c
> > +++ b/block/bsg-lib.c
> > @@ -293,6 +293,7 @@ static const struct blk_mq_ops bsg_mq_ops = {
> >  	.queue_rq	= bsg_queue_rq,
> >  	.init_request	= bsg_init_rq,
> >  	.exit_request	= bsg_exit_rq,
> > +	.initialize_rq_fn = bsg_initialize_rq,
> >  	.complete	= bsg_complete,
> >  };
> >  
> > @@ -329,7 +330,6 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
> >  		goto out_queue;
> >  	}
> >  
> > -	q->initialize_rq_fn = bsg_initialize_rq;
> >  	q->queuedata = dev;
> >  	q->bsg_job_fn = job_fn;
> >  	blk_queue_flag_set(QUEUE_FLAG_BIDI, q);
> 
> Did you get a chance to test with the above?
> 

Yes, found time today. This works now. I extended my small test-suite to
test both sending commands against FC-Host and FC-Rport bsg-devices.
Both seem to work with zFCP as far as my tests go (but I only have like
3 commands in that "test-suite", so its not extensive).
Jens Axboe Oct. 26, 2018, 4:08 p.m. UTC | #22 
On 10/26/18 10:06 AM, Benjamin Block wrote:
> On Thu, Oct 25, 2018 at 05:12:59PM -0600, Jens Axboe wrote:
>> On 10/23/18 12:07 PM, Jens Axboe wrote:
>>> On 10/23/18 11:40 AM, Benjamin Block wrote:
>>>>
>>>> So I tried 4.19.0 with only the two patches from you:
>>>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
>>>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
>>>>
>>>> This fixed the first warning from before, as you suggested, but it still
>>>> crash like this:
>>>>
>>>> [ ] Unable to handle kernel pointer dereference in virtual kernel address space
>>>> [ ] Failing address: 0000000000000000 TEID: 0000000000000483
>>>> [ ] Fault in home space mode while using kernel ASCE.
>>>> [ ] AS:00000000025f0007 R3:00000000dffb8007 S:00000000dffbf000 P:000000000000013d
>>>> [ ] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
>>>> [ ] Modules linked in: ....
>>>> [ ] CPU: 2 PID: 609 Comm: kworker/2:1H Kdump: loaded Tainted: G        W         4.19.0-bb-next+ #1
>>>> [ ] Hardware name: IBM 3906 M03 704 (LPAR)
>>>> [ ] Workqueue: kblockd blk_mq_run_work_fn
>>>> [ ] Krnl PSW : 0704e00180000000 000003ff806a6b40 (zfcp_fc_exec_bsg_job+0x1c0/0x440 [zfcp])
>>>> [ ]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
>>>> [ ] Krnl GPRS: 0000000000000000 0000000083e0f3c0 0000000000000000 0000030000000000
>>>> [ ]            0000030000000000 000003ff806a6b3a 00000000a86b5948 00000000a86b5988
>>>> [ ]            0000000083e0f3f0 0000000000000000 00000000a86b5938 00000000984aee80
>>>> [ ]            00000000a86b5800 000003ff806ba950 000003ff806a6b3a 0000000098a5ed88
>>>> [ ] Krnl Code: 000003ff806a6b30: b9040029            lgr     %r2,%r9
>>>>                000003ff806a6b34: c0e5ffff8b7e        brasl   %r14,3ff80698230
>>>>               #000003ff806a6b3a: e310f0a00004        lg      %r1,160(%r15)
>>>>               >000003ff806a6b40: e31090000024        stg     %r1,0(%r9)
>>>>                000003ff806a6b46: 4120a040            la      %r2,64(%r10)
>>>>                000003ff806a6b4a: c0e5ffff8ae7        brasl   %r14,3ff80698118
>>>>                000003ff806a6b50: e310a0400004        lg      %r1,64(%r10)
>>>>                000003ff806a6b56: e310f0a00024        stg     %r1,160(%r15)
>>>> [ ] Call Trace:
>>>> [ ] ([<000003ff806a6aa2>] zfcp_fc_exec_bsg_job+0x122/0x440 [zfcp])
>>>> [ ]  [<000003ff8061e928>] fc_bsg_dispatch+0x310/0x398 [scsi_transport_fc]
>>>> [ ]  [<0000000000d2995a>] bsg_queue_rq+0x15a/0x198
>>>> [ ]  [<0000000000d03566>] blk_mq_dispatch_rq_list+0x966/0xf90
>>>> [ ]  [<0000000000d0e37a>] blk_mq_sched_dispatch_requests+0x3fa/0x410
>>>> [ ]  [<0000000000cfc230>] __blk_mq_run_hw_queue+0x218/0x248
>>>> [ ]  [<00000000001cb124>] process_one_work+0x8c4/0xe90
>>>> [ ]  [<00000000001cbe58>] worker_thread+0x768/0xbb0
>>>> [ ]  [<00000000001dc67a>] kthread+0x22a/0x248
>>>> [ ]  [<000000000137b812>] kernel_thread_starter+0x6/0xc
>>>> [ ]  [<000000000137b80c>] kernel_thread_starter+0x0/0xc
>>>> [ ] INFO: lockdep is turned off.
>>>> [ ] Last Breaking-Event-Address:
>>>> [ ]  [<00000000005d9b08>] __asan_store8+0x98/0xa0
>>>> [ ]
>>>> [ ] Kernel panic - not syncing: Fatal exception: panic_on_oops
>>>>
>>>> zfcp_fc_exec_bsg_job+0x1c0 is here:
>>>>
>>>>     int zfcp_fc_exec_bsg_job(struct 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;
>>>>             struct fc_rport *rport = fc_bsg_to_rport(job);
>>>>
>>>>             shost = rport ? rport_to_shost(rport) : fc_bsg_to_shost(job);
>>>>             adapter = (struct zfcp_adapter *)shost->hostdata[0];
>>>>
>>>>             if (!(atomic_read(&adapter->status) & ZFCP_STATUS_COMMON_OPEN))
>>>>                     return -EINVAL;
>>>>
>>>> ==>         ct_els->req = job->request_payload.sg_list;
>>>>             ct_els->resp = job->reply_payload.sg_list;
>>>>             ct_els->handler_data = job;
>>>>
>>>>             switch (bsg_request->msgcode) {
>>>>             case FC_BSG_RPT_ELS:
>>>>             case FC_BSG_HST_ELS_NOLOGIN:
>>>>                     return zfcp_fc_exec_els_job(job, adapter);
>>>>             case FC_BSG_RPT_CT:
>>>>             case FC_BSG_HST_CT:
>>>>                     return zfcp_fc_exec_ct_job(job, adapter);
>>>>             default:
>>>>                     return -EINVAL;
>>>>             }
>>>>     }
>>>>
>>>> I took a dump to find out how struct bsg_job looks like when it crashes
>>>> (register clobbering isn't as bad here and I verified that job->dev is valid).
>>>>
>>>>     crash> print *((struct bsg_job *)0x00000000a86b5938)
>>>>     $5 = {
>>>>       dev = 0x9af10358,
>>>>       kref = {
>>>>         refcount = {
>>>>           refs = {
>>>>             counter = 0x1
>>>>           }
>>>>         }
>>>>       },
>>>>       timeout = 0x384,
>>>>       request = 0x83e0f3f0,
>>>>       reply = 0x9559d500,
>>>>       request_len = 0x14,
>>>>       reply_len = 0x0,
>>>>       request_payload = {
>>>>         payload_len = 0x14,
>>>>         sg_cnt = 0x1,
>>>>         sg_list = 0x83e0f3c0
>>>>       },
>>>>       reply_payload = {
>>>>         payload_len = 0x1000,
>>>>         sg_cnt = 0x1,
>>>>         sg_list = 0x83e0f1e0
>>>>       },
>>>>       result = 0x0,
>>>>       reply_payload_rcv_len = 0x0,
>>>> ==>   dd_data = 0x0
>>>>     }
>>>>
>>>> This is where it breaks, job->dd_data is 0x0, so when it wants to write
>>>> to it, it fails.
>>>>
>>>> So We set
>>>>
>>>>     struct fc_function_template zfcp_transport_functions = {
>>>>             ....
>>>>             .dd_bsg_size = sizeof(struct zfcp_fsf_ct_els);
>>>>             ....
>>>>     }
>>>>
>>>> We get to zfcp_fc_exec_bsg_job() via fc_bsg_host_dispatch(). Previously
>>>> this was initially allocated in bsg_setup_queue() with:
>>>>
>>>>             q->cmd_size = sizeof(struct bsg_job) + dd_job_size;
>>>>
>>>> And then in bsg_initialize_rq() with:
>>>>
>>>>             job->dd_data = job + 1;
>>>>
>>>> Sry, I haven't check the rest of your patch yet. But just to
>>>> double-check, I ran some tests against 4.18.15, and there stuff still
>>>> works, so it didn't break w/o me noticing in between (always possible
>>>
>>> Doh, yeah it's obvious now. Try this one on top. I missed it due to the
>>> similar naming, I'll kill off q->initalize_rq_fn as well.
>>>
>>>
>>> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
>>> index 58c9886394d8..198cf42b74a3 100644
>>> --- a/block/bsg-lib.c
>>> +++ b/block/bsg-lib.c
>>> @@ -293,6 +293,7 @@ static const struct blk_mq_ops bsg_mq_ops = {
>>>  	.queue_rq	= bsg_queue_rq,
>>>  	.init_request	= bsg_init_rq,
>>>  	.exit_request	= bsg_exit_rq,
>>> +	.initialize_rq_fn = bsg_initialize_rq,
>>>  	.complete	= bsg_complete,
>>>  };
>>>  
>>> @@ -329,7 +330,6 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
>>>  		goto out_queue;
>>>  	}
>>>  
>>> -	q->initialize_rq_fn = bsg_initialize_rq;
>>>  	q->queuedata = dev;
>>>  	q->bsg_job_fn = job_fn;
>>>  	blk_queue_flag_set(QUEUE_FLAG_BIDI, q);
>>
>> Did you get a chance to test with the above?
>>
> 
> Yes, found time today. This works now. I extended my small test-suite to
> test both sending commands against FC-Host and FC-Rport bsg-devices.
> Both seem to work with zFCP as far as my tests go (but I only have like
> 3 commands in that "test-suite", so its not extensive).

Excellent! Can I add your tested-by to the patch?
Benjamin Block Oct. 26, 2018, 4:31 p.m. UTC | #23 
On Fri, Oct 26, 2018 at 10:08:30AM -0600, Jens Axboe wrote:
> On 10/26/18 10:06 AM, Benjamin Block wrote:
> > On Thu, Oct 25, 2018 at 05:12:59PM -0600, Jens Axboe wrote:
> >> On 10/23/18 12:07 PM, Jens Axboe wrote:
> >>> On 10/23/18 11:40 AM, Benjamin Block wrote:
> >>>>
> >>>> So I tried 4.19.0 with only the two patches from you:
> >>>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
> >>>> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
> >>>>
> >>>> This fixed the first warning from before, as you suggested, but it still
> >>>> crash like this:
> >>>>
> >>>> [ ] Unable to handle kernel pointer dereference in virtual kernel address space
> >>>> [ ] Failing address: 0000000000000000 TEID: 0000000000000483
> >>>> [ ] Fault in home space mode while using kernel ASCE.
> >>>> [ ] AS:00000000025f0007 R3:00000000dffb8007 S:00000000dffbf000 P:000000000000013d
> >>>> [ ] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> >>>> [ ] Modules linked in: ....
> >>>> [ ] CPU: 2 PID: 609 Comm: kworker/2:1H Kdump: loaded Tainted: G        W         4.19.0-bb-next+ #1
> >>>> [ ] Hardware name: IBM 3906 M03 704 (LPAR)
> >>>> [ ] Workqueue: kblockd blk_mq_run_work_fn
> >>>> [ ] Krnl PSW : 0704e00180000000 000003ff806a6b40 (zfcp_fc_exec_bsg_job+0x1c0/0x440 [zfcp])
> >>>> [ ]            R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
> >>>> [ ] Krnl GPRS: 0000000000000000 0000000083e0f3c0 0000000000000000 0000030000000000
> >>>> [ ]            0000030000000000 000003ff806a6b3a 00000000a86b5948 00000000a86b5988
> >>>> [ ]            0000000083e0f3f0 0000000000000000 00000000a86b5938 00000000984aee80
> >>>> [ ]            00000000a86b5800 000003ff806ba950 000003ff806a6b3a 0000000098a5ed88
> >>>> [ ] Krnl Code: 000003ff806a6b30: b9040029            lgr     %r2,%r9
> >>>>                000003ff806a6b34: c0e5ffff8b7e        brasl   %r14,3ff80698230
> >>>>               #000003ff806a6b3a: e310f0a00004        lg      %r1,160(%r15)
> >>>>               >000003ff806a6b40: e31090000024        stg     %r1,0(%r9)
> >>>>                000003ff806a6b46: 4120a040            la      %r2,64(%r10)
> >>>>                000003ff806a6b4a: c0e5ffff8ae7        brasl   %r14,3ff80698118
> >>>>                000003ff806a6b50: e310a0400004        lg      %r1,64(%r10)
> >>>>                000003ff806a6b56: e310f0a00024        stg     %r1,160(%r15)
> >>>> [ ] Call Trace:
> >>>> [ ] ([<000003ff806a6aa2>] zfcp_fc_exec_bsg_job+0x122/0x440 [zfcp])
> >>>> [ ]  [<000003ff8061e928>] fc_bsg_dispatch+0x310/0x398 [scsi_transport_fc]
> >>>> [ ]  [<0000000000d2995a>] bsg_queue_rq+0x15a/0x198
> >>>> [ ]  [<0000000000d03566>] blk_mq_dispatch_rq_list+0x966/0xf90
> >>>> [ ]  [<0000000000d0e37a>] blk_mq_sched_dispatch_requests+0x3fa/0x410
> >>>> [ ]  [<0000000000cfc230>] __blk_mq_run_hw_queue+0x218/0x248
> >>>> [ ]  [<00000000001cb124>] process_one_work+0x8c4/0xe90
> >>>> [ ]  [<00000000001cbe58>] worker_thread+0x768/0xbb0
> >>>> [ ]  [<00000000001dc67a>] kthread+0x22a/0x248
> >>>> [ ]  [<000000000137b812>] kernel_thread_starter+0x6/0xc
> >>>> [ ]  [<000000000137b80c>] kernel_thread_starter+0x0/0xc
> >>>> [ ] INFO: lockdep is turned off.
> >>>> [ ] Last Breaking-Event-Address:
> >>>> [ ]  [<00000000005d9b08>] __asan_store8+0x98/0xa0
> >>>> [ ]
> >>>> [ ] Kernel panic - not syncing: Fatal exception: panic_on_oops
> >>>>
> >>>> zfcp_fc_exec_bsg_job+0x1c0 is here:
> >>>>
> >>>>     int zfcp_fc_exec_bsg_job(struct 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;
> >>>>             struct fc_rport *rport = fc_bsg_to_rport(job);
> >>>>
> >>>>             shost = rport ? rport_to_shost(rport) : fc_bsg_to_shost(job);
> >>>>             adapter = (struct zfcp_adapter *)shost->hostdata[0];
> >>>>
> >>>>             if (!(atomic_read(&adapter->status) & ZFCP_STATUS_COMMON_OPEN))
> >>>>                     return -EINVAL;
> >>>>
> >>>> ==>         ct_els->req = job->request_payload.sg_list;
> >>>>             ct_els->resp = job->reply_payload.sg_list;
> >>>>             ct_els->handler_data = job;
> >>>>
> >>>>             switch (bsg_request->msgcode) {
> >>>>             case FC_BSG_RPT_ELS:
> >>>>             case FC_BSG_HST_ELS_NOLOGIN:
> >>>>                     return zfcp_fc_exec_els_job(job, adapter);
> >>>>             case FC_BSG_RPT_CT:
> >>>>             case FC_BSG_HST_CT:
> >>>>                     return zfcp_fc_exec_ct_job(job, adapter);
> >>>>             default:
> >>>>                     return -EINVAL;
> >>>>             }
> >>>>     }
> >>>>
> >>>> I took a dump to find out how struct bsg_job looks like when it crashes
> >>>> (register clobbering isn't as bad here and I verified that job->dev is valid).
> >>>>
> >>>>     crash> print *((struct bsg_job *)0x00000000a86b5938)
> >>>>     $5 = {
> >>>>       dev = 0x9af10358,
> >>>>       kref = {
> >>>>         refcount = {
> >>>>           refs = {
> >>>>             counter = 0x1
> >>>>           }
> >>>>         }
> >>>>       },
> >>>>       timeout = 0x384,
> >>>>       request = 0x83e0f3f0,
> >>>>       reply = 0x9559d500,
> >>>>       request_len = 0x14,
> >>>>       reply_len = 0x0,
> >>>>       request_payload = {
> >>>>         payload_len = 0x14,
> >>>>         sg_cnt = 0x1,
> >>>>         sg_list = 0x83e0f3c0
> >>>>       },
> >>>>       reply_payload = {
> >>>>         payload_len = 0x1000,
> >>>>         sg_cnt = 0x1,
> >>>>         sg_list = 0x83e0f1e0
> >>>>       },
> >>>>       result = 0x0,
> >>>>       reply_payload_rcv_len = 0x0,
> >>>> ==>   dd_data = 0x0
> >>>>     }
> >>>>
> >>>> This is where it breaks, job->dd_data is 0x0, so when it wants to write
> >>>> to it, it fails.
> >>>>
> >>>> So We set
> >>>>
> >>>>     struct fc_function_template zfcp_transport_functions = {
> >>>>             ....
> >>>>             .dd_bsg_size = sizeof(struct zfcp_fsf_ct_els);
> >>>>             ....
> >>>>     }
> >>>>
> >>>> We get to zfcp_fc_exec_bsg_job() via fc_bsg_host_dispatch(). Previously
> >>>> this was initially allocated in bsg_setup_queue() with:
> >>>>
> >>>>             q->cmd_size = sizeof(struct bsg_job) + dd_job_size;
> >>>>
> >>>> And then in bsg_initialize_rq() with:
> >>>>
> >>>>             job->dd_data = job + 1;
> >>>>
> >>>> Sry, I haven't check the rest of your patch yet. But just to
> >>>> double-check, I ran some tests against 4.18.15, and there stuff still
> >>>> works, so it didn't break w/o me noticing in between (always possible
> >>>
> >>> Doh, yeah it's obvious now. Try this one on top. I missed it due to the
> >>> similar naming, I'll kill off q->initalize_rq_fn as well.
> >>>
> >>>
> >>> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
> >>> index 58c9886394d8..198cf42b74a3 100644
> >>> --- a/block/bsg-lib.c
> >>> +++ b/block/bsg-lib.c
> >>> @@ -293,6 +293,7 @@ static const struct blk_mq_ops bsg_mq_ops = {
> >>>  	.queue_rq	= bsg_queue_rq,
> >>>  	.init_request	= bsg_init_rq,
> >>>  	.exit_request	= bsg_exit_rq,
> >>> +	.initialize_rq_fn = bsg_initialize_rq,
> >>>  	.complete	= bsg_complete,
> >>>  };
> >>>  
> >>> @@ -329,7 +330,6 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
> >>>  		goto out_queue;
> >>>  	}
> >>>  
> >>> -	q->initialize_rq_fn = bsg_initialize_rq;
> >>>  	q->queuedata = dev;
> >>>  	q->bsg_job_fn = job_fn;
> >>>  	blk_queue_flag_set(QUEUE_FLAG_BIDI, q);
> >>
> >> Did you get a chance to test with the above?
> >>
> > 
> > Yes, found time today. This works now. I extended my small test-suite to
> > test both sending commands against FC-Host and FC-Rport bsg-devices.
> > Both seem to work with zFCP as far as my tests go (but I only have like
> > 3 commands in that "test-suite", so its not extensive).
> 
> Excellent! Can I add your tested-by to the patch?
> 

Sure.
diff mbox series

Patch

diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index f3501cdaf1a6..1aa0ed3fc339 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -21,7 +21,7 @@ 
  *
  */
 #include <linux/slab.h>
-#include <linux/blkdev.h>
+#include <linux/blk-mq.h>
 #include <linux/delay.h>
 #include <linux/scatterlist.h>
 #include <linux/bsg-lib.h>
@@ -129,7 +129,7 @@  static void bsg_teardown_job(struct kref *kref)
 	kfree(job->request_payload.sg_list);
 	kfree(job->reply_payload.sg_list);
 
-	blk_end_request_all(rq, BLK_STS_OK);
+	blk_mq_end_request(rq, BLK_STS_OK);
 }
 
 void bsg_job_put(struct bsg_job *job)
@@ -162,10 +162,10 @@  void bsg_job_done(struct bsg_job *job, int result,
 EXPORT_SYMBOL_GPL(bsg_job_done);
 
 /**
- * bsg_softirq_done - softirq done routine for destroying the bsg requests
+ * bsg_complete - softirq done routine for destroying the bsg requests
  * @rq: BSG request that holds the job to be destroyed
  */
-static void bsg_softirq_done(struct request *rq)
+static void bsg_complete(struct request *rq)
 {
 	struct bsg_job *job = blk_mq_rq_to_pdu(rq);
 
@@ -224,54 +224,46 @@  static bool bsg_prepare_job(struct device *dev, struct request *req)
 }
 
 /**
- * bsg_request_fn - generic handler for bsg requests
- * @q: request queue to manage
+ * bsg_queue_rq - generic handler for bsg requests
+ * @hctx: hardware queue
+ * @bd: queue data
  *
  * On error the create_bsg_job function should return a -Exyz error value
  * that will be set to ->result.
  *
  * Drivers/subsys should pass this to the queue init function.
  */
-static void bsg_request_fn(struct request_queue *q)
-	__releases(q->queue_lock)
-	__acquires(q->queue_lock)
+static blk_status_t bsg_queue_rq(struct blk_mq_hw_ctx *hctx,
+				 const struct blk_mq_queue_data *bd)
 {
+	struct request_queue *q = hctx->queue;
 	struct device *dev = q->queuedata;
-	struct request *req;
+	struct request *req = bd->rq;
 	int ret;
 
+	blk_mq_start_request(req);
+
 	if (!get_device(dev))
-		return;
-
-	while (1) {
-		req = blk_fetch_request(q);
-		if (!req)
-			break;
-		spin_unlock_irq(q->queue_lock);
-
-		if (!bsg_prepare_job(dev, req)) {
-			blk_end_request_all(req, BLK_STS_OK);
-			spin_lock_irq(q->queue_lock);
-			continue;
-		}
-
-		ret = q->bsg_job_fn(blk_mq_rq_to_pdu(req));
-		spin_lock_irq(q->queue_lock);
-		if (ret)
-			break;
-	}
+		return BLK_STS_IOERR;
+
+	if (!bsg_prepare_job(dev, req))
+		return BLK_STS_IOERR;
+
+	ret = q->bsg_job_fn(blk_mq_rq_to_pdu(req));
+	if (ret)
+		return BLK_STS_IOERR;
 
-	spin_unlock_irq(q->queue_lock);
 	put_device(dev);
-	spin_lock_irq(q->queue_lock);
+	return BLK_STS_OK;
 }
 
 /* called right after the request is allocated for the request_queue */
-static int bsg_init_rq(struct request_queue *q, struct request *req, gfp_t gfp)
+static int bsg_init_rq(struct blk_mq_tag_set *set, struct request *req,
+		       unsigned int hctx_idx, unsigned int numa_node)
 {
 	struct bsg_job *job = blk_mq_rq_to_pdu(req);
 
-	job->reply = kzalloc(SCSI_SENSE_BUFFERSIZE, gfp);
+	job->reply = kzalloc(SCSI_SENSE_BUFFERSIZE, GFP_KERNEL);
 	if (!job->reply)
 		return -ENOMEM;
 	return 0;
@@ -289,13 +281,21 @@  static void bsg_initialize_rq(struct request *req)
 	job->dd_data = job + 1;
 }
 
-static void bsg_exit_rq(struct request_queue *q, struct request *req)
+static void bsg_exit_rq(struct blk_mq_tag_set *set, struct request *req,
+		       unsigned int hctx_idx)
 {
 	struct bsg_job *job = blk_mq_rq_to_pdu(req);
 
 	kfree(job->reply);
 }
 
+static const struct blk_mq_ops bsg_mq_ops = {
+	.queue_rq	= bsg_queue_rq,
+	.init_request	= bsg_init_rq,
+	.exit_request	= bsg_exit_rq,
+	.complete	= bsg_complete,
+};
+
 /**
  * bsg_setup_queue - Create and add the bsg hooks so we can receive requests
  * @dev: device to attach bsg device to
@@ -306,26 +306,32 @@  static void bsg_exit_rq(struct request_queue *q, struct request *req)
 struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
 		bsg_job_fn *job_fn, int dd_job_size)
 {
+	struct blk_mq_tag_set *set;
 	struct request_queue *q;
-	int ret;
+	int ret = -ENOMEM;
 
-	q = blk_alloc_queue(GFP_KERNEL);
-	if (!q)
+	set = kzalloc(sizeof(*set), GFP_KERNEL);
+	if (set)
 		return ERR_PTR(-ENOMEM);
-	q->cmd_size = sizeof(struct bsg_job) + dd_job_size;
-	q->init_rq_fn = bsg_init_rq;
-	q->exit_rq_fn = bsg_exit_rq;
-	q->initialize_rq_fn = bsg_initialize_rq;
-	q->request_fn = bsg_request_fn;
 
-	ret = blk_init_allocated_queue(q);
-	if (ret)
-		goto out_cleanup_queue;
+	set->ops = &bsg_mq_ops;
+	set->nr_hw_queues = 1;
+	set->queue_depth = 128;
+	set->numa_node = NUMA_NO_NODE;
+	set->cmd_size = sizeof(struct bsg_job) + dd_job_size;
+	if (blk_mq_alloc_tag_set(set))
+		goto out_tag_set;
+
+	q = blk_mq_init_queue(set);
+	if (IS_ERR(q)) {
+		ret = PTR_ERR(q);
+		goto out_queue;
+	}
 
+	q->initialize_rq_fn = bsg_initialize_rq;
 	q->queuedata = dev;
 	q->bsg_job_fn = job_fn;
 	blk_queue_flag_set(QUEUE_FLAG_BIDI, q);
-	blk_queue_softirq_done(q, bsg_softirq_done);
 	blk_queue_rq_timeout(q, BLK_DEFAULT_SG_TIMEOUT);
 
 	ret = bsg_register_queue(q, dev, name, &bsg_transport_ops);
@@ -338,6 +344,10 @@  struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
 	return q;
 out_cleanup_queue:
 	blk_cleanup_queue(q);
+out_queue:
+	blk_mq_free_tag_set(set);
+out_tag_set:
+	kfree(set);
 	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(bsg_setup_queue);
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 381668fa135d..3dccc4874920 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -3592,7 +3592,7 @@  fc_bsg_job_timeout(struct request *req)
 
 	/* the blk_end_sync_io() doesn't check the error */
 	if (inflight)
-		__blk_complete_request(req);
+		blk_mq_end_request(req, BLK_STS_IOERR);
 	return BLK_EH_DONE;
 }
 
@@ -3684,14 +3684,9 @@  static void
 fc_bsg_goose_queue(struct fc_rport *rport)
 {
 	struct request_queue *q = rport->rqst_q;
-	unsigned long flags;
-
-	if (!q)
-		return;
 
-	spin_lock_irqsave(q->queue_lock, flags);
-	blk_run_queue_async(q);
-	spin_unlock_irqrestore(q->queue_lock, flags);
+	if (q)
+		blk_mq_run_hw_queues(q, true);
 }
 
 /**
@@ -3759,6 +3754,37 @@  static int fc_bsg_dispatch(struct bsg_job *job)
 		return fc_bsg_host_dispatch(shost, job);
 }
 
+static blk_status_t fc_bsg_rport_prep(struct fc_rport *rport)
+{
+	if (rport->port_state == FC_PORTSTATE_BLOCKED &&
+	    !(rport->flags & FC_RPORT_FAST_FAIL_TIMEDOUT))
+		return BLK_STS_RESOURCE;
+
+	if (rport->port_state != FC_PORTSTATE_ONLINE)
+		return BLK_STS_IOERR;
+
+	return BLK_STS_OK;
+}
+
+
+static int fc_bsg_dispatch_prep(struct bsg_job *job)
+{
+	struct fc_rport *rport = fc_bsg_to_rport(job);
+	blk_status_t ret;
+
+	ret = fc_bsg_rport_prep(rport);
+	switch (ret) {
+	case BLK_STS_OK:
+		break;
+	case BLK_STS_RESOURCE:
+		return -EAGAIN;
+	default:
+		return -EIO;
+	}
+
+	return fc_bsg_dispatch(job);
+}
+
 /**
  * fc_bsg_hostadd - Create and add the bsg hooks so we can receive requests
  * @shost:	shost for fc_host
@@ -3789,25 +3815,10 @@  fc_bsg_hostadd(struct Scsi_Host *shost, struct fc_host_attrs *fc_host)
 	}
 	__scsi_init_queue(shost, q);
 	blk_queue_rq_timed_out(q, fc_bsg_job_timeout);
-	blk_queue_rq_timeout(q, FC_DEFAULT_BSG_TIMEOUT);
 	fc_host->rqst_q = q;
 	return 0;
 }
 
-static int fc_bsg_rport_prep(struct request_queue *q, struct request *req)
-{
-	struct fc_rport *rport = dev_to_rport(q->queuedata);
-
-	if (rport->port_state == FC_PORTSTATE_BLOCKED &&
-	    !(rport->flags & FC_RPORT_FAST_FAIL_TIMEDOUT))
-		return BLKPREP_DEFER;
-
-	if (rport->port_state != FC_PORTSTATE_ONLINE)
-		return BLKPREP_KILL;
-
-	return BLKPREP_OK;
-}
-
 /**
  * fc_bsg_rportadd - Create and add the bsg hooks so we can receive requests
  * @shost:	shost that rport is attached to
@@ -3825,16 +3836,14 @@  fc_bsg_rportadd(struct Scsi_Host *shost, struct fc_rport *rport)
 	if (!i->f->bsg_request)
 		return -ENOTSUPP;
 
-	q = bsg_setup_queue(dev, dev_name(dev), fc_bsg_dispatch,
+	q = bsg_setup_queue(dev, dev_name(dev), fc_bsg_dispatch_prep,
 			i->f->dd_bsg_size);
 	if (IS_ERR(q)) {
 		dev_err(dev, "failed to setup bsg queue\n");
 		return PTR_ERR(q);
 	}
 	__scsi_init_queue(shost, q);
-	blk_queue_prep_rq(q, fc_bsg_rport_prep);
 	blk_queue_rq_timed_out(q, fc_bsg_job_timeout);
-	blk_queue_rq_timeout(q, BLK_DEFAULT_SG_TIMEOUT);
 	rport->rqst_q = q;
 	return 0;
 }