[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110411165923.GA15442@infradead.org>
Date: Mon, 11 Apr 2011 12:59:23 -0400
From: "hch@...radead.org" <hch@...radead.org>
To: NeilBrown <neilb@...e.de>
Cc: Mike Snitzer <snitzer@...hat.com>,
Jens Axboe <jaxboe@...ionio.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"hch@...radead.org" <hch@...radead.org>, dm-devel@...hat.com,
linux-raid@...r.kernel.org
Subject: Re: [PATCH 05/10] block: remove per-queue plugging
On Mon, Apr 11, 2011 at 02:50:22PM +1000, NeilBrown wrote:
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 273d60b..903ce8d 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -2674,19 +2674,23 @@ static void flush_plug_list(struct blk_plug *plug)
> struct request_queue *q;
> unsigned long flags;
> struct request *rq;
> + struct list_head head;
>
> BUG_ON(plug->magic != PLUG_MAGIC);
>
> if (list_empty(&plug->list))
> return;
> + list_add(&head, &plug->list);
> + list_del_init(&plug->list);
>
> if (plug->should_sort)
> - list_sort(NULL, &plug->list, plug_rq_cmp);
> + list_sort(NULL, &head, plug_rq_cmp);
> + plug->should_sort = 0;
As Jens mentioned this should be list_splice_init. But looking over
flush_plug_list the code there seems strange to me.
What does the local_irq_save in flush_plug_list protect? Why don't
we need it over the list_sort? And do we still need it when first
splicing the list to a local one?
It's one of these cases where I'd really like to see more comments
explaining why the code is doing what it's doing.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists