Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
Greetings, James. On Fri, Apr 01, 2005 at 12:09:48PM -0600, James Bottomley wrote: > On Fri, 2005-04-01 at 14:01 +0900, Tejun Heo wrote: > > > Well, REQ_SPECIAL is the signal to the mid-layer that we've allocated > > > the resources necessary to process the command, so in practice it will > > > be turned on for every requeue request (because we set it when the > > > command is prepared), > > > > Sorry, but where? AFAICT, only blk_insert_request() and > > scsi_init_io() on sgtable allocation failure set REQ_SPECIAL during > > scsi midlayer processing. This patch replaces blk_insert_request() > > with blk_requeue_request() and the next patch removes REQ_SPECIAL > > setting in scsi_init_io(). > > > > REQ_SPECIAL is currently overloaded to mean two different things. > > > > * The request is a special request. > > * The request has been requeued using scsi_queue_insert(). > >i.e. It has been prepped. > > But its true meaning is defined by the block layer (since it's a block > flag). It's supposed to mean that the ->special field of the request is > in use to carry data meaningful to the underlying driver. SCSI sets it > on that basis. > > So, if I understand correctly, based on the fact that the current block > code in fact never bothers with REQ_SPECIAL, but only checks req- > >special, you're proposing that we need never actually set REQ_SPECIAL > when making use of the ->special field? Thus you want to use > REQ_SPECIAL to distinguish between internally generated commands and > external commands? That sounds fine as long as the block API gets > updated to reflect this (comments in linux/blkdev.h shoudl be fine). Yeap, That's what I'm proposing. Block API never cares about REQ_SPECIAL flag or ->special field except for when determining if requests can be merged - both fields are independently checked in this case. And IDE driver already uses the flag regardless of ->special field. Do you want me to clear up the comment? > > > The other reason I don't like this is that we've been trying hard to > > > sweep excess block knowledge out of the mid-layer. I don't think > > > REQ_SOFTBARRIER is anything we really have to know about. > > > > We currently requeue using two different block functions. > > > > * blk_insert_request(): this function does two things > > 1. enqueue special requests > > 2. turn on REQ_SPECIAL|REQ_SOFTBARRIER and call > >blk_requeue_request(). This is used only by scsi midlayer. > > * blk_requeue_request() > > > > REQ_SOFTBARRIER tells the request queue that other requests shouldn't > > pass this one. We need this to be set for prepped requests; > > otherwise, it may, theoretically, deadlock (unprepped request waiting > > for the prepped request back in the queue). So, the current code > > > > * depends on blk_insert_request() sets REQ_SOFTBARRIER when > >requeueing. It works but nothing in the interface or semantics > >is clear about it. Why expect a request reinserted using > >blk_insert_request() gets REQ_SOFTBARRIER turned on while a request > >reinserted with blk_requeue_request() doesn't? or why are there > >two different paths doing mostly the same thing with slightly > >different semantics? > > * requeueing using blk_requeue_request() in scsi_request_fn() doesn't > >turn on either REQ_SPECIAL or REQ_SOFTBARRIER. Missing REQ_SPECIAL > >is okay, as we have the extra path in prep_fn (if it ever gets requeued > >due to medium failure, so gets re-prepped), but missing > >REQ_SOFTBARRIER can *theoretically* cause problem. > > > > So, it's more likely becoming less dependent on unobvious behavior of > > block layer. As we need the request to stay on top, we tell the block > > layer to do so, instead of depending on blk_insert_request() > > unobviously doing it for us. > > But that's the point. This is non obvious behaviour in the block layer, > so SCSI doesn't want to know about it (and the block maintainer won't > want to trawl through the SCSI and other block drivers altering it if it > changes). Yes, it's non-obvious behavior of blk_insert_request() when used for requeueing and, SCSI is the *only* user. This patch removes the unobvious path. > If the bug is that the block layer isn't setting it on all > our requeues where it should, then either we're using the wrong API or > the block layer API is buggy. But block drivers in general don't have to inhibit reordering requeued requests. SCSI midlayer caches resources over requeueing, so it's SCSI midlayer's requirement that requeued requests shouldn't be passed. IOW, it's SCSI midlayer's responsibility to tell the block layer not to reschedule the request. REQ_SOFTBARRIER has well-defined meaning to tell just that. It's not a block-layer internal thing. It's a public interface. IOW, this patch uses well-defined flag to tell the block layer what the SCSI midlayer wants. I don't see any problem there. If you're uncomfortable
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
On Fri, 2005-04-01 at 14:01 +0900, Tejun Heo wrote: > > Well, REQ_SPECIAL is the signal to the mid-layer that we've allocated > > the resources necessary to process the command, so in practice it will > > be turned on for every requeue request (because we set it when the > > command is prepared), > > Sorry, but where? AFAICT, only blk_insert_request() and > scsi_init_io() on sgtable allocation failure set REQ_SPECIAL during > scsi midlayer processing. This patch replaces blk_insert_request() > with blk_requeue_request() and the next patch removes REQ_SPECIAL > setting in scsi_init_io(). > > REQ_SPECIAL is currently overloaded to mean two different things. > > * The request is a special request. > * The request has been requeued using scsi_queue_insert(). >i.e. It has been prepped. But its true meaning is defined by the block layer (since it's a block flag). It's supposed to mean that the ->special field of the request is in use to carry data meaningful to the underlying driver. SCSI sets it on that basis. So, if I understand correctly, based on the fact that the current block code in fact never bothers with REQ_SPECIAL, but only checks req- >special, you're proposing that we need never actually set REQ_SPECIAL when making use of the ->special field? Thus you want to use REQ_SPECIAL to distinguish between internally generated commands and external commands? That sounds fine as long as the block API gets updated to reflect this (comments in linux/blkdev.h shoudl be fine). > > The other reason I don't like this is that we've been trying hard to > > sweep excess block knowledge out of the mid-layer. I don't think > > REQ_SOFTBARRIER is anything we really have to know about. > > We currently requeue using two different block functions. > > * blk_insert_request(): this function does two things > 1. enqueue special requests > 2. turn on REQ_SPECIAL|REQ_SOFTBARRIER and call > blk_requeue_request(). This is used only by scsi midlayer. > * blk_requeue_request() > > REQ_SOFTBARRIER tells the request queue that other requests shouldn't > pass this one. We need this to be set for prepped requests; > otherwise, it may, theoretically, deadlock (unprepped request waiting > for the prepped request back in the queue). So, the current code > > * depends on blk_insert_request() sets REQ_SOFTBARRIER when >requeueing. It works but nothing in the interface or semantics >is clear about it. Why expect a request reinserted using >blk_insert_request() gets REQ_SOFTBARRIER turned on while a request >reinserted with blk_requeue_request() doesn't? or why are there >two different paths doing mostly the same thing with slightly >different semantics? > * requeueing using blk_requeue_request() in scsi_request_fn() doesn't >turn on either REQ_SPECIAL or REQ_SOFTBARRIER. Missing REQ_SPECIAL >is okay, as we have the extra path in prep_fn (if it ever gets requeued >due to medium failure, so gets re-prepped), but missing >REQ_SOFTBARRIER can *theoretically* cause problem. > > So, it's more likely becoming less dependent on unobvious behavior of > block layer. As we need the request to stay on top, we tell the block > layer to do so, instead of depending on blk_insert_request() > unobviously doing it for us. But that's the point. This is non obvious behaviour in the block layer, so SCSI doesn't want to know about it (and the block maintainer won't want to trawl through the SCSI and other block drivers altering it if it changes). If the bug is that the block layer isn't setting it on all our requeues where it should, then either we're using the wrong API or the block layer API is buggy. James - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
On Fri, 2005-04-01 at 14:01 +0900, Tejun Heo wrote: Well, REQ_SPECIAL is the signal to the mid-layer that we've allocated the resources necessary to process the command, so in practice it will be turned on for every requeue request (because we set it when the command is prepared), Sorry, but where? AFAICT, only blk_insert_request() and scsi_init_io() on sgtable allocation failure set REQ_SPECIAL during scsi midlayer processing. This patch replaces blk_insert_request() with blk_requeue_request() and the next patch removes REQ_SPECIAL setting in scsi_init_io(). REQ_SPECIAL is currently overloaded to mean two different things. * The request is a special request. * The request has been requeued using scsi_queue_insert(). i.e. It has been prepped. But its true meaning is defined by the block layer (since it's a block flag). It's supposed to mean that the -special field of the request is in use to carry data meaningful to the underlying driver. SCSI sets it on that basis. So, if I understand correctly, based on the fact that the current block code in fact never bothers with REQ_SPECIAL, but only checks req- special, you're proposing that we need never actually set REQ_SPECIAL when making use of the -special field? Thus you want to use REQ_SPECIAL to distinguish between internally generated commands and external commands? That sounds fine as long as the block API gets updated to reflect this (comments in linux/blkdev.h shoudl be fine). The other reason I don't like this is that we've been trying hard to sweep excess block knowledge out of the mid-layer. I don't think REQ_SOFTBARRIER is anything we really have to know about. We currently requeue using two different block functions. * blk_insert_request(): this function does two things 1. enqueue special requests 2. turn on REQ_SPECIAL|REQ_SOFTBARRIER and call blk_requeue_request(). This is used only by scsi midlayer. * blk_requeue_request() REQ_SOFTBARRIER tells the request queue that other requests shouldn't pass this one. We need this to be set for prepped requests; otherwise, it may, theoretically, deadlock (unprepped request waiting for the prepped request back in the queue). So, the current code * depends on blk_insert_request() sets REQ_SOFTBARRIER when requeueing. It works but nothing in the interface or semantics is clear about it. Why expect a request reinserted using blk_insert_request() gets REQ_SOFTBARRIER turned on while a request reinserted with blk_requeue_request() doesn't? or why are there two different paths doing mostly the same thing with slightly different semantics? * requeueing using blk_requeue_request() in scsi_request_fn() doesn't turn on either REQ_SPECIAL or REQ_SOFTBARRIER. Missing REQ_SPECIAL is okay, as we have the extra path in prep_fn (if it ever gets requeued due to medium failure, so gets re-prepped), but missing REQ_SOFTBARRIER can *theoretically* cause problem. So, it's more likely becoming less dependent on unobvious behavior of block layer. As we need the request to stay on top, we tell the block layer to do so, instead of depending on blk_insert_request() unobviously doing it for us. But that's the point. This is non obvious behaviour in the block layer, so SCSI doesn't want to know about it (and the block maintainer won't want to trawl through the SCSI and other block drivers altering it if it changes). If the bug is that the block layer isn't setting it on all our requeues where it should, then either we're using the wrong API or the block layer API is buggy. James - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
Greetings, James. On Fri, Apr 01, 2005 at 12:09:48PM -0600, James Bottomley wrote: On Fri, 2005-04-01 at 14:01 +0900, Tejun Heo wrote: Well, REQ_SPECIAL is the signal to the mid-layer that we've allocated the resources necessary to process the command, so in practice it will be turned on for every requeue request (because we set it when the command is prepared), Sorry, but where? AFAICT, only blk_insert_request() and scsi_init_io() on sgtable allocation failure set REQ_SPECIAL during scsi midlayer processing. This patch replaces blk_insert_request() with blk_requeue_request() and the next patch removes REQ_SPECIAL setting in scsi_init_io(). REQ_SPECIAL is currently overloaded to mean two different things. * The request is a special request. * The request has been requeued using scsi_queue_insert(). i.e. It has been prepped. But its true meaning is defined by the block layer (since it's a block flag). It's supposed to mean that the -special field of the request is in use to carry data meaningful to the underlying driver. SCSI sets it on that basis. So, if I understand correctly, based on the fact that the current block code in fact never bothers with REQ_SPECIAL, but only checks req- special, you're proposing that we need never actually set REQ_SPECIAL when making use of the -special field? Thus you want to use REQ_SPECIAL to distinguish between internally generated commands and external commands? That sounds fine as long as the block API gets updated to reflect this (comments in linux/blkdev.h shoudl be fine). Yeap, That's what I'm proposing. Block API never cares about REQ_SPECIAL flag or -special field except for when determining if requests can be merged - both fields are independently checked in this case. And IDE driver already uses the flag regardless of -special field. Do you want me to clear up the comment? The other reason I don't like this is that we've been trying hard to sweep excess block knowledge out of the mid-layer. I don't think REQ_SOFTBARRIER is anything we really have to know about. We currently requeue using two different block functions. * blk_insert_request(): this function does two things 1. enqueue special requests 2. turn on REQ_SPECIAL|REQ_SOFTBARRIER and call blk_requeue_request(). This is used only by scsi midlayer. * blk_requeue_request() REQ_SOFTBARRIER tells the request queue that other requests shouldn't pass this one. We need this to be set for prepped requests; otherwise, it may, theoretically, deadlock (unprepped request waiting for the prepped request back in the queue). So, the current code * depends on blk_insert_request() sets REQ_SOFTBARRIER when requeueing. It works but nothing in the interface or semantics is clear about it. Why expect a request reinserted using blk_insert_request() gets REQ_SOFTBARRIER turned on while a request reinserted with blk_requeue_request() doesn't? or why are there two different paths doing mostly the same thing with slightly different semantics? * requeueing using blk_requeue_request() in scsi_request_fn() doesn't turn on either REQ_SPECIAL or REQ_SOFTBARRIER. Missing REQ_SPECIAL is okay, as we have the extra path in prep_fn (if it ever gets requeued due to medium failure, so gets re-prepped), but missing REQ_SOFTBARRIER can *theoretically* cause problem. So, it's more likely becoming less dependent on unobvious behavior of block layer. As we need the request to stay on top, we tell the block layer to do so, instead of depending on blk_insert_request() unobviously doing it for us. But that's the point. This is non obvious behaviour in the block layer, so SCSI doesn't want to know about it (and the block maintainer won't want to trawl through the SCSI and other block drivers altering it if it changes). Yes, it's non-obvious behavior of blk_insert_request() when used for requeueing and, SCSI is the *only* user. This patch removes the unobvious path. If the bug is that the block layer isn't setting it on all our requeues where it should, then either we're using the wrong API or the block layer API is buggy. But block drivers in general don't have to inhibit reordering requeued requests. SCSI midlayer caches resources over requeueing, so it's SCSI midlayer's requirement that requeued requests shouldn't be passed. IOW, it's SCSI midlayer's responsibility to tell the block layer not to reschedule the request. REQ_SOFTBARRIER has well-defined meaning to tell just that. It's not a block-layer internal thing. It's a public interface. IOW, this patch uses well-defined flag to tell the block layer what the SCSI midlayer wants. I don't see any problem there. If you're uncomfortable with using REQ_* flag directly, writing a wrapper shouldn't be a problem, but, IMHO, current form seems fine. Thanks. -- tejun -
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
Hello, James. On Thu, Mar 31, 2005 at 11:53:20AM -0600, James Bottomley wrote: > On Thu, 2005-03-31 at 18:07 +0900, Tejun Heo wrote: > > 01_scsi_no_REQ_SPECIAL_on_requeue.patch > > > > blk_insert_request() has 'reinsert' argument, which, when set, > > turns on REQ_SPECIAL and REQ_SOFTBARRIER and requeues the > > request. SCSI midlayer was the only user of this feature and > > all requeued requests become special requests defeating > > quiesce state. This patch makes scsi midlayer use > > blk_requeue_request() for requeueing and removes 'reinsert' > > feature from blk_insert_request(). > > Well, REQ_SPECIAL is the signal to the mid-layer that we've allocated > the resources necessary to process the command, so in practice it will > be turned on for every requeue request (because we set it when the > command is prepared), Sorry, but where? AFAICT, only blk_insert_request() and scsi_init_io() on sgtable allocation failure set REQ_SPECIAL during scsi midlayer processing. This patch replaces blk_insert_request() with blk_requeue_request() and the next patch removes REQ_SPECIAL setting in scsi_init_io(). REQ_SPECIAL is currently overloaded to mean two different things. * The request is a special request. * The request has been requeued using scsi_queue_insert(). i.e. It has been prepped. However, #2 can be tested by rq->special != NULL condition, and, in fact, the prep_fn already has the code. This is why this and the next patch don't break the requeue path. IMHO, this proves the subtlety of the REQ_SPECIAL semantics. Which path is taken on which case gets kind of confusing by meaning two different things with REQ_SPECIAL. > so this patch would have no effect on your stated > quiesce problem. However, if you think about how requests work with > head insertion and one command following another, there's really not a > huge problem here either. I agree that it's not a huge problem, but it's subtle and has the potential of causing probably non-destructive but confusing behavior on rare cases. > The other reason I don't like this is that we've been trying hard to > sweep excess block knowledge out of the mid-layer. I don't think > REQ_SOFTBARRIER is anything we really have to know about. We currently requeue using two different block functions. * blk_insert_request(): this function does two things 1. enqueue special requests 2. turn on REQ_SPECIAL|REQ_SOFTBARRIER and call blk_requeue_request(). This is used only by scsi midlayer. * blk_requeue_request() REQ_SOFTBARRIER tells the request queue that other requests shouldn't pass this one. We need this to be set for prepped requests; otherwise, it may, theoretically, deadlock (unprepped request waiting for the prepped request back in the queue). So, the current code * depends on blk_insert_request() sets REQ_SOFTBARRIER when requeueing. It works but nothing in the interface or semantics is clear about it. Why expect a request reinserted using blk_insert_request() gets REQ_SOFTBARRIER turned on while a request reinserted with blk_requeue_request() doesn't? or why are there two different paths doing mostly the same thing with slightly different semantics? * requeueing using blk_requeue_request() in scsi_request_fn() doesn't turn on either REQ_SPECIAL or REQ_SOFTBARRIER. Missing REQ_SPECIAL is okay, as we have the extra path in prep_fn (if it ever gets requeued due to medium failure, so gets re-prepped), but missing REQ_SOFTBARRIER can *theoretically* cause problem. So, it's more likely becoming less dependent on unobvious behavior of block layer. As we need the request to stay on top, we tell the block layer to do so, instead of depending on blk_insert_request() unobviously doing it for us. Thanks. -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
On Thu, Mar 31, 2005 at 11:12:11AM +0100, Christoph Hellwig wrote: > On Thu, Mar 31, 2005 at 06:07:55PM +0900, Tejun Heo wrote: > > 01_scsi_no_REQ_SPECIAL_on_requeue.patch > > > > blk_insert_request() has 'reinsert' argument, which, when set, > > turns on REQ_SPECIAL and REQ_SOFTBARRIER and requeues the > > request. SCSI midlayer was the only user of this feature and > > all requeued requests become special requests defeating > > quiesce state. This patch makes scsi midlayer use > > blk_requeue_request() for requeueing and removes 'reinsert' > > feature from blk_insert_request(). > > > > Note: In drivers/scsi/scsi_lib.c, scsi_single_lun_run() and > > scsi_run_queue() are moved upward unchanged. > > That lest part doesn't belong into this patch at all. Actually, it is, as scsi_queue_insert() is changed to call scsi_run_queue() explicitly. However, scsi_queue_insert() is removed later, so the change is pretty dumb. Maybe I'll add prototype and remove it together later, or reorder patches. Thanks. -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
On Thu, 2005-03-31 at 18:07 +0900, Tejun Heo wrote: > 01_scsi_no_REQ_SPECIAL_on_requeue.patch > > blk_insert_request() has 'reinsert' argument, which, when set, > turns on REQ_SPECIAL and REQ_SOFTBARRIER and requeues the > request. SCSI midlayer was the only user of this feature and > all requeued requests become special requests defeating > quiesce state. This patch makes scsi midlayer use > blk_requeue_request() for requeueing and removes 'reinsert' > feature from blk_insert_request(). Well, REQ_SPECIAL is the signal to the mid-layer that we've allocated the resources necessary to process the command, so in practice it will be turned on for every requeue request (because we set it when the command is prepared), so this patch would have no effect on your stated quiesce problem. However, if you think about how requests work with head insertion and one command following another, there's really not a huge problem here either. The other reason I don't like this is that we've been trying hard to sweep excess block knowledge out of the mid-layer. I don't think REQ_SOFTBARRIER is anything we really have to know about. James - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
On Thu, Mar 31, 2005 at 06:07:55PM +0900, Tejun Heo wrote: > 01_scsi_no_REQ_SPECIAL_on_requeue.patch > > blk_insert_request() has 'reinsert' argument, which, when set, > turns on REQ_SPECIAL and REQ_SOFTBARRIER and requeues the > request. SCSI midlayer was the only user of this feature and > all requeued requests become special requests defeating > quiesce state. This patch makes scsi midlayer use > blk_requeue_request() for requeueing and removes 'reinsert' > feature from blk_insert_request(). > > Note: In drivers/scsi/scsi_lib.c, scsi_single_lun_run() and > scsi_run_queue() are moved upward unchanged. That lest part doesn't belong into this patch at all. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
01_scsi_no_REQ_SPECIAL_on_requeue.patch
blk_insert_request() has 'reinsert' argument, which, when set,
turns on REQ_SPECIAL and REQ_SOFTBARRIER and requeues the
request. SCSI midlayer was the only user of this feature and
all requeued requests become special requests defeating
quiesce state. This patch makes scsi midlayer use
blk_requeue_request() for requeueing and removes 'reinsert'
feature from blk_insert_request().
Note: In drivers/scsi/scsi_lib.c, scsi_single_lun_run() and
scsi_run_queue() are moved upward unchanged.
Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
drivers/block/ll_rw_blk.c | 20 +--
drivers/block/paride/pd.c |2
drivers/block/sx8.c |4
drivers/scsi/scsi_lib.c | 238 +++---
include/linux/blkdev.h|2
5 files changed, 133 insertions(+), 133 deletions(-)
Index: scsi-export/drivers/block/ll_rw_blk.c
===
--- scsi-export.orig/drivers/block/ll_rw_blk.c 2005-03-31 17:42:05.0
+0900
+++ scsi-export/drivers/block/ll_rw_blk.c 2005-03-31 18:06:19.0
+0900
@@ -2028,7 +2028,6 @@ EXPORT_SYMBOL(blk_requeue_request);
* @rq:request to be inserted
* @at_head: insert request at head or tail of queue
* @data: private data
- * @reinsert: true if request it a reinsertion of previously processed one
*
* Description:
*Many block devices need to execute commands asynchronously, so they don't
@@ -2043,8 +2042,9 @@ EXPORT_SYMBOL(blk_requeue_request);
*host that is unable to accept a particular command.
*/
void blk_insert_request(request_queue_t *q, struct request *rq,
- int at_head, void *data, int reinsert)
+ int at_head, void *data)
{
+ int where = at_head ? ELEVATOR_INSERT_FRONT : ELEVATOR_INSERT_BACK;
unsigned long flags;
/*
@@ -2061,20 +2061,12 @@ void blk_insert_request(request_queue_t
/*
* If command is tagged, release the tag
*/
- if (reinsert)
- blk_requeue_request(q, rq);
- else {
- int where = ELEVATOR_INSERT_BACK;
+ if (blk_rq_tagged(rq))
+ blk_queue_end_tag(q, rq);
- if (at_head)
- where = ELEVATOR_INSERT_FRONT;
+ drive_stat_acct(rq, rq->nr_sectors, 1);
+ __elv_add_request(q, rq, where, 0);
- if (blk_rq_tagged(rq))
- blk_queue_end_tag(q, rq);
-
- drive_stat_acct(rq, rq->nr_sectors, 1);
- __elv_add_request(q, rq, where, 0);
- }
if (blk_queue_plugged(q))
__generic_unplug_device(q);
else
Index: scsi-export/drivers/block/paride/pd.c
===
--- scsi-export.orig/drivers/block/paride/pd.c 2005-03-31 17:42:05.0
+0900
+++ scsi-export/drivers/block/paride/pd.c 2005-03-31 18:06:19.0
+0900
@@ -723,7 +723,7 @@ static int pd_special_command(struct pd_
rq.ref_count = 1;
rq.waiting =
rq.end_io = blk_end_sync_rq;
- blk_insert_request(disk->gd->queue, , 0, func, 0);
+ blk_insert_request(disk->gd->queue, , 0, func);
wait_for_completion();
rq.waiting = NULL;
if (rq.errors)
Index: scsi-export/drivers/block/sx8.c
===
--- scsi-export.orig/drivers/block/sx8.c2005-03-31 17:42:05.0
+0900
+++ scsi-export/drivers/block/sx8.c 2005-03-31 18:06:19.0 +0900
@@ -614,7 +614,7 @@ static int carm_array_info (struct carm_
spin_unlock_irq(>lock);
DPRINTK("blk_insert_request, tag == %u\n", idx);
- blk_insert_request(host->oob_q, crq->rq, 1, crq, 0);
+ blk_insert_request(host->oob_q, crq->rq, 1, crq);
return 0;
@@ -653,7 +653,7 @@ static int carm_send_special (struct car
crq->msg_bucket = (u32) rc;
DPRINTK("blk_insert_request, tag == %u\n", idx);
- blk_insert_request(host->oob_q, crq->rq, 1, crq, 0);
+ blk_insert_request(host->oob_q, crq->rq, 1, crq);
return 0;
}
Index: scsi-export/drivers/scsi/scsi_lib.c
===
--- scsi-export.orig/drivers/scsi/scsi_lib.c2005-03-31 17:42:05.0
+0900
+++ scsi-export/drivers/scsi/scsi_lib.c 2005-03-31 18:06:19.0 +0900
@@ -65,6 +65,109 @@ struct scsi_host_sg_pool scsi_sg_pools[]
/*
+ * Called for single_lun devices on IO completion. Clear starget_sdev_user,
+ * and call blk_run_queue for all the scsi_devices on the target -
+ * including current_sdev first.
+ *
+ * Called with *no* scsi locks held.
+ */
+static void scsi_single_lun_run(struct scsi_device *current_sdev)
+{
+ struct Scsi_Host *shost = current_sdev->host;
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
01_scsi_no_REQ_SPECIAL_on_requeue.patch
blk_insert_request() has 'reinsert' argument, which, when set,
turns on REQ_SPECIAL and REQ_SOFTBARRIER and requeues the
request. SCSI midlayer was the only user of this feature and
all requeued requests become special requests defeating
quiesce state. This patch makes scsi midlayer use
blk_requeue_request() for requeueing and removes 'reinsert'
feature from blk_insert_request().
Note: In drivers/scsi/scsi_lib.c, scsi_single_lun_run() and
scsi_run_queue() are moved upward unchanged.
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
drivers/block/ll_rw_blk.c | 20 +--
drivers/block/paride/pd.c |2
drivers/block/sx8.c |4
drivers/scsi/scsi_lib.c | 238 +++---
include/linux/blkdev.h|2
5 files changed, 133 insertions(+), 133 deletions(-)
Index: scsi-export/drivers/block/ll_rw_blk.c
===
--- scsi-export.orig/drivers/block/ll_rw_blk.c 2005-03-31 17:42:05.0
+0900
+++ scsi-export/drivers/block/ll_rw_blk.c 2005-03-31 18:06:19.0
+0900
@@ -2028,7 +2028,6 @@ EXPORT_SYMBOL(blk_requeue_request);
* @rq:request to be inserted
* @at_head: insert request at head or tail of queue
* @data: private data
- * @reinsert: true if request it a reinsertion of previously processed one
*
* Description:
*Many block devices need to execute commands asynchronously, so they don't
@@ -2043,8 +2042,9 @@ EXPORT_SYMBOL(blk_requeue_request);
*host that is unable to accept a particular command.
*/
void blk_insert_request(request_queue_t *q, struct request *rq,
- int at_head, void *data, int reinsert)
+ int at_head, void *data)
{
+ int where = at_head ? ELEVATOR_INSERT_FRONT : ELEVATOR_INSERT_BACK;
unsigned long flags;
/*
@@ -2061,20 +2061,12 @@ void blk_insert_request(request_queue_t
/*
* If command is tagged, release the tag
*/
- if (reinsert)
- blk_requeue_request(q, rq);
- else {
- int where = ELEVATOR_INSERT_BACK;
+ if (blk_rq_tagged(rq))
+ blk_queue_end_tag(q, rq);
- if (at_head)
- where = ELEVATOR_INSERT_FRONT;
+ drive_stat_acct(rq, rq-nr_sectors, 1);
+ __elv_add_request(q, rq, where, 0);
- if (blk_rq_tagged(rq))
- blk_queue_end_tag(q, rq);
-
- drive_stat_acct(rq, rq-nr_sectors, 1);
- __elv_add_request(q, rq, where, 0);
- }
if (blk_queue_plugged(q))
__generic_unplug_device(q);
else
Index: scsi-export/drivers/block/paride/pd.c
===
--- scsi-export.orig/drivers/block/paride/pd.c 2005-03-31 17:42:05.0
+0900
+++ scsi-export/drivers/block/paride/pd.c 2005-03-31 18:06:19.0
+0900
@@ -723,7 +723,7 @@ static int pd_special_command(struct pd_
rq.ref_count = 1;
rq.waiting = wait;
rq.end_io = blk_end_sync_rq;
- blk_insert_request(disk-gd-queue, rq, 0, func, 0);
+ blk_insert_request(disk-gd-queue, rq, 0, func);
wait_for_completion(wait);
rq.waiting = NULL;
if (rq.errors)
Index: scsi-export/drivers/block/sx8.c
===
--- scsi-export.orig/drivers/block/sx8.c2005-03-31 17:42:05.0
+0900
+++ scsi-export/drivers/block/sx8.c 2005-03-31 18:06:19.0 +0900
@@ -614,7 +614,7 @@ static int carm_array_info (struct carm_
spin_unlock_irq(host-lock);
DPRINTK(blk_insert_request, tag == %u\n, idx);
- blk_insert_request(host-oob_q, crq-rq, 1, crq, 0);
+ blk_insert_request(host-oob_q, crq-rq, 1, crq);
return 0;
@@ -653,7 +653,7 @@ static int carm_send_special (struct car
crq-msg_bucket = (u32) rc;
DPRINTK(blk_insert_request, tag == %u\n, idx);
- blk_insert_request(host-oob_q, crq-rq, 1, crq, 0);
+ blk_insert_request(host-oob_q, crq-rq, 1, crq);
return 0;
}
Index: scsi-export/drivers/scsi/scsi_lib.c
===
--- scsi-export.orig/drivers/scsi/scsi_lib.c2005-03-31 17:42:05.0
+0900
+++ scsi-export/drivers/scsi/scsi_lib.c 2005-03-31 18:06:19.0 +0900
@@ -65,6 +65,109 @@ struct scsi_host_sg_pool scsi_sg_pools[]
/*
+ * Called for single_lun devices on IO completion. Clear starget_sdev_user,
+ * and call blk_run_queue for all the scsi_devices on the target -
+ * including current_sdev first.
+ *
+ * Called with *no* scsi locks held.
+ */
+static void scsi_single_lun_run(struct scsi_device *current_sdev)
+{
+ struct Scsi_Host *shost = current_sdev-host;
+
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
On Thu, Mar 31, 2005 at 06:07:55PM +0900, Tejun Heo wrote: 01_scsi_no_REQ_SPECIAL_on_requeue.patch blk_insert_request() has 'reinsert' argument, which, when set, turns on REQ_SPECIAL and REQ_SOFTBARRIER and requeues the request. SCSI midlayer was the only user of this feature and all requeued requests become special requests defeating quiesce state. This patch makes scsi midlayer use blk_requeue_request() for requeueing and removes 'reinsert' feature from blk_insert_request(). Note: In drivers/scsi/scsi_lib.c, scsi_single_lun_run() and scsi_run_queue() are moved upward unchanged. That lest part doesn't belong into this patch at all. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
On Thu, 2005-03-31 at 18:07 +0900, Tejun Heo wrote: 01_scsi_no_REQ_SPECIAL_on_requeue.patch blk_insert_request() has 'reinsert' argument, which, when set, turns on REQ_SPECIAL and REQ_SOFTBARRIER and requeues the request. SCSI midlayer was the only user of this feature and all requeued requests become special requests defeating quiesce state. This patch makes scsi midlayer use blk_requeue_request() for requeueing and removes 'reinsert' feature from blk_insert_request(). Well, REQ_SPECIAL is the signal to the mid-layer that we've allocated the resources necessary to process the command, so in practice it will be turned on for every requeue request (because we set it when the command is prepared), so this patch would have no effect on your stated quiesce problem. However, if you think about how requests work with head insertion and one command following another, there's really not a huge problem here either. The other reason I don't like this is that we've been trying hard to sweep excess block knowledge out of the mid-layer. I don't think REQ_SOFTBARRIER is anything we really have to know about. James - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
On Thu, Mar 31, 2005 at 11:12:11AM +0100, Christoph Hellwig wrote: On Thu, Mar 31, 2005 at 06:07:55PM +0900, Tejun Heo wrote: 01_scsi_no_REQ_SPECIAL_on_requeue.patch blk_insert_request() has 'reinsert' argument, which, when set, turns on REQ_SPECIAL and REQ_SOFTBARRIER and requeues the request. SCSI midlayer was the only user of this feature and all requeued requests become special requests defeating quiesce state. This patch makes scsi midlayer use blk_requeue_request() for requeueing and removes 'reinsert' feature from blk_insert_request(). Note: In drivers/scsi/scsi_lib.c, scsi_single_lun_run() and scsi_run_queue() are moved upward unchanged. That lest part doesn't belong into this patch at all. Actually, it is, as scsi_queue_insert() is changed to call scsi_run_queue() explicitly. However, scsi_queue_insert() is removed later, so the change is pretty dumb. Maybe I'll add prototype and remove it together later, or reorder patches. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH scsi-misc-2.6 01/13] scsi: don't use blk_insert_request() for requeueing
Hello, James. On Thu, Mar 31, 2005 at 11:53:20AM -0600, James Bottomley wrote: On Thu, 2005-03-31 at 18:07 +0900, Tejun Heo wrote: 01_scsi_no_REQ_SPECIAL_on_requeue.patch blk_insert_request() has 'reinsert' argument, which, when set, turns on REQ_SPECIAL and REQ_SOFTBARRIER and requeues the request. SCSI midlayer was the only user of this feature and all requeued requests become special requests defeating quiesce state. This patch makes scsi midlayer use blk_requeue_request() for requeueing and removes 'reinsert' feature from blk_insert_request(). Well, REQ_SPECIAL is the signal to the mid-layer that we've allocated the resources necessary to process the command, so in practice it will be turned on for every requeue request (because we set it when the command is prepared), Sorry, but where? AFAICT, only blk_insert_request() and scsi_init_io() on sgtable allocation failure set REQ_SPECIAL during scsi midlayer processing. This patch replaces blk_insert_request() with blk_requeue_request() and the next patch removes REQ_SPECIAL setting in scsi_init_io(). REQ_SPECIAL is currently overloaded to mean two different things. * The request is a special request. * The request has been requeued using scsi_queue_insert(). i.e. It has been prepped. However, #2 can be tested by rq-special != NULL condition, and, in fact, the prep_fn already has the code. This is why this and the next patch don't break the requeue path. IMHO, this proves the subtlety of the REQ_SPECIAL semantics. Which path is taken on which case gets kind of confusing by meaning two different things with REQ_SPECIAL. so this patch would have no effect on your stated quiesce problem. However, if you think about how requests work with head insertion and one command following another, there's really not a huge problem here either. I agree that it's not a huge problem, but it's subtle and has the potential of causing probably non-destructive but confusing behavior on rare cases. The other reason I don't like this is that we've been trying hard to sweep excess block knowledge out of the mid-layer. I don't think REQ_SOFTBARRIER is anything we really have to know about. We currently requeue using two different block functions. * blk_insert_request(): this function does two things 1. enqueue special requests 2. turn on REQ_SPECIAL|REQ_SOFTBARRIER and call blk_requeue_request(). This is used only by scsi midlayer. * blk_requeue_request() REQ_SOFTBARRIER tells the request queue that other requests shouldn't pass this one. We need this to be set for prepped requests; otherwise, it may, theoretically, deadlock (unprepped request waiting for the prepped request back in the queue). So, the current code * depends on blk_insert_request() sets REQ_SOFTBARRIER when requeueing. It works but nothing in the interface or semantics is clear about it. Why expect a request reinserted using blk_insert_request() gets REQ_SOFTBARRIER turned on while a request reinserted with blk_requeue_request() doesn't? or why are there two different paths doing mostly the same thing with slightly different semantics? * requeueing using blk_requeue_request() in scsi_request_fn() doesn't turn on either REQ_SPECIAL or REQ_SOFTBARRIER. Missing REQ_SPECIAL is okay, as we have the extra path in prep_fn (if it ever gets requeued due to medium failure, so gets re-prepped), but missing REQ_SOFTBARRIER can *theoretically* cause problem. So, it's more likely becoming less dependent on unobvious behavior of block layer. As we need the request to stay on top, we tell the block layer to do so, instead of depending on blk_insert_request() unobviously doing it for us. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
