* [PATCH] SCSI: remove noop in fc_bsg_goose_queue()
@ 2010-09-21 15:19 Hillf Danton
2010-09-21 15:41 ` James Bottomley
0 siblings, 1 reply; 6+ messages in thread
From: Hillf Danton @ 2010-09-21 15:19 UTC (permalink / raw)
To: James Smart, linux-scsi, Mike Christie, FUJITA Tomonori,
"James E.J. Bottomley" <James.>
The tests for QUEUE_FLAG_REENTER seem unnecessary.
And check for get_device() is added.
Signed-off-by: Hillf Danton <dhillf@gmail.com>
---
--- o/linux-2.6.36-rc4/drivers/scsi/scsi_transport_fc.c 2010-09-13
07:07:38.000000000 +0800
+++ m/linux-2.6.36-rc4/drivers/scsi/scsi_transport_fc.c 2010-09-21
22:05:38.000000000 +0800
@@ -3766,16 +3766,11 @@ fc_bsg_goose_queue(struct fc_rport *rpor
if (!rport->rqst_q)
return;
- get_device(&rport->dev);
+ if (! get_device(&rport->dev))
+ return;
spin_lock_irqsave(rport->rqst_q->queue_lock, flags);
- flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
- !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
- if (flagset)
- queue_flag_set(QUEUE_FLAG_REENTER, rport->rqst_q);
__blk_run_queue(rport->rqst_q);
- if (flagset)
- queue_flag_clear(QUEUE_FLAG_REENTER, rport->rqst_q);
spin_unlock_irqrestore(rport->rqst_q->queue_lock, flags);
put_device(&rport->dev);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SCSI: remove noop in fc_bsg_goose_queue()
2010-09-21 15:19 [PATCH] SCSI: remove noop in fc_bsg_goose_queue() Hillf Danton
@ 2010-09-21 15:41 ` James Bottomley
2010-09-21 17:35 ` Hillf Danton
2010-09-23 15:10 ` Hillf Danton
0 siblings, 2 replies; 6+ messages in thread
From: James Bottomley @ 2010-09-21 15:41 UTC (permalink / raw)
To: Hillf Danton; +Cc: James Smart, linux-scsi, Mike Christie, FUJITA Tomonori
On Tue, 2010-09-21 at 23:19 +0800, Hillf Danton wrote:
> The tests for QUEUE_FLAG_REENTER seem unnecessary.
> And check for get_device() is added.
OK, so you've done a few newbie patches; it's time to graduate.
> Signed-off-by: Hillf Danton <dhillf@gmail.com>
> ---
>
> --- o/linux-2.6.36-rc4/drivers/scsi/scsi_transport_fc.c 2010-09-13
> 07:07:38.000000000 +0800
> +++ m/linux-2.6.36-rc4/drivers/scsi/scsi_transport_fc.c 2010-09-21
> 22:05:38.000000000 +0800
> @@ -3766,16 +3766,11 @@ fc_bsg_goose_queue(struct fc_rport *rpor
> if (!rport->rqst_q)
> return;
>
> - get_device(&rport->dev);
> + if (! get_device(&rport->dev))
> + return;
The expression in the if clause is never true ... can you tell me why?
> spin_lock_irqsave(rport->rqst_q->queue_lock, flags);
> - flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
> - !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
> - if (flagset)
> - queue_flag_set(QUEUE_FLAG_REENTER, rport->rqst_q);
> __blk_run_queue(rport->rqst_q);
> - if (flagset)
> - queue_flag_clear(QUEUE_FLAG_REENTER, rport->rqst_q);
> spin_unlock_irqrestore(rport->rqst_q->queue_lock, flags);
this code doesn't do anything because there's a bug in it. If you can
work out what it's trying to do, you should be able to fix the bug.
James
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SCSI: remove noop in fc_bsg_goose_queue()
2010-09-21 15:41 ` James Bottomley
@ 2010-09-21 17:35 ` Hillf Danton
2010-09-23 15:10 ` Hillf Danton
1 sibling, 0 replies; 6+ messages in thread
From: Hillf Danton @ 2010-09-21 17:35 UTC (permalink / raw)
To: James Bottomley; +Cc: James Smart, linux-scsi, Mike Christie, FUJITA Tomonori
On Tue, Sep 21, 2010 at 11:41 PM, James Bottomley
<James.Bottomley@suse.de> wrote:
> On Tue, 2010-09-21 at 23:19 +0800, Hillf Danton wrote:
>> The tests for QUEUE_FLAG_REENTER seem unnecessary.
>> And check for get_device() is added.
>
> OK, so you've done a few newbie patches; it's time to graduate.
>
What is the certificate :-)
>> Signed-off-by: Hillf Danton <dhillf@gmail.com>
>> ---
>>
>> --- o/linux-2.6.36-rc4/drivers/scsi/scsi_transport_fc.c 2010-09-13
>> 07:07:38.000000000 +0800
>> +++ m/linux-2.6.36-rc4/drivers/scsi/scsi_transport_fc.c 2010-09-21
>> 22:05:38.000000000 +0800
>> @@ -3766,16 +3766,11 @@ fc_bsg_goose_queue(struct fc_rport *rpor
>> if (!rport->rqst_q)
>> return;
>>
>> - get_device(&rport->dev);
>> + if (! get_device(&rport->dev))
>> + return;
>
> The expression in the if clause is never true ... can you tell me why?
>
It is simple. What get_device does is clear, and I am paranoid.
struct device *get_device(struct device *dev)
{
return dev ? to_dev(kobject_get(&dev->kobj)) : NULL;
}
>> spin_lock_irqsave(rport->rqst_q->queue_lock, flags);
>> - flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
>> - !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
>> - if (flagset)
>> - queue_flag_set(QUEUE_FLAG_REENTER, rport->rqst_q);
>> __blk_run_queue(rport->rqst_q);
>> - if (flagset)
>> - queue_flag_clear(QUEUE_FLAG_REENTER, rport->rqst_q);
>> spin_unlock_irqrestore(rport->rqst_q->queue_lock, flags);
>
> this code doesn't do anything because there's a bug in it. If you can
> work out what it's trying to do, you should be able to fix the bug.
>
> James
>
It is hard to point out what is the bug, since flagset never is true.
And I guess that simply calling blk_run_queue() seems fine,
leaving check for QUEUE_FLAG_REENTER to be done in block core.
Hillf
--- o/linux-2.6.36-rc4/drivers/scsi/scsi_transport_fc.c 2010-09-13
07:07:38.000000000 +0800
+++ m/linux-2.6.36-rc4/drivers/scsi/scsi_transport_fc.c 2010-09-22
01:37:42.000000000 +0800
@@ -3760,24 +3760,11 @@ fail_host_msg:
static void
fc_bsg_goose_queue(struct fc_rport *rport)
{
- int flagset;
- unsigned long flags;
-
if (!rport->rqst_q)
return;
get_device(&rport->dev);
-
- spin_lock_irqsave(rport->rqst_q->queue_lock, flags);
- flagset = test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags) &&
- !test_bit(QUEUE_FLAG_REENTER, &rport->rqst_q->queue_flags);
- if (flagset)
- queue_flag_set(QUEUE_FLAG_REENTER, rport->rqst_q);
- __blk_run_queue(rport->rqst_q);
- if (flagset)
- queue_flag_clear(QUEUE_FLAG_REENTER, rport->rqst_q);
- spin_unlock_irqrestore(rport->rqst_q->queue_lock, flags);
-
+ blk_run_queue(rport->rqst_q);
put_device(&rport->dev);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SCSI: remove noop in fc_bsg_goose_queue()
2010-09-21 15:41 ` James Bottomley
2010-09-21 17:35 ` Hillf Danton
@ 2010-09-23 15:10 ` Hillf Danton
2010-09-26 22:58 ` James Bottomley
1 sibling, 1 reply; 6+ messages in thread
From: Hillf Danton @ 2010-09-23 15:10 UTC (permalink / raw)
To: James Bottomley; +Cc: James Smart, linux-scsi, Mike Christie, FUJITA Tomonori
On Tue, Sep 21, 2010 at 11:41 PM, James Bottomley
<James.Bottomley@suse.de> wrote:
>
> this code doesn't do anything because there's a bug in it. If you can
> work out what it's trying to do, you should be able to fix the bug.
>
> James
>
What is it, James? Because I could not goto deep sleep with this bug in ear.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SCSI: remove noop in fc_bsg_goose_queue()
2010-09-23 15:10 ` Hillf Danton
@ 2010-09-26 22:58 ` James Bottomley
2010-10-01 2:30 ` Hillf Danton
0 siblings, 1 reply; 6+ messages in thread
From: James Bottomley @ 2010-09-26 22:58 UTC (permalink / raw)
To: Hillf Danton; +Cc: James Smart, linux-scsi, Mike Christie, FUJITA Tomonori
On Thu, 2010-09-23 at 23:10 +0800, Hillf Danton wrote:
> On Tue, Sep 21, 2010 at 11:41 PM, James Bottomley
> <James.Bottomley@suse.de> wrote:
> >
> > this code doesn't do anything because there's a bug in it. If you can
> > work out what it's trying to do, you should be able to fix the bug.
> >
> > James
> >
> What is it, James? Because I could not goto deep sleep with this bug in ear.
Think about it ... it's clearly something to do with the flag ... what
is it? Google is useful as well as looking in the block source code.
The key would be knowing what the flag does.
James
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SCSI: remove noop in fc_bsg_goose_queue()
2010-09-26 22:58 ` James Bottomley
@ 2010-10-01 2:30 ` Hillf Danton
0 siblings, 0 replies; 6+ messages in thread
From: Hillf Danton @ 2010-10-01 2:30 UTC (permalink / raw)
To: James Bottomley; +Cc: James Smart, linux-scsi, Mike Christie, FUJITA Tomonori
On Mon, Sep 27, 2010 at 6:58 AM, James Bottomley
<James.Bottomley@suse.de> wrote:
> Think about it ... it's clearly something to do with the flag ... what
> is it? Google is useful as well as looking in the block source code.
> The key would be knowing what the flag does.
>
> James
>
node.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-01 2:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-21 15:19 [PATCH] SCSI: remove noop in fc_bsg_goose_queue() Hillf Danton
2010-09-21 15:41 ` James Bottomley
2010-09-21 17:35 ` Hillf Danton
2010-09-23 15:10 ` Hillf Danton
2010-09-26 22:58 ` James Bottomley
2010-10-01 2:30 ` Hillf Danton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox