public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: PROBLEM: kernel BUG at block/cfq-iosched.c:3402
  2010-03-19  5:31 PROBLEM: kernel BUG at block/cfq-iosched.c:3402 Hugh Daschbach
@ 2010-03-02  0:20 ` Dmitry Torokhov
  2010-03-19  5:31 ` [PATCH] Don't change direction flags in struct request Hugh Daschbach
  1 sibling, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2010-03-02  0:20 UTC (permalink / raw)
  To: Hugh Daschbach
  Cc: James E.J. Bottomley, Chandra Seetharaman, Eddie Williams,
	linux-scsi, linux-kernel

Hi,

On Thu, Mar 18, 2010 at 10:31:29PM -0700, Hugh Daschbach wrote:
	^^^^^^^^^^^^

> Apologies in advance if this seems like nagging.  I've posted this
> before, but never saw it pass through the mailing list.
> 

Could it be that they were sent from more distant future and not reached
us yet? ;)

-- 
Dmitry

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Don't change direction flags in struct request.
  2010-03-19  5:31 ` [PATCH] Don't change direction flags in struct request Hugh Daschbach
@ 2010-03-02  5:26   ` James Bottomley
  2010-03-02  7:54     ` Mike Christie
  2010-03-12 22:04   ` Mike Christie
  1 sibling, 1 reply; 9+ messages in thread
From: James Bottomley @ 2010-03-02  5:26 UTC (permalink / raw)
  To: Hugh Daschbach
  Cc: Chandra Seetharaman, Eddie Williams, linux-scsi, linux-kernel

On Thu, 2010-03-18 at 22:31 -0700, Hugh Daschbach wrote:
> The EMC multipath device handler should not change the I/O direction
> flags of its trespass command request.
> 
> The CFQ elevator may BUG if the direction flags on an I/O request are
> changed after allocation.  cfq_set_request() and cfq_put_request()
> count READ and WRITE requests separately.  Changing the I/O request
> direction after blk_get_request() allocates the request throws off
> this CFQ accounting.

This description doesn't really match what the problem seems to be
below:

> Signed-off-by: Hugh Daschbach <hdasch@broadcom.com>
> ---
>  drivers/scsi/device_handler/scsi_dh_emc.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
> index 6196675..3709342 100644
> --- a/drivers/scsi/device_handler/scsi_dh_emc.c
> +++ b/drivers/scsi/device_handler/scsi_dh_emc.c
> @@ -269,10 +269,12 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
>  				unsigned char *buffer)
>  {
>  	struct request *rq;
> +	int mode = READ;
>  	int len = 0;
>  
> -	rq = blk_get_request(sdev->request_queue,
> -			(cmd == MODE_SELECT) ? WRITE : READ, GFP_NOIO);
> +	if (cmd == MODE_SELECT || cmd == MODE_SELECT_10)
> +		mode = WRITE;
> +	rq = blk_get_request(sdev->request_queue, mode, GFP_NOIO);

So the actual bug is failure to set WRITE for MODE_SELECT_10.

>  	if (!rq) {
>  		sdev_printk(KERN_INFO, sdev, "get_req: blk_get_request failed");
>  		return NULL;
> @@ -284,12 +286,10 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
>  	switch (cmd) {
>  	case MODE_SELECT:
>  		len = sizeof(short_trespass);
> -		rq->cmd_flags |= REQ_RW;

And this is just cosmetic ... the flags already having been set by the
allocation, they don't need to be set again.

>  		rq->cmd[1] = 0x10;
>  		break;
>  	case MODE_SELECT_10:
>  		len = sizeof(long_trespass);
> -		rq->cmd_flags |= REQ_RW;
>  		rq->cmd[1] = 0x10;
>  		break;
>  	case INQUIRY:

Chandra, can you tidy this up and take it, please?

Thanks,

James



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Don't change direction flags in struct request.
  2010-03-02  5:26   ` James Bottomley
@ 2010-03-02  7:54     ` Mike Christie
  2010-03-03 10:29       ` James Bottomley
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Christie @ 2010-03-02  7:54 UTC (permalink / raw)
  To: James Bottomley
  Cc: Hugh Daschbach, Chandra Seetharaman, Eddie Williams, linux-scsi,
	linux-kernel, Hannes Reinecke

On 03/01/2010 11:26 PM, James Bottomley wrote:
> On Thu, 2010-03-18 at 22:31 -0700, Hugh Daschbach wrote:
>> The EMC multipath device handler should not change the I/O direction
>> flags of its trespass command request.
>>
>> The CFQ elevator may BUG if the direction flags on an I/O request are
>> changed after allocation.  cfq_set_request() and cfq_put_request()
>> count READ and WRITE requests separately.  Changing the I/O request
>> direction after blk_get_request() allocates the request throws off
>> this CFQ accounting.
>
> This description doesn't really match what the problem seems to be
> below:
>
>> Signed-off-by: Hugh Daschbach<hdasch@broadcom.com>
>> ---
>>   drivers/scsi/device_handler/scsi_dh_emc.c |    8 ++++----
>>   1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
>> index 6196675..3709342 100644
>> --- a/drivers/scsi/device_handler/scsi_dh_emc.c
>> +++ b/drivers/scsi/device_handler/scsi_dh_emc.c
>> @@ -269,10 +269,12 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
>>   				unsigned char *buffer)
>>   {
>>   	struct request *rq;
>> +	int mode = READ;
>>   	int len = 0;
>>
>> -	rq = blk_get_request(sdev->request_queue,
>> -			(cmd == MODE_SELECT) ? WRITE : READ, GFP_NOIO);
>> +	if (cmd == MODE_SELECT || cmd == MODE_SELECT_10)
>> +		mode = WRITE;
>> +	rq = blk_get_request(sdev->request_queue, mode, GFP_NOIO);
>
> So the actual bug is failure to set WRITE for MODE_SELECT_10.
>

I think we have a fix for this and some len issues in that module that 
was sent here:
http://marc.info/?l=linux-scsi&m=125978574800618&w=2

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Don't change direction flags in struct request.
  2010-03-02  7:54     ` Mike Christie
@ 2010-03-03 10:29       ` James Bottomley
  0 siblings, 0 replies; 9+ messages in thread
From: James Bottomley @ 2010-03-03 10:29 UTC (permalink / raw)
  To: Mike Christie
  Cc: Hugh Daschbach, Chandra Seetharaman, Eddie Williams, linux-scsi,
	linux-kernel, Hannes Reinecke

On Tue, 2010-03-02 at 01:54 -0600, Mike Christie wrote:
> On 03/01/2010 11:26 PM, James Bottomley wrote:
> > On Thu, 2010-03-18 at 22:31 -0700, Hugh Daschbach wrote:
> >> The EMC multipath device handler should not change the I/O direction
> >> flags of its trespass command request.
> >>
> >> The CFQ elevator may BUG if the direction flags on an I/O request are
> >> changed after allocation.  cfq_set_request() and cfq_put_request()
> >> count READ and WRITE requests separately.  Changing the I/O request
> >> direction after blk_get_request() allocates the request throws off
> >> this CFQ accounting.
> >
> > This description doesn't really match what the problem seems to be
> > below:
> >
> >> Signed-off-by: Hugh Daschbach<hdasch@broadcom.com>
> >> ---
> >>   drivers/scsi/device_handler/scsi_dh_emc.c |    8 ++++----
> >>   1 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
> >> index 6196675..3709342 100644
> >> --- a/drivers/scsi/device_handler/scsi_dh_emc.c
> >> +++ b/drivers/scsi/device_handler/scsi_dh_emc.c
> >> @@ -269,10 +269,12 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
> >>   				unsigned char *buffer)
> >>   {
> >>   	struct request *rq;
> >> +	int mode = READ;
> >>   	int len = 0;
> >>
> >> -	rq = blk_get_request(sdev->request_queue,
> >> -			(cmd == MODE_SELECT) ? WRITE : READ, GFP_NOIO);
> >> +	if (cmd == MODE_SELECT || cmd == MODE_SELECT_10)
> >> +		mode = WRITE;
> >> +	rq = blk_get_request(sdev->request_queue, mode, GFP_NOIO);
> >
> > So the actual bug is failure to set WRITE for MODE_SELECT_10.
> >
> 
> I think we have a fix for this and some len issues in that module that 
> was sent here:
> http://marc.info/?l=linux-scsi&m=125978574800618&w=2

Yes, you did ... the need to change the From: field caused me to mark it
in a different way and ultimately lose it ... I'll add it this time ...

James



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Don't change direction flags in struct request.
  2010-03-19  5:31 ` [PATCH] Don't change direction flags in struct request Hugh Daschbach
  2010-03-02  5:26   ` James Bottomley
@ 2010-03-12 22:04   ` Mike Christie
  2010-03-12 22:26     ` Hugh Daschbach
  2010-03-19  2:21     ` Hugh Daschbach
  1 sibling, 2 replies; 9+ messages in thread
From: Mike Christie @ 2010-03-12 22:04 UTC (permalink / raw)
  To: Hugh Daschbach
  Cc: James E.J. Bottomley, Chandra Seetharaman, Eddie Williams,
	linux-scsi, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2433 bytes --]

On 03/19/2010 12:31 AM, Hugh Daschbach wrote:
> The EMC multipath device handler should not change the I/O direction
> flags of its trespass command request.
>
> The CFQ elevator may BUG if the direction flags on an I/O request are
> changed after allocation.  cfq_set_request() and cfq_put_request()
> count READ and WRITE requests separately.  Changing the I/O request
> direction after blk_get_request() allocates the request throws off
> this CFQ accounting.
>
> Signed-off-by: Hugh Daschbach<hdasch@broadcom.com>
> ---
>   drivers/scsi/device_handler/scsi_dh_emc.c |    8 ++++----
>   1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
> index 6196675..3709342 100644
> --- a/drivers/scsi/device_handler/scsi_dh_emc.c
> +++ b/drivers/scsi/device_handler/scsi_dh_emc.c
> @@ -269,10 +269,12 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
>   				unsigned char *buffer)
>   {
>   	struct request *rq;
> +	int mode = READ;
>   	int len = 0;
>
> -	rq = blk_get_request(sdev->request_queue,
> -			(cmd == MODE_SELECT) ? WRITE : READ, GFP_NOIO);
> +	if (cmd == MODE_SELECT || cmd == MODE_SELECT_10)
> +		mode = WRITE;
> +	rq = blk_get_request(sdev->request_queue, mode, GFP_NOIO);
>   	if (!rq) {
>   		sdev_printk(KERN_INFO, sdev, "get_req: blk_get_request failed");
>   		return NULL;
> @@ -284,12 +286,10 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
>   	switch (cmd) {
>   	case MODE_SELECT:
>   		len = sizeof(short_trespass);
> -		rq->cmd_flags |= REQ_RW;
>   		rq->cmd[1] = 0x10;
>   		break;
>   	case MODE_SELECT_10:
>   		len = sizeof(long_trespass);
> -		rq->cmd_flags |= REQ_RW;
>   		rq->cmd[1] = 0x10;
>   		break;
>   	case INQUIRY:

Did you try the patch I linked to the last time you posted about this?

I think there should be a patch which fixes this problem here:
http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commitdiff;h=6c71dcb28ff9b63b814a0b76a256f5dae08d3e0d;hp=3a5b27bf6f29574d667230c7e76e4b83fe3014e0
I think it got into 2.6.34-rc1.

With that patch mode selects should get set up as writes.

Does it work? If it does then I think we just need the attached cleanup 
patch which removes the cmd_flags setting. The patch that got merged 
fixed the problem of initializing the request properly, but it forgot to 
remove the cmd_flags setting.

[-- Attachment #2: emc-cleanup-cmd-flags.patch --]
[-- Type: text/plain, Size: 823 bytes --]

blk_get_request sets the cmd_flags, so we should not and do not
need to set them. If we did set them to a different value then
it can cause a oops in the elevator code.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>

diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
index 63032ec..2752892 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -284,13 +284,11 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
 	switch (cmd) {
 	case MODE_SELECT:
 		len = sizeof(short_trespass);
-		rq->cmd_flags |= REQ_RW;
 		rq->cmd[1] = 0x10;
 		rq->cmd[4] = len;
 		break;
 	case MODE_SELECT_10:
 		len = sizeof(long_trespass);
-		rq->cmd_flags |= REQ_RW;
 		rq->cmd[1] = 0x10;
 		rq->cmd[8] = len;
 		break;

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: [PATCH] Don't change direction flags in struct request.
  2010-03-12 22:04   ` Mike Christie
@ 2010-03-12 22:26     ` Hugh Daschbach
  2010-03-19  2:21     ` Hugh Daschbach
  1 sibling, 0 replies; 9+ messages in thread
From: Hugh Daschbach @ 2010-03-12 22:26 UTC (permalink / raw)
  To: Mike Christie
  Cc: James E.J. Bottomley, Chandra Seetharaman, Eddie Williams,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org

Mike Christie [mailto:michaelc@cs.wisc.edu] writes:

> On 03/19/2010 12:31 AM, Hugh Daschbach wrote:
>> The EMC multipath device handler should not change the I/O direction
>> flags of its trespass command request.

...

> Did you try the patch I linked to the last time you posted about this?

I have not.  When I read it, I believed it would fix the issue.

> I think there should be a patch which fixes this problem here:
> http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commitdiff;h=6c71dcb28ff9b63b814a0b76a256f5dae08d3e0d;hp=3a5b27bf6f29574d667230c7e76e4b83fe3014e0
> I think it got into 2.6.34-rc1.

Then I'll pull it and test it.

> With that patch mode selects should get set up as writes.
>
> Does it work? If it does then I think we just need the attached cleanup 
> patch which removes the cmd_flags setting. The patch that got merged 
> fixed the problem of initializing the request properly, but it forgot to 
> remove the cmd_flags setting.

Agree.  I'll test it early next week and confirm.

Thanks,
Hugh

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] Don't change direction flags in struct request.
  2010-03-12 22:04   ` Mike Christie
  2010-03-12 22:26     ` Hugh Daschbach
@ 2010-03-19  2:21     ` Hugh Daschbach
  1 sibling, 0 replies; 9+ messages in thread
From: Hugh Daschbach @ 2010-03-19  2:21 UTC (permalink / raw)
  To: Hugh Daschbach, Mike Christie
  Cc: James E.J. Bottomley, Chandra Seetharaman, Eddie Williams,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org

Hugh Daschbach [mailto:hdasch@broadcom.com] wrote:
> Mike Christie [mailto:michaelc@cs.wisc.edu] writes:

>> On 03/19/2010 12:31 AM, Hugh Daschbach wrote:
>>> The EMC multipath device handler should not change the I/O direction
>>> flags of its trespass command request.

...

>> Did you try the patch I linked to the last time you posted about this?

> I have not.  When I read it, I believed it would fix the issue.

>> I think there should be a patch which fixes this problem here:
>> http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commitdiff;h=6c71dcb28ff9b63b814a0b76a256f5dae08d3e0d;hp=3a5b27bf6f29574d667230c7e76e4b83fe3014e0
>> I think it got into 2.6.34-rc1.

> Then I'll pull it and test it.

>> With that patch mode selects should get set up as writes.
>>
>> Does it work? If it does then I think we just need the attached cleanup 
>> patch which removes the cmd_flags setting. The patch that got merged 
>> fixed the problem of initializing the request properly, but it forgot to 
>> remove the cmd_flags setting.

> Agree.  I'll test it early next week and confirm.

I have pulled and tested this patch.  It does fix my issue.

Thanks,
Hugh


^ permalink raw reply	[flat|nested] 9+ messages in thread

* PROBLEM: kernel BUG at block/cfq-iosched.c:3402
@ 2010-03-19  5:31 Hugh Daschbach
  2010-03-02  0:20 ` Dmitry Torokhov
  2010-03-19  5:31 ` [PATCH] Don't change direction flags in struct request Hugh Daschbach
  0 siblings, 2 replies; 9+ messages in thread
From: Hugh Daschbach @ 2010-03-19  5:31 UTC (permalink / raw)
  To: James E.J. Bottomley, Chandra Seetharaman, Eddie Williams
  Cc: linux-scsi, linux-kernel, Hugh Daschbach

Apologies in advance if this seems like nagging.  I've posted this
before, but never saw it pass through the mailing list.

For long trespass CDBs, the EMC multipath device handler (in
get_req()) allocates a "struct request" with READ specified in the I/O
direction flag.  It then changes this to a WRITE before
send_trespass_cmd() calls blk_execute_rq().  This provokes a BUG() in
cfq_put_request() after the I/O completes and the request is released.

cfq_set_request() and cfq_put_request() count READ and WRITE requests
separately.  Changing the I/O request direction after blk_get_request()
allocates the request throws off this CFQ accounting.

Here's a traceback of the BUG.

Dec 28 15:06:07 ------------[ cut here ]------------
kernel BUG at block/cfq-iosched.c:3402!
invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
last sysfs file: /sys/devices/pci0000:00/0000:00:03.0/0000:02:00.1/irq
CPU 1
Modules linked in: dm_round_robin scsi_dh_emc bnx2fc libfcoe libfc scsi_transport_fc cnic 8021q nfs fscache nfsd lockd bridge nfs_acl auth_rpcgss exportfs stp bnep sco l2cap crc16 bluetooth sunrpc iptable_filter ip_tables ip6table_filter ip6_tables x_tables ipv6 loop dm_multipath scsi_dh uinput sr_mod cdrom bnx2x ata_piix mdio serio_raw pcspkr rtc_cmos libata rtc_core rtc_lib bnx2 sg button joydev dcdbas dm_snapshot dm_zero dm_mirror dm_region_hash dm_log dm_mod shpchp mptsas mptscsih mptbase scsi_transport_sas sd_mod scsi_mod ext3 jbd uhci_hcd ohci_hcd ehci_hcd [last unloaded: microcode]
Pid: 1645, comm: kmpath_handlerd Tainted: G        W  2.6.32-next #33 PowerEdge T710
RIP: 0010:[<ffffffff81168581>]  [<ffffffff81168581>] cfq_put_request+0x28/0x66
RSP: 0018:ffff880128e4dc50  EFLAGS: 00010046
RAX: 0000000000000000 RBX: ffff8800ace10ea0 RCX: ffff880128ded1e0
RDX: 0000000000000001 RSI: ffff8800ace10ea0 RDI: ffff8800ace10ea0
RBP: ffff880128e4dc60 R08: ffff880128e4db90 R09: ffff880128ded218
R10: ffff880128e4dbe0 R11: ffffffff8115d3d9 R12: ffff880091b37ee0
R13: ffff8801124f3460 R14: 0000000000002000 R15: 0000000000000000
FS:  0000000000000000(0000) GS:ffff88002f200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000931000 CR3: 000000010f220000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process kmpath_handlerd (pid: 1645, threadinfo ffff880128e4c000, task ffff880128decb40)
Stack:
 ffff8800ace10ea0 0000000001282c4f ffff880128e4dc70 ffffffff8115667d
<0> ffff880128e4dca0 ffffffff8115ce21 ffff8801124f3460 ffff8800ace10ea0
<0> 0000000000000282 ffff8801124f27f0 ffff880128e4dcd0 ffffffff8115d3e7
Call Trace:
 [<ffffffff8115667d>] elv_put_request+0x19/0x1b
 [<ffffffff8115ce21>] __blk_put_request+0x88/0xb9
 [<ffffffff8115d3e7>] blk_put_request+0x2e/0x46
 [<ffffffffa00206ed>] send_trespass_cmd+0x24e/0x260 [scsi_dh_emc]
 [<ffffffffa0020d9b>] clariion_activate+0x55/0x13e [scsi_dh_emc]
 [<ffffffff81062818>] ? trace_hardirqs_on_caller+0x113/0x13e
 [<ffffffffa0268cd7>] ? pg_init_done+0x0/0x187 [dm_multipath]
 [<ffffffffa01767ec>] scsi_dh_activate+0x70/0xbd [scsi_dh]
 [<ffffffffa0268cd7>] ? pg_init_done+0x0/0x187 [dm_multipath]
 [<ffffffffa026a4c1>] ? activate_path+0x0/0x2b [dm_multipath]
 [<ffffffffa026a4ea>] activate_path+0x29/0x2b [dm_multipath]
 [<ffffffff810519eb>] worker_thread+0x1af/0x2ae
 [<ffffffff81051992>] ? worker_thread+0x156/0x2ae
 [<ffffffff812f1a9a>] ? schedule+0x8e6/0x996
 [<ffffffff810549e4>] ? autoremove_wake_function+0x0/0x38
 [<ffffffff8105183c>] ? worker_thread+0x0/0x2ae
 [<ffffffff810548eb>] kthread+0x7d/0x86
 [<ffffffff810038fa>] child_rip+0xa/0x20
 [<ffffffff812f487c>] ? restore_args+0x0/0x30
 [<ffffffff81179e0b>] ? debug_object_activate+0x38/0xd8
 [<ffffffff81058de1>] ? sched_clock_cpu+0x42/0xc7
 [<ffffffff8105486e>] ? kthread+0x0/0x86
 [<ffffffff810038f0>] ? child_rip+0x0/0x20
Code: 5c c9 c3 55 48 89 e5 41 54 53 4c 8b a7 a8 00 00 00 48 89 fb 4d 85 e4 74 4b 8b 47 48 83 e0 01 48 63 d0 41 8b 44 94 68 85 c0 75 04 <0f> 0b eb fe ff c8 41 89 44 94 68 48 8b 87 a0 00 00 00 48 8b 78
RIP  [<ffffffff81168581>] cfq_put_request+0x28/0x66
 RSP <ffff880128e4dc50>
---[ end trace fd0fef9ed4a171a1 ]---

Hugh Daschbach (1):
  Don't change direction flags in struct request.

 drivers/scsi/device_handler/scsi_dh_emc.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] Don't change direction flags in struct request.
  2010-03-19  5:31 PROBLEM: kernel BUG at block/cfq-iosched.c:3402 Hugh Daschbach
  2010-03-02  0:20 ` Dmitry Torokhov
@ 2010-03-19  5:31 ` Hugh Daschbach
  2010-03-02  5:26   ` James Bottomley
  2010-03-12 22:04   ` Mike Christie
  1 sibling, 2 replies; 9+ messages in thread
From: Hugh Daschbach @ 2010-03-19  5:31 UTC (permalink / raw)
  To: James E.J. Bottomley, Chandra Seetharaman, Eddie Williams
  Cc: linux-scsi, linux-kernel, Hugh Daschbach

The EMC multipath device handler should not change the I/O direction
flags of its trespass command request.

The CFQ elevator may BUG if the direction flags on an I/O request are
changed after allocation.  cfq_set_request() and cfq_put_request()
count READ and WRITE requests separately.  Changing the I/O request
direction after blk_get_request() allocates the request throws off
this CFQ accounting.

Signed-off-by: Hugh Daschbach <hdasch@broadcom.com>
---
 drivers/scsi/device_handler/scsi_dh_emc.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
index 6196675..3709342 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -269,10 +269,12 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
 				unsigned char *buffer)
 {
 	struct request *rq;
+	int mode = READ;
 	int len = 0;
 
-	rq = blk_get_request(sdev->request_queue,
-			(cmd == MODE_SELECT) ? WRITE : READ, GFP_NOIO);
+	if (cmd == MODE_SELECT || cmd == MODE_SELECT_10)
+		mode = WRITE;
+	rq = blk_get_request(sdev->request_queue, mode, GFP_NOIO);
 	if (!rq) {
 		sdev_printk(KERN_INFO, sdev, "get_req: blk_get_request failed");
 		return NULL;
@@ -284,12 +286,10 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
 	switch (cmd) {
 	case MODE_SELECT:
 		len = sizeof(short_trespass);
-		rq->cmd_flags |= REQ_RW;
 		rq->cmd[1] = 0x10;
 		break;
 	case MODE_SELECT_10:
 		len = sizeof(long_trespass);
-		rq->cmd_flags |= REQ_RW;
 		rq->cmd[1] = 0x10;
 		break;
 	case INQUIRY:
-- 
1.6.6.196.g1f735



^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2010-03-19  2:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-19  5:31 PROBLEM: kernel BUG at block/cfq-iosched.c:3402 Hugh Daschbach
2010-03-02  0:20 ` Dmitry Torokhov
2010-03-19  5:31 ` [PATCH] Don't change direction flags in struct request Hugh Daschbach
2010-03-02  5:26   ` James Bottomley
2010-03-02  7:54     ` Mike Christie
2010-03-03 10:29       ` James Bottomley
2010-03-12 22:04   ` Mike Christie
2010-03-12 22:26     ` Hugh Daschbach
2010-03-19  2:21     ` Hugh Daschbach

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox