On Thu 30-01-14 18:01:20, Frederic Weisbecker wrote:
> On Thu, Jan 30, 2014 at 04:45:23PM +0100, Jan Kara wrote:
> > Hi,
> >
> > On Thu 30-01-14 13:39:18, Frederic Weisbecker wrote:
> > > I'm currently working on some cleanups on IPI code too and working on top
> > > of these patches, just have a few comments:
> > Great, thanks!
> >
> > > On Mon, Dec 23, 2013 at 09:39:23PM +0100, Jan Kara wrote:
> > > > Abusing rq->csd.list for a list of requests to complete is rather ugly.
> > > > Especially since using queuelist should be safe and much cleaner.
> > >
> > > It would be nice to have a few more details that explain why doing so is
> > > safe
> > > wrt a block request lifecycle. At least something that tells why
> > > rq->queuelist
> > > can't be ever used concurrently by the time we send the IPI and we
> > > trigger/raise
> > > the softirq.
> > Sure. Should I send the patch to you with an updated changelog and added
> > comment you requested?
>
> Yeah that would be nice!
OK, the updated patch is attached.
> For more precision, I would like to apply these:
>
> 1) block: Stop abusing csd.list for fifo_time
> 2) block: Stop abusing rq->csd.list in blk-softirq
> 3) kernel: use lockless list for smp_call_function_single()
> 4) smp: Teach __smp_call_function_single() to check for offline cpus
>
> This is because I need to tweak a bit the IPI code for some nohz
> functionnality. I'm not sure about the result but in any case these
> llist based cleanups look very nice, so lets try to push them. I'm also
> working on some consolidation between __smp_call_function_single()
> and smp_call_function_single() since they share almost the same code.
>
> Also this shouldn't conflict with Andrew's tree if he applies these as well
> since -mm is based on -next. And the printk part should still go through his
> tree I think.
Sure, that should be no problem. Jens might have the patch somewhere in
the linux-block.git but any conflict should be easy to resolve.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
>From a7782fa4cee73a0581eded978eacf52cb0db1ec7 Mon Sep 17 00:00:00 2001
From: Jan Kara <[email protected]>
Date: Tue, 17 Dec 2013 23:47:41 +0100
Subject: [PATCH] block: Stop abusing rq->csd.list in blk-softirq
Abusing rq->csd.list for a list of requests to complete is rather ugly.
We use rq->queuelist instead which is much cleaner. It is safe because
queuelist is used by the block layer only for requests waiting to be
submitted to a device. Thus it is unused when irq reports the request IO
is finished.
Signed-off-by: Jan Kara <[email protected]>
---
block/blk-softirq.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/block/blk-softirq.c b/block/blk-softirq.c
index 57790c1a97eb..b5c37d96cf0e 100644
--- a/block/blk-softirq.c
+++ b/block/blk-softirq.c
@@ -30,8 +30,8 @@ static void blk_done_softirq(struct softirq_action *h)
while (!list_empty(&local_list)) {
struct request *rq;
- rq = list_entry(local_list.next, struct request, csd.list);
- list_del_init(&rq->csd.list);
+ rq = list_entry(local_list.next, struct request, queuelist);
+ list_del_init(&rq->queuelist);
rq->q->softirq_done_fn(rq);
}
}
@@ -45,9 +45,14 @@ static void trigger_softirq(void *data)
local_irq_save(flags);
list = this_cpu_ptr(&blk_cpu_done);
- list_add_tail(&rq->csd.list, list);
+ /*
+ * We reuse queuelist for a list of requests to process. Since the
+ * queuelist is used by the block layer only for requests waiting to be
+ * submitted to the device it is unused now.
+ */
+ list_add_tail(&rq->queuelist, list);
- if (list->next == &rq->csd.list)
+ if (list->next == &rq->queuelist)
raise_softirq_irqoff(BLOCK_SOFTIRQ);
local_irq_restore(flags);
@@ -136,7 +141,7 @@ void __blk_complete_request(struct request *req)
struct list_head *list;
do_local:
list = this_cpu_ptr(&blk_cpu_done);
- list_add_tail(&req->csd.list, list);
+ list_add_tail(&req->queuelist, list);
/*
* if the list only contains our just added request,
@@ -144,7 +149,7 @@ do_local:
* entries there, someone already raised the irq but it
* hasn't run yet.
*/
- if (list->next == &req->csd.list)
+ if (list->next == &req->queuelist)
raise_softirq_irqoff(BLOCK_SOFTIRQ);
} else if (raise_blk_irq(ccpu, req))
goto do_local;
--
1.8.1.4
