Hi Kiyoshi,
Kiyoshi Ueda wrote:
> This patch adds ->complete_io() hook for request stacking.
> Request stacking drivers (such as request-based dm) can set
> a callback for completion.
> (The hook is not called in blk_end_io(), since request-based dm uses
> it for clone completion in the following appendix patches.)
>
[ .. ]
I would rather have rq->complete_io() to be pointing to blk_end_io in the
default case, this way rq->complete_io() would always be valid and we
would be saving us the if() clause.
> Index: 2.6.25-rc1/block/blk-core.c
> ===================================================================
> --- 2.6.25-rc1.orig/block/blk-core.c
> +++ 2.6.25-rc1/block/blk-core.c
> @@ -138,6 +138,7 @@ void rq_init(struct request_queue *q, st
> rq->data = NULL;
> rq->sense = NULL;
> rq->end_io = NULL;
> + rq->complete_io = NULL;
rq->complete_io = blk_end_io;
> rq->end_io_data = NULL;
> rq->next_rq = NULL;
> }
> @@ -1917,6 +1918,9 @@ static int blk_end_io(struct request *rq
> **/
> int blk_end_request(struct request *rq, int error, unsigned int nr_bytes)
> {
> + if (rq->complete_io)
> + return rq->complete_io(rq, error, nr_bytes, 0, NULL);
> +
> return blk_end_io(rq, error, nr_bytes, 0, NULL);
> }
So when using my proposal this would just become:
{
BUG_ON(!rq->complete_io);
return rq->complete_io(rq, error, nr_bytes, 0, NULL);
}
> EXPORT_SYMBOL_GPL(blk_end_request);
> @@ -1936,6 +1940,27 @@ EXPORT_SYMBOL_GPL(blk_end_request);
> **/
> int __blk_end_request(struct request *rq, int error, unsigned int nr_bytes)
> {
> + if (unlikely(blk_queue_stackable(rq->q)))
> + printk(KERN_WARNING "dev: %s: Not ready for request stacking, "
> + "but the device driver set it stackable. "
> + "Need to fix the device driver!\n",
> + rq->rq_disk ? rq->rq_disk->disk_name : "?");
> +
> + if (unlikely(rq->complete_io)) {
> + /*
> + * If we invoke the ->complete_io here, the request submitter's
> + * handler would get deadlock on the queue lock.
> + *
> + * This happens, when the request submitter didn't check
> + * whether the queue is stackable or the device driver marked
> + * the queue stackable.
> + * Need to fix the request submitter or the device driver.
> + */
> + printk(KERN_ERR "The device driver isn't ready for "
> + "request stacking.\n");
> + BUG();
> + }
> +
This could be skipped entirely then.
> if (blk_fs_request(rq) || blk_pc_request(rq)) {
> if (__end_that_request_first(rq, error, nr_bytes))
> return 1;
> @@ -1966,6 +1991,9 @@ EXPORT_SYMBOL_GPL(__blk_end_request);
> int blk_end_bidi_request(struct request *rq, int error, unsigned int nr_bytes,
> unsigned int bidi_bytes)
> {
> + if (rq->complete_io)
> + return rq->complete_io(rq, error, nr_bytes, bidi_bytes, NULL);
> +
> return blk_end_io(rq, error, nr_bytes, bidi_bytes, NULL);
> }
return rq->complete_io(rq, error, nr_bytes, bidi_bytes, NULL);
> EXPORT_SYMBOL_GPL(blk_end_bidi_request);
> @@ -1999,6 +2027,9 @@ int blk_end_request_callback(struct requ
> unsigned int nr_bytes,
> int (drv_callback)(struct request *))
> {
> + if (rq->complete_io)
> + return rq->complete_io(rq, error, nr_bytes, 0, drv_callback);
> +
> return blk_end_io(rq, error, nr_bytes, 0, drv_callback);
return rq->complete_io(rq, error, nr_bytes, bidi_bytes, NULL);
> }
> EXPORT_SYMBOL_GPL(blk_end_request_callback);
> Index: 2.6.25-rc1/include/linux/blkdev.h
> ===================================================================
> --- 2.6.25-rc1.orig/include/linux/blkdev.h
> +++ 2.6.25-rc1/include/linux/blkdev.h
> @@ -42,6 +42,9 @@ void copy_io_context(struct io_context *
>
> struct request;
> typedef void (rq_end_io_fn)(struct request *, int);
> +typedef int (rq_complete_io_fn)(struct request *rq, int error,
> + unsigned int nr_bytes, unsigned int bidi_bytes,
> + int (drv_callback)(struct request *));
>
> struct request_list {
> int count[2];
> @@ -228,6 +231,7 @@ struct request {
> * completion callback.
> */
> rq_end_io_fn *end_io;
> + rq_complete_io_fn *complete_io;
> void *end_io_data;
>
> /* for bidi */
> @@ -401,6 +405,7 @@ struct request_queue
> #define QUEUE_FLAG_PLUGGED 7 /* queue is plugged */
> #define QUEUE_FLAG_ELVSWITCH 8 /* don't use elevator, just do FIFO */
> #define QUEUE_FLAG_BIDI 9 /* queue supports bidi requests */
> +#define QUEUE_FLAG_STACKABLE 10 /* queue supports request stacking */
>
> enum {
> /*
> @@ -446,6 +451,8 @@ enum {
> #define blk_queue_tagged(q) test_bit(QUEUE_FLAG_QUEUED, &(q)->queue_flags)
> #define blk_queue_stopped(q) test_bit(QUEUE_FLAG_STOPPED, &(q)->queue_flags)
> #define blk_queue_flushing(q) ((q)->ordseq)
> +#define blk_queue_stackable(q) \
> + test_bit(QUEUE_FLAG_STACKABLE, &(q)->queue_flags)
>
> #define blk_fs_request(rq) ((rq)->cmd_type == REQ_TYPE_FS)
> #define blk_pc_request(rq) ((rq)->cmd_type == REQ_TYPE_BLOCK_PC)
> Index: 2.6.25-rc1/drivers/scsi/scsi_lib.c
> ===================================================================
> --- 2.6.25-rc1.orig/drivers/scsi/scsi_lib.c
> +++ 2.6.25-rc1/drivers/scsi/scsi_lib.c
> @@ -1597,6 +1597,13 @@ struct request_queue *__scsi_alloc_queue
> */
> blk_queue_dma_alignment(q, 0x03);
>
> + /*
> + * SCSI is ready for request stacking, since it doesn't hold
> + * queue lock when completing request.
> + * (It doesn't use __blk_end_request().)
> + */
> + set_bit(QUEUE_FLAG_STACKABLE, &q->queue_flags);
> +
> return q;
> }
> EXPORT_SYMBOL(__scsi_alloc_queue);
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Apart from this: Great work!
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
