Hi James, Jens, On Wed, 12 Dec 2007 07:53:36 -0500, James Bottomley wrote: > On Tue, 2007-12-11 at 17:40 -0500, Kiyoshi Ueda wrote: > > This patch adds 2 new interfaces for request completion: > > o blk_end_request() : called without queue lock > > o __blk_end_request() : called with queue lock held > > > > blk_end_request takes 'error' as an argument instead of 'uptodate', > > which current end_that_request_* take. > > The meanings of values are below and the value is used when bio is > > completed. > > 0 : success > > < 0 : error > > > > Some device drivers call some generic functions below between > > end_that_request_{first/chunk} and end_that_request_last(). > > o add_disk_randomness() > > o blk_queue_end_tag() > > o blkdev_dequeue_request() > > If we can roll the whole thing together, that would be nice. However, > the way you're doing it with this patch, we now have an asymmetrical > interface: The request routine must explicitly start the tag, but now > doesn't have to end it. > > We really need symmetry. Either go back to start tag/end tag, or absorb > the whole lot into the block infrastructure. > > The original reason for the explicit start/end is that there are some > requests on a tagged device that aren't able to be tagged by the block > layer (some devices reserve tag numbers for specific meanings). > However, I don't think there's any driver that actually implemented this > feature. As far as I investigated in 2.6.24-rc5, only scsi uses the blk_queue_tag and no files in drivers/scsi reserving tag_index in Scsi_Host->bqt. So I would like to take the "absorbing start tag in the block layer" way. The patch below is on top of the blk_end_request patch-set. Is it acceptable? Thanks, Kiyoshi Ueda Subject: [PATCH 31/31] blk_end_request: merge start_tag to block layer This patch merges blk_queue_start_tag() into blkdev_dequeue_request(). blk_queue_start_tag() and blk_queue_end_tag() are a pair of interfaces for starting/ending request tagging. Since with the new blk_end_request interfaces, blk_queue_end_tag() is done implicitly in the block layer, blk_queue_start_tag() should be done in the block layer, too, for keeping the interface symmetric. Originally, the start/end tag was not done by the block layer so that drivers can choose tag numbers for their specific meanings. But no driver uses the feature. Scsi is the only user of the block layer tagging and it uses the generic blk_queue_start/end_tag. So moving the start/end tag to the block layer is not an issue now. Cc: James Bottomley <James.Bottomley@xxxxxxxxxxxx> Signed-off-by: Kiyoshi Ueda <k-ueda@xxxxxxxxxxxxx> Signed-off-by: Jun'ichi Nomura <j-nomura@xxxxxxxxxxxxx> --- block/ll_rw_blk.c | 2 +- drivers/scsi/scsi_lib.c | 3 +-- include/linux/blkdev.h | 11 ++++++----- 3 files changed, 8 insertions(+), 8 deletions(-) Index: 2.6.24-rc5/block/ll_rw_blk.c =================================================================== --- 2.6.24-rc5.orig/block/ll_rw_blk.c +++ 2.6.24-rc5/block/ll_rw_blk.c @@ -1115,7 +1115,7 @@ int blk_queue_start_tag(struct request_q rq->cmd_flags |= REQ_QUEUED; rq->tag = tag; bqt->tag_index[tag] = rq; - blkdev_dequeue_request(rq); + elv_dequeue_request(q, rq); list_add(&rq->queuelist, &q->tag_busy_list); bqt->busy++; return 0; Index: 2.6.24-rc5/drivers/scsi/scsi_lib.c =================================================================== --- 2.6.24-rc5.orig/drivers/scsi/scsi_lib.c +++ 2.6.24-rc5/drivers/scsi/scsi_lib.c @@ -1530,8 +1530,7 @@ static void scsi_request_fn(struct reque /* * Remove the request from the request list. */ - if (!(blk_queue_tagged(q) && !blk_queue_start_tag(q, req))) - blkdev_dequeue_request(req); + blkdev_dequeue_request(req); sdev->device_busy++; spin_unlock(q->queue_lock); Index: 2.6.24-rc5/include/linux/blkdev.h =================================================================== --- 2.6.24-rc5.orig/include/linux/blkdev.h +++ 2.6.24-rc5/include/linux/blkdev.h @@ -747,11 +747,6 @@ extern void blk_complete_request(struct extern unsigned int blk_rq_bytes(struct request *rq); extern unsigned int blk_rq_cur_bytes(struct request *rq); -static inline void blkdev_dequeue_request(struct request *req) -{ - elv_dequeue_request(req->q, req); -} - /* * Access functions for manipulating queue properties */ @@ -814,6 +809,12 @@ static inline struct request *blk_map_qu return bqt->tag_index[tag]; } +static inline void blkdev_dequeue_request(struct request *req) +{ + if (!(blk_queue_tagged(req->q) && !blk_queue_start_tag(req->q, req))) + elv_dequeue_request(req->q, req); +} + extern int blkdev_issue_flush(struct block_device *, sector_t *); #define MAX_PHYS_SEGMENTS 128 - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html