[PATCH V2] umem: fix up unplugging

[PATCH V2] umem: fix up unplugging

[Tao Guo]

Fix a regression introduced by 7eaceaccab5f40 ("block: remove per-queue
plugging").  In that patch, Jens removed the whole mm_unplug_device()
function, which used to be the trigger to make umem start to work.

We need to implement unplugging to make umem start to work, or I/O will
never be triggered.

Signed-off-by: Tao Guo <Tao.Guo@emc.com>
Cc: Neil Brown <neilb@suse.de>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Shaohua Li <shli@kernel.org>
Cc: <stable@vger.kernel.org>
---
 drivers/block/umem.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/drivers/block/umem.c b/drivers/block/umem.c
index aa27120..9a72277 100644
--- a/drivers/block/umem.c
+++ b/drivers/block/umem.c
@@ -513,6 +513,44 @@ static void process_page(unsigned long data)
 	}
 }
 
+struct mm_plug_cb {
+	struct blk_plug_cb cb;
+	struct cardinfo *card;
+};
+
+static void mm_unplug(struct blk_plug_cb *cb)
+{
+	struct mm_plug_cb *mmcb = container_of(cb, struct mm_plug_cb, cb);
+
+	spin_lock_irq(&mmcb->card->lock);
+	activate(mmcb->card);
+	spin_unlock_irq(&mmcb->card->lock);
+	kfree(mmcb);
+}
+
+static int mm_check_plugged(struct cardinfo *card)
+{
+	struct blk_plug *plug = current->plug;
+	struct mm_plug_cb *mmcb;
+
+	if (!plug)
+		return 0;
+
+	list_for_each_entry(mmcb, &plug->cb_list, cb.list) {
+		if (mmcb->cb.callback == mm_unplug && mmcb->card == card)
+			return 1;
+	}
+	/* Not currently on the callback list */
+	mmcb = kmalloc(sizeof(*mmcb), GFP_ATOMIC);
+	if (!mmcb)
+		return 0;
+
+	mmcb->card = card;
+	mmcb->cb.callback = mm_unplug;
+	list_add(&mmcb->cb.list, &plug->cb_list);
+	return 1;
+}
+
 static void mm_make_request(struct request_queue *q, struct bio *bio)
 {
 	struct cardinfo *card = q->queuedata;
@@ -523,6 +561,8 @@ static void mm_make_request(struct request_queue *q, struct bio *bio)
 	*card->biotail = bio;
 	bio->bi_next = NULL;
 	card->biotail = &bio->bi_next;
+	if (bio->bi_rw & REQ_SYNC || !mm_check_plugged(card))
+		activate(card);
 	spin_unlock_irq(&card->lock);
 
 	return;
-- 
1.7.7.6

Re: [PATCH V2] umem: fix up unplugging

[NeilBrown]

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

On Thu,  7 Jun 2012 13:07:15 -0400 Tao Guo <glorioustao@gmail.com> wrote:

> Fix a regression introduced by 7eaceaccab5f40 ("block: remove per-queue
> plugging").  In that patch, Jens removed the whole mm_unplug_device()
> function, which used to be the trigger to make umem start to work.
> 
> We need to implement unplugging to make umem start to work, or I/O will
> never be triggered.
> 
> Signed-off-by: Tao Guo <Tao.Guo@emc.com>
> Cc: Neil Brown <neilb@suse.de>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Shaohua Li <shli@kernel.org>
> Cc: <stable@vger.kernel.org>

This is certainly a simpler patch that the first 2 and that it important if
it is heading for 3.4.y.
It looks like it will do what it should so assuming you have tested it (which
I'm sure you have)

  Acked-by: NeilBrown <neilb@suse.de>

Longer term I think it does make sense to move some of this plugging code
into block/blk-XXX so that it can be shared by umem and md, and improved to
serve them both better.

In particular I think that the _check_plugged function should return the 
struct xx_plug_cb structure (rather than 'true' or 'false') and the
make_request function should then link any new requests into that structure.
The unplug function can then activate that list of requests in whatever way
suits the particular device.
This will ensure that if separate threads are submitting requests at the same
time, they won't compete with each other, and an unplug on one thread won't
release the requests that are plugged by the other.

NeilBrown

> ---
>  drivers/block/umem.c |   40 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/block/umem.c b/drivers/block/umem.c
> index aa27120..9a72277 100644
> --- a/drivers/block/umem.c
> +++ b/drivers/block/umem.c
> @@ -513,6 +513,44 @@ static void process_page(unsigned long data)
>  	}
>  }
>  
> +struct mm_plug_cb {
> +	struct blk_plug_cb cb;
> +	struct cardinfo *card;
> +};
> +
> +static void mm_unplug(struct blk_plug_cb *cb)
> +{
> +	struct mm_plug_cb *mmcb = container_of(cb, struct mm_plug_cb, cb);
> +
> +	spin_lock_irq(&mmcb->card->lock);
> +	activate(mmcb->card);
> +	spin_unlock_irq(&mmcb->card->lock);
> +	kfree(mmcb);
> +}
> +
> +static int mm_check_plugged(struct cardinfo *card)
> +{
> +	struct blk_plug *plug = current->plug;
> +	struct mm_plug_cb *mmcb;
> +
> +	if (!plug)
> +		return 0;
> +
> +	list_for_each_entry(mmcb, &plug->cb_list, cb.list) {
> +		if (mmcb->cb.callback == mm_unplug && mmcb->card == card)
> +			return 1;
> +	}
> +	/* Not currently on the callback list */
> +	mmcb = kmalloc(sizeof(*mmcb), GFP_ATOMIC);
> +	if (!mmcb)
> +		return 0;
> +
> +	mmcb->card = card;
> +	mmcb->cb.callback = mm_unplug;
> +	list_add(&mmcb->cb.list, &plug->cb_list);
> +	return 1;
> +}
> +
>  static void mm_make_request(struct request_queue *q, struct bio *bio)
>  {
>  	struct cardinfo *card = q->queuedata;
> @@ -523,6 +561,8 @@ static void mm_make_request(struct request_queue *q, struct bio *bio)
>  	*card->biotail = bio;
>  	bio->bi_next = NULL;
>  	card->biotail = &bio->bi_next;
> +	if (bio->bi_rw & REQ_SYNC || !mm_check_plugged(card))
> +		activate(card);
>  	spin_unlock_irq(&card->lock);
>  
>  	return;


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH V2] umem: fix up unplugging

[Jens Axboe]

On Thu, Jun 07 2012, Tao Guo wrote:
> Fix a regression introduced by 7eaceaccab5f40 ("block: remove per-queue
> plugging").  In that patch, Jens removed the whole mm_unplug_device()
> function, which used to be the trigger to make umem start to work.
> 
> We need to implement unplugging to make umem start to work, or I/O will
> never be triggered.

Applied this one, dropped the other two. Would still be nice to move
this generic functionality into the block core and unify md/umem in this
regard for 3.6, if you would like to do that.

-- 
Jens Axboe

Re: [PATCH V2] umem: fix up unplugging

[Tao Guo]

Thanks Jens. I can do that for 3.6 kernel.
-Tao

On Thu, Jun 14, 2012 at 3:18 AM, Jens Axboe <jaxboe@fusionio.com> wrote:
> On Thu, Jun 07 2012, Tao Guo wrote:
>> Fix a regression introduced by 7eaceaccab5f40 ("block: remove per-queue
>> plugging").  In that patch, Jens removed the whole mm_unplug_device()
>> function, which used to be the trigger to make umem start to work.
>>
>> We need to implement unplugging to make umem start to work, or I/O will
>> never be triggered.
>
> Applied this one, dropped the other two. Would still be nice to move
> this generic functionality into the block core and unify md/umem in this
> regard for 3.6, if you would like to do that.
>
> --
> Jens Axboe
>