[RFC PATCH 00/11] smp: Single IPI cleanups

[RFC PATCH 00/11] smp: Single IPI cleanups

[Frederic Weisbecker]

Hi,

Here is a batch of cleanups on the generic single IPI code. I've
gathered a few patches from Jan, split up some, and added a few more
so that __smp_call_function_single() becomes less obscure.

Comments?

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	core/ipi-rfc

Thanks,
	Frederic
---

Frederic Weisbecker (6):
      block: Remove useless IPI struct initialization
      smp: Consolidate the various smp_call_function_single() declensions
      smp: Move __smp_call_function_single() below its safe version
      watchdog: Simplify a little the IPI call
      smp: Remove wait argument from __smp_call_function_single()
      smp: Enhance and precise the role & requirements of __smp_call_function_single()

Jan Kara (5):
      block: Stop abusing csd.list for fifo_time
      block: Stop abusing rq->csd.list in blk-softirq
      smp: Iterate functions through llist_for_each_entry_safe()
      smp: Remove unused list_head from csd
      smp: Teach __smp_call_function_single() to check for offline cpus


 block/blk-mq.c            |   2 +-
 block/blk-softirq.c       |  19 ++++---
 block/cfq-iosched.c       |   8 +--
 block/deadline-iosched.c  |   8 +--
 drivers/block/null_blk.c  |   2 +-
 drivers/cpuidle/coupled.c |   2 +-
 include/linux/blkdev.h    |   1 +
 include/linux/elevator.h  |  11 +---
 include/linux/smp.h       |   8 +--
 kernel/sched/core.c       |   2 +-
 kernel/smp.c              | 139 ++++++++++++++++++++++------------------------
 kernel/up.c               |   4 +-
 kernel/watchdog.c         |   3 +-
 net/core/dev.c            |   2 +-
 14 files changed, 97 insertions(+), 114 deletions(-)

[PATCH 01/11] block: Stop abusing csd.list for fifo_time

[Frederic Weisbecker]

From: Jan Kara <jack@suse.cz>

Block layer currently abuses rq->csd.list.next for storing fifo_time.
That is a terrible hack and completely unnecessary as well. Union
achieves the same space saving in a cleaner way.

Signed-off-by: Jan Kara <jack@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 block/cfq-iosched.c      | 8 ++++----
 block/deadline-iosched.c | 8 ++++----
 include/linux/blkdev.h   | 1 +
 include/linux/elevator.h | 6 ------
 4 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 744833b..5873e4a 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -2367,10 +2367,10 @@ cfq_merged_requests(struct request_queue *q, struct request *rq,
 	 * reposition in fifo if next is older than rq
 	 */
 	if (!list_empty(&rq->queuelist) && !list_empty(&next->queuelist) &&
-	    time_before(rq_fifo_time(next), rq_fifo_time(rq)) &&
+	    time_before(next->fifo_time, rq->fifo_time) &&
 	    cfqq == RQ_CFQQ(next)) {
 		list_move(&rq->queuelist, &next->queuelist);
-		rq_set_fifo_time(rq, rq_fifo_time(next));
+		rq->fifo_time = next->fifo_time;
 	}
 
 	if (cfqq->next_rq == next)
@@ -2814,7 +2814,7 @@ static struct request *cfq_check_fifo(struct cfq_queue *cfqq)
 		return NULL;
 
 	rq = rq_entry_fifo(cfqq->fifo.next);
-	if (time_before(jiffies, rq_fifo_time(rq)))
+	if (time_before(jiffies, rq->fifo_time))
 		rq = NULL;
 
 	cfq_log_cfqq(cfqq->cfqd, cfqq, "fifo=%p", rq);
@@ -3927,7 +3927,7 @@ static void cfq_insert_request(struct request_queue *q, struct request *rq)
 	cfq_log_cfqq(cfqd, cfqq, "insert_request");
 	cfq_init_prio_data(cfqq, RQ_CIC(rq));
 
-	rq_set_fifo_time(rq, jiffies + cfqd->cfq_fifo_expire[rq_is_sync(rq)]);
+	rq->fifo_time = jiffies + cfqd->cfq_fifo_expire[rq_is_sync(rq)];
 	list_add_tail(&rq->queuelist, &cfqq->fifo);
 	cfq_add_rq_rb(rq);
 	cfqg_stats_update_io_add(RQ_CFQG(rq), cfqd->serving_group,
diff --git a/block/deadline-iosched.c b/block/deadline-iosched.c
index 9ef6640..a753df2 100644
--- a/block/deadline-iosched.c
+++ b/block/deadline-iosched.c
@@ -106,7 +106,7 @@ deadline_add_request(struct request_queue *q, struct request *rq)
 	/*
 	 * set expire time and add to fifo list
 	 */
-	rq_set_fifo_time(rq, jiffies + dd->fifo_expire[data_dir]);
+	rq->fifo_time = jiffies + dd->fifo_expire[data_dir];
 	list_add_tail(&rq->queuelist, &dd->fifo_list[data_dir]);
 }
 
@@ -174,9 +174,9 @@ deadline_merged_requests(struct request_queue *q, struct request *req,
 	 * and move into next position (next will be deleted) in fifo
 	 */
 	if (!list_empty(&req->queuelist) && !list_empty(&next->queuelist)) {
-		if (time_before(rq_fifo_time(next), rq_fifo_time(req))) {
+		if (time_before(next->fifo_time, req->fifo_time)) {
 			list_move(&req->queuelist, &next->queuelist);
-			rq_set_fifo_time(req, rq_fifo_time(next));
+			req->fifo_time = next->fifo_time;
 		}
 	}
 
@@ -230,7 +230,7 @@ static inline int deadline_check_fifo(struct deadline_data *dd, int ddir)
 	/*
 	 * rq is expired!
 	 */
-	if (time_after_eq(jiffies, rq_fifo_time(rq)))
+	if (time_after_eq(jiffies, rq->fifo_time))
 		return 1;
 
 	return 0;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8678c43..dda98e3 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -99,6 +99,7 @@ struct request {
 	union {
 		struct call_single_data csd;
 		struct work_struct mq_flush_data;
+		unsigned long fifo_time;
 	};
 
 	struct request_queue *q;
diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index 306dd8c..0bdfd46 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -202,12 +202,6 @@ enum {
 #define rq_end_sector(rq)	(blk_rq_pos(rq) + blk_rq_sectors(rq))
 #define rb_entry_rq(node)	rb_entry((node), struct request, rb_node)
 
-/*
- * Hack to reuse the csd.list list_head as the fifo time holder while
- * the request is in the io scheduler. Saves an unsigned long in rq.
- */
-#define rq_fifo_time(rq)	((unsigned long) (rq)->csd.list.next)
-#define rq_set_fifo_time(rq,exp)	((rq)->csd.list.next = (void *) (exp))
 #define rq_entry_fifo(ptr)	list_entry((ptr), struct request, queuelist)
 #define rq_fifo_clear(rq)	do {		\
 	list_del_init(&(rq)->queuelist);	\
-- 
1.8.3.1

[PATCH 02/11] block: Remove useless IPI struct initialization

[Frederic Weisbecker]

rq_fifo_clear() reset the csd.list through INIT_LIST_HEAD for no clear
purpose. The csd.list doesn't need to be initialized as a list head
because it's only ever used as a list node.

Lets remove this useless initialization.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/elevator.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index 0bdfd46..df63bd3 100644
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -203,10 +203,7 @@ enum {
 #define rb_entry_rq(node)	rb_entry((node), struct request, rb_node)
 
 #define rq_entry_fifo(ptr)	list_entry((ptr), struct request, queuelist)
-#define rq_fifo_clear(rq)	do {		\
-	list_del_init(&(rq)->queuelist);	\
-	INIT_LIST_HEAD(&(rq)->csd.list);	\
-	} while (0)
+#define rq_fifo_clear(rq)	list_del_init(&(rq)->queuelist)
 
 #else /* CONFIG_BLOCK */
 
-- 
1.8.3.1

Re: [PATCH 02/11] block: Remove useless IPI struct initialization

[Jan Kara]

On Sat 08-02-14 17:18:31, Frederic Weisbecker wrote:
> rq_fifo_clear() reset the csd.list through INIT_LIST_HEAD for no clear
> purpose. The csd.list doesn't need to be initialized as a list head
> because it's only ever used as a list node.
> 
> Lets remove this useless initialization.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
  Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
> ---
>  include/linux/elevator.h | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/include/linux/elevator.h b/include/linux/elevator.h
> index 0bdfd46..df63bd3 100644
> --- a/include/linux/elevator.h
> +++ b/include/linux/elevator.h
> @@ -203,10 +203,7 @@ enum {
>  #define rb_entry_rq(node)	rb_entry((node), struct request, rb_node)
>  
>  #define rq_entry_fifo(ptr)	list_entry((ptr), struct request, queuelist)
> -#define rq_fifo_clear(rq)	do {		\
> -	list_del_init(&(rq)->queuelist);	\
> -	INIT_LIST_HEAD(&(rq)->csd.list);	\
> -	} while (0)
> +#define rq_fifo_clear(rq)	list_del_init(&(rq)->queuelist)
>  
>  #else /* CONFIG_BLOCK */
>  
> -- 
> 1.8.3.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH 03/11] block: Stop abusing rq->csd.list in blk-softirq

[Frederic Weisbecker]

From: Jan Kara <jack@suse.cz>

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 <jack@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 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 57790c1..b5c37d9 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.3.1

[PATCH 04/11] smp: Iterate functions through llist_for_each_entry_safe()

[Frederic Weisbecker]

From: Jan Kara <jack@suse.cz>

The IPI function llist iteration is open coded. Lets simplify this
with using an llist iterator.

Also we want to keep the iteration safe against possible
csd.llist->next value reuse from the IPI handler. At least the block
subsystem used to do such things so lets stay careful and use
llist_for_each_entry_safe().

Signed-off-by: Jan Kara <jack@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/smp.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/kernel/smp.c b/kernel/smp.c
index ffee35b..e3852de 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -151,7 +151,8 @@ static void generic_exec_single(int cpu, struct call_single_data *csd, int wait)
  */
 void generic_smp_call_function_single_interrupt(void)
 {
-	struct llist_node *entry, *next;
+	struct llist_node *entry;
+	struct call_single_data *csd, *csd_next;
 
 	/*
 	 * Shouldn't receive this interrupt on a cpu that is not yet online.
@@ -161,16 +162,9 @@ void generic_smp_call_function_single_interrupt(void)
 	entry = llist_del_all(&__get_cpu_var(call_single_queue));
 	entry = llist_reverse_order(entry);
 
-	while (entry) {
-		struct call_single_data *csd;
-
-		next = entry->next;
-
-		csd = llist_entry(entry, struct call_single_data, llist);
+	llist_for_each_entry_safe(csd, csd_next, entry, llist) {
 		csd->func(csd->info);
 		csd_unlock(csd);
-
-		entry = next;
 	}
 }
 
-- 
1.8.3.1

[PATCH 05/11] smp: Remove unused list_head from csd

[Frederic Weisbecker]

From: Jan Kara <jack@suse.cz>

Now that we got rid of all the remaining code which fiddled with csd.list,
lets remove it.

Signed-off-by: Jan Kara <jack@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/smp.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/linux/smp.h b/include/linux/smp.h
index 3834f43..9a1b8ba 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -17,10 +17,7 @@ extern void cpu_idle(void);
 
 typedef void (*smp_call_func_t)(void *info);
 struct call_single_data {
-	union {
-		struct list_head list;
-		struct llist_node llist;
-	};
+	struct llist_node llist;
 	smp_call_func_t func;
 	void *info;
 	u16 flags;
-- 
1.8.3.1

[PATCH 06/11] smp: Teach __smp_call_function_single() to check for offline cpus

[Frederic Weisbecker]

From: Jan Kara <jack@suse.cz>

Align __smp_call_function_single() with smp_call_function_single() so
that it also checks whether requested cpu is still online.

Signed-off-by: Jan Kara <jack@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 include/linux/smp.h |  3 +--
 kernel/smp.c        | 11 +++++++----
 kernel/up.c         |  5 +++--
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/linux/smp.h b/include/linux/smp.h
index 9a1b8ba..1e8c721 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -50,8 +50,7 @@ void on_each_cpu_cond(bool (*cond_func)(int cpu, void *info),
 		smp_call_func_t func, void *info, bool wait,
 		gfp_t gfp_flags);
 
-void __smp_call_function_single(int cpuid, struct call_single_data *data,
-				int wait);
+int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait);
 
 #ifdef CONFIG_SMP
 
diff --git a/kernel/smp.c b/kernel/smp.c
index e3852de..5ff14e3 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -276,18 +276,18 @@ EXPORT_SYMBOL_GPL(smp_call_function_any);
 /**
  * __smp_call_function_single(): Run a function on a specific CPU
  * @cpu: The CPU to run on.
- * @data: Pre-allocated and setup data structure
+ * @csd: Pre-allocated and setup data structure
  * @wait: If true, wait until function has completed on specified CPU.
  *
  * Like smp_call_function_single(), but allow caller to pass in a
  * pre-allocated data structure. Useful for embedding @data inside
  * other structures, for instance.
  */
-void __smp_call_function_single(int cpu, struct call_single_data *csd,
-				int wait)
+int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait)
 {
 	unsigned int this_cpu;
 	unsigned long flags;
+	int err = 0;
 
 	this_cpu = get_cpu();
 	/*
@@ -303,11 +303,14 @@ void __smp_call_function_single(int cpu, struct call_single_data *csd,
 		local_irq_save(flags);
 		csd->func(csd->info);
 		local_irq_restore(flags);
-	} else {
+	} else if ((unsigned)cpu < nr_cpu_ids && cpu_online(cpu)) {
 		csd_lock(csd);
 		generic_exec_single(cpu, csd, wait);
+	} else {
+		err = -ENXIO;	/* CPU not online */
 	}
 	put_cpu();
+	return err;
 }
 EXPORT_SYMBOL_GPL(__smp_call_function_single);
 
diff --git a/kernel/up.c b/kernel/up.c
index 509403e..cdf03d1 100644
--- a/kernel/up.c
+++ b/kernel/up.c
@@ -22,14 +22,15 @@ int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
 }
 EXPORT_SYMBOL(smp_call_function_single);
 
-void __smp_call_function_single(int cpu, struct call_single_data *csd,
-				int wait)
+int __smp_call_function_single(int cpu, struct call_single_data *csd,
+			       int wait)
 {
 	unsigned long flags;
 
 	local_irq_save(flags);
 	csd->func(csd->info);
 	local_irq_restore(flags);
+	return 0;
 }
 EXPORT_SYMBOL(__smp_call_function_single);
 
-- 
1.8.3.1

[PATCH 07/11] smp: Consolidate the various smp_call_function_single() declensions

[Frederic Weisbecker]

__smp_call_function_single() and smp_call_function_single() share some
code that can be factorized: execute inline when the target is local,
check if the target is online, lock the csd, call generic_exec_single().

Lets move the common parts to generic_exec_single().

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/smp.c | 80 +++++++++++++++++++++++++++++-------------------------------
 1 file changed, 39 insertions(+), 41 deletions(-)

diff --git a/kernel/smp.c b/kernel/smp.c
index 5ff14e3..64bb0d4 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -117,13 +117,43 @@ static void csd_unlock(struct call_single_data *csd)
 	csd->flags &= ~CSD_FLAG_LOCK;
 }
 
+static DEFINE_PER_CPU_SHARED_ALIGNED(struct call_single_data, csd_data);
+
 /*
  * Insert a previously allocated call_single_data element
  * for execution on the given CPU. data must already have
  * ->func, ->info, and ->flags set.
  */
-static void generic_exec_single(int cpu, struct call_single_data *csd, int wait)
+static int generic_exec_single(int cpu, struct call_single_data *csd,
+			       smp_call_func_t func, void *info, int wait)
 {
+	struct call_single_data csd_stack = { .flags = 0 };
+	unsigned long flags;
+
+
+	if (cpu == smp_processor_id()) {
+		local_irq_save(flags);
+		func(info);
+		local_irq_restore(flags);
+		return 0;
+	}
+
+
+	if ((unsigned)cpu >= nr_cpu_ids || !cpu_online(cpu))
+		return -ENXIO;
+
+
+	if (!csd) {
+		csd = &csd_stack;
+		if (!wait)
+			csd = &__get_cpu_var(csd_data);
+	}
+
+	csd_lock(csd);
+
+	csd->func = func;
+	csd->info = info;
+
 	if (wait)
 		csd->flags |= CSD_FLAG_WAIT;
 
@@ -143,6 +173,8 @@ static void generic_exec_single(int cpu, struct call_single_data *csd, int wait)
 
 	if (wait)
 		csd_lock_wait(csd);
+
+	return 0;
 }
 
 /*
@@ -168,8 +200,6 @@ void generic_smp_call_function_single_interrupt(void)
 	}
 }
 
-static DEFINE_PER_CPU_SHARED_ALIGNED(struct call_single_data, csd_data);
-
 /*
  * smp_call_function_single - Run a function on a specific CPU
  * @func: The function to run. This must be fast and non-blocking.
@@ -181,12 +211,8 @@ static DEFINE_PER_CPU_SHARED_ALIGNED(struct call_single_data, csd_data);
 int smp_call_function_single(int cpu, smp_call_func_t func, void *info,
 			     int wait)
 {
-	struct call_single_data d = {
-		.flags = 0,
-	};
-	unsigned long flags;
 	int this_cpu;
-	int err = 0;
+	int err;
 
 	/*
 	 * prevent preemption and reschedule on another processor,
@@ -203,26 +229,7 @@ int smp_call_function_single(int cpu, smp_call_func_t func, void *info,
 	WARN_ON_ONCE(cpu_online(this_cpu) && irqs_disabled()
 		     && !oops_in_progress);
 
-	if (cpu == this_cpu) {
-		local_irq_save(flags);
-		func(info);
-		local_irq_restore(flags);
-	} else {
-		if ((unsigned)cpu < nr_cpu_ids && cpu_online(cpu)) {
-			struct call_single_data *csd = &d;
-
-			if (!wait)
-				csd = &__get_cpu_var(csd_data);
-
-			csd_lock(csd);
-
-			csd->func = func;
-			csd->info = info;
-			generic_exec_single(cpu, csd, wait);
-		} else {
-			err = -ENXIO;	/* CPU not online */
-		}
-	}
+	err = generic_exec_single(cpu, NULL, func, info, wait);
 
 	put_cpu();
 
@@ -285,9 +292,8 @@ EXPORT_SYMBOL_GPL(smp_call_function_any);
  */
 int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait)
 {
-	unsigned int this_cpu;
-	unsigned long flags;
 	int err = 0;
+	int this_cpu;
 
 	this_cpu = get_cpu();
 	/*
@@ -296,20 +302,12 @@ int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait)
 	 * send smp call function interrupt to this cpu and as such deadlocks
 	 * can't happen.
 	 */
-	WARN_ON_ONCE(cpu_online(smp_processor_id()) && wait && irqs_disabled()
+	WARN_ON_ONCE(cpu_online(this_cpu) && wait && irqs_disabled()
 		     && !oops_in_progress);
 
-	if (cpu == this_cpu) {
-		local_irq_save(flags);
-		csd->func(csd->info);
-		local_irq_restore(flags);
-	} else if ((unsigned)cpu < nr_cpu_ids && cpu_online(cpu)) {
-		csd_lock(csd);
-		generic_exec_single(cpu, csd, wait);
-	} else {
-		err = -ENXIO;	/* CPU not online */
-	}
+	err = generic_exec_single(cpu, csd, csd->func, csd->info, wait);
 	put_cpu();
+
 	return err;
 }
 EXPORT_SYMBOL_GPL(__smp_call_function_single);
-- 
1.8.3.1

Re: [PATCH 07/11] smp: Consolidate the various smp_call_function_single() declensions

[Jan Kara]

On Sat 08-02-14 17:18:36, Frederic Weisbecker wrote:
> __smp_call_function_single() and smp_call_function_single() share some
> code that can be factorized: execute inline when the target is local,
> check if the target is online, lock the csd, call generic_exec_single().
> 
> Lets move the common parts to generic_exec_single().
  The patch looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> ---
>  kernel/smp.c | 80 +++++++++++++++++++++++++++++-------------------------------
>  1 file changed, 39 insertions(+), 41 deletions(-)
> 
> diff --git a/kernel/smp.c b/kernel/smp.c
> index 5ff14e3..64bb0d4 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -117,13 +117,43 @@ static void csd_unlock(struct call_single_data *csd)
>  	csd->flags &= ~CSD_FLAG_LOCK;
>  }
>  
> +static DEFINE_PER_CPU_SHARED_ALIGNED(struct call_single_data, csd_data);
> +
>  /*
>   * Insert a previously allocated call_single_data element
>   * for execution on the given CPU. data must already have
>   * ->func, ->info, and ->flags set.
>   */
> -static void generic_exec_single(int cpu, struct call_single_data *csd, int wait)
> +static int generic_exec_single(int cpu, struct call_single_data *csd,
> +			       smp_call_func_t func, void *info, int wait)
>  {
> +	struct call_single_data csd_stack = { .flags = 0 };
> +	unsigned long flags;
> +
> +
> +	if (cpu == smp_processor_id()) {
> +		local_irq_save(flags);
> +		func(info);
> +		local_irq_restore(flags);
> +		return 0;
> +	}
> +
> +
> +	if ((unsigned)cpu >= nr_cpu_ids || !cpu_online(cpu))
> +		return -ENXIO;
> +
> +
> +	if (!csd) {
> +		csd = &csd_stack;
> +		if (!wait)
> +			csd = &__get_cpu_var(csd_data);
> +	}
> +
> +	csd_lock(csd);
> +
> +	csd->func = func;
> +	csd->info = info;
> +
>  	if (wait)
>  		csd->flags |= CSD_FLAG_WAIT;
>  
> @@ -143,6 +173,8 @@ static void generic_exec_single(int cpu, struct call_single_data *csd, int wait)
>  
>  	if (wait)
>  		csd_lock_wait(csd);
> +
> +	return 0;
>  }
>  
>  /*
> @@ -168,8 +200,6 @@ void generic_smp_call_function_single_interrupt(void)
>  	}
>  }
>  
> -static DEFINE_PER_CPU_SHARED_ALIGNED(struct call_single_data, csd_data);
> -
>  /*
>   * smp_call_function_single - Run a function on a specific CPU
>   * @func: The function to run. This must be fast and non-blocking.
> @@ -181,12 +211,8 @@ static DEFINE_PER_CPU_SHARED_ALIGNED(struct call_single_data, csd_data);
>  int smp_call_function_single(int cpu, smp_call_func_t func, void *info,
>  			     int wait)
>  {
> -	struct call_single_data d = {
> -		.flags = 0,
> -	};
> -	unsigned long flags;
>  	int this_cpu;
> -	int err = 0;
> +	int err;
>  
>  	/*
>  	 * prevent preemption and reschedule on another processor,
> @@ -203,26 +229,7 @@ int smp_call_function_single(int cpu, smp_call_func_t func, void *info,
>  	WARN_ON_ONCE(cpu_online(this_cpu) && irqs_disabled()
>  		     && !oops_in_progress);
>  
> -	if (cpu == this_cpu) {
> -		local_irq_save(flags);
> -		func(info);
> -		local_irq_restore(flags);
> -	} else {
> -		if ((unsigned)cpu < nr_cpu_ids && cpu_online(cpu)) {
> -			struct call_single_data *csd = &d;
> -
> -			if (!wait)
> -				csd = &__get_cpu_var(csd_data);
> -
> -			csd_lock(csd);
> -
> -			csd->func = func;
> -			csd->info = info;
> -			generic_exec_single(cpu, csd, wait);
> -		} else {
> -			err = -ENXIO;	/* CPU not online */
> -		}
> -	}
> +	err = generic_exec_single(cpu, NULL, func, info, wait);
>  
>  	put_cpu();
>  
> @@ -285,9 +292,8 @@ EXPORT_SYMBOL_GPL(smp_call_function_any);
>   */
>  int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait)
>  {
> -	unsigned int this_cpu;
> -	unsigned long flags;
>  	int err = 0;
> +	int this_cpu;
>  
>  	this_cpu = get_cpu();
>  	/*
> @@ -296,20 +302,12 @@ int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait)
>  	 * send smp call function interrupt to this cpu and as such deadlocks
>  	 * can't happen.
>  	 */
> -	WARN_ON_ONCE(cpu_online(smp_processor_id()) && wait && irqs_disabled()
> +	WARN_ON_ONCE(cpu_online(this_cpu) && wait && irqs_disabled()
>  		     && !oops_in_progress);
>  
> -	if (cpu == this_cpu) {
> -		local_irq_save(flags);
> -		csd->func(csd->info);
> -		local_irq_restore(flags);
> -	} else if ((unsigned)cpu < nr_cpu_ids && cpu_online(cpu)) {
> -		csd_lock(csd);
> -		generic_exec_single(cpu, csd, wait);
> -	} else {
> -		err = -ENXIO;	/* CPU not online */
> -	}
> +	err = generic_exec_single(cpu, csd, csd->func, csd->info, wait);
>  	put_cpu();
> +
>  	return err;
>  }
>  EXPORT_SYMBOL_GPL(__smp_call_function_single);
> -- 
> 1.8.3.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH 08/11] smp: Move __smp_call_function_single() below its safe version

[Frederic Weisbecker]

Move this function closer to __smp_call_function_single(). These functions
have very similar behavior and should be displayed in the same block
for clarity.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/smp.c | 64 ++++++++++++++++++++++++++++++------------------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/kernel/smp.c b/kernel/smp.c
index 64bb0d4..fa04ab9 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -237,6 +237,38 @@ int smp_call_function_single(int cpu, smp_call_func_t func, void *info,
 }
 EXPORT_SYMBOL(smp_call_function_single);
 
+/**
+ * __smp_call_function_single(): Run a function on a specific CPU
+ * @cpu: The CPU to run on.
+ * @csd: Pre-allocated and setup data structure
+ * @wait: If true, wait until function has completed on specified CPU.
+ *
+ * Like smp_call_function_single(), but allow caller to pass in a
+ * pre-allocated data structure. Useful for embedding @data inside
+ * other structures, for instance.
+ */
+int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait)
+{
+	int err = 0;
+	int this_cpu;
+
+	this_cpu = get_cpu();
+	/*
+	 * Can deadlock when called with interrupts disabled.
+	 * We allow cpu's that are not yet online though, as no one else can
+	 * send smp call function interrupt to this cpu and as such deadlocks
+	 * can't happen.
+	 */
+	WARN_ON_ONCE(cpu_online(this_cpu) && wait && irqs_disabled()
+		     && !oops_in_progress);
+
+	err = generic_exec_single(cpu, csd, csd->func, csd->info, wait);
+	put_cpu();
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(__smp_call_function_single);
+
 /*
  * smp_call_function_any - Run a function on any of the given cpus
  * @mask: The mask of cpus it can run on.
@@ -281,38 +313,6 @@ call:
 EXPORT_SYMBOL_GPL(smp_call_function_any);
 
 /**
- * __smp_call_function_single(): Run a function on a specific CPU
- * @cpu: The CPU to run on.
- * @csd: Pre-allocated and setup data structure
- * @wait: If true, wait until function has completed on specified CPU.
- *
- * Like smp_call_function_single(), but allow caller to pass in a
- * pre-allocated data structure. Useful for embedding @data inside
- * other structures, for instance.
- */
-int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait)
-{
-	int err = 0;
-	int this_cpu;
-
-	this_cpu = get_cpu();
-	/*
-	 * Can deadlock when called with interrupts disabled.
-	 * We allow cpu's that are not yet online though, as no one else can
-	 * send smp call function interrupt to this cpu and as such deadlocks
-	 * can't happen.
-	 */
-	WARN_ON_ONCE(cpu_online(this_cpu) && wait && irqs_disabled()
-		     && !oops_in_progress);
-
-	err = generic_exec_single(cpu, csd, csd->func, csd->info, wait);
-	put_cpu();
-
-	return err;
-}
-EXPORT_SYMBOL_GPL(__smp_call_function_single);
-
-/**
  * smp_call_function_many(): Run a function on a set of other CPUs.
  * @mask: The set of cpus to run on (only runs on online subset).
  * @func: The function to run. This must be fast and non-blocking.
-- 
1.8.3.1

Re: [PATCH 08/11] smp: Move __smp_call_function_single() below its safe version

[Jan Kara]

On Sat 08-02-14 17:18:37, Frederic Weisbecker wrote:
> Move this function closer to __smp_call_function_single(). These functions
> have very similar behavior and should be displayed in the same block
> for clarity.
  Whatever :). You can add:
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> ---
>  kernel/smp.c | 64 ++++++++++++++++++++++++++++++------------------------------
>  1 file changed, 32 insertions(+), 32 deletions(-)
> 
> diff --git a/kernel/smp.c b/kernel/smp.c
> index 64bb0d4..fa04ab9 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -237,6 +237,38 @@ int smp_call_function_single(int cpu, smp_call_func_t func, void *info,
>  }
>  EXPORT_SYMBOL(smp_call_function_single);
>  
> +/**
> + * __smp_call_function_single(): Run a function on a specific CPU
> + * @cpu: The CPU to run on.
> + * @csd: Pre-allocated and setup data structure
> + * @wait: If true, wait until function has completed on specified CPU.
> + *
> + * Like smp_call_function_single(), but allow caller to pass in a
> + * pre-allocated data structure. Useful for embedding @data inside
> + * other structures, for instance.
> + */
> +int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait)
> +{
> +	int err = 0;
> +	int this_cpu;
> +
> +	this_cpu = get_cpu();
> +	/*
> +	 * Can deadlock when called with interrupts disabled.
> +	 * We allow cpu's that are not yet online though, as no one else can
> +	 * send smp call function interrupt to this cpu and as such deadlocks
> +	 * can't happen.
> +	 */
> +	WARN_ON_ONCE(cpu_online(this_cpu) && wait && irqs_disabled()
> +		     && !oops_in_progress);
> +
> +	err = generic_exec_single(cpu, csd, csd->func, csd->info, wait);
> +	put_cpu();
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(__smp_call_function_single);
> +
>  /*
>   * smp_call_function_any - Run a function on any of the given cpus
>   * @mask: The mask of cpus it can run on.
> @@ -281,38 +313,6 @@ call:
>  EXPORT_SYMBOL_GPL(smp_call_function_any);
>  
>  /**
> - * __smp_call_function_single(): Run a function on a specific CPU
> - * @cpu: The CPU to run on.
> - * @csd: Pre-allocated and setup data structure
> - * @wait: If true, wait until function has completed on specified CPU.
> - *
> - * Like smp_call_function_single(), but allow caller to pass in a
> - * pre-allocated data structure. Useful for embedding @data inside
> - * other structures, for instance.
> - */
> -int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait)
> -{
> -	int err = 0;
> -	int this_cpu;
> -
> -	this_cpu = get_cpu();
> -	/*
> -	 * Can deadlock when called with interrupts disabled.
> -	 * We allow cpu's that are not yet online though, as no one else can
> -	 * send smp call function interrupt to this cpu and as such deadlocks
> -	 * can't happen.
> -	 */
> -	WARN_ON_ONCE(cpu_online(this_cpu) && wait && irqs_disabled()
> -		     && !oops_in_progress);
> -
> -	err = generic_exec_single(cpu, csd, csd->func, csd->info, wait);
> -	put_cpu();
> -
> -	return err;
> -}
> -EXPORT_SYMBOL_GPL(__smp_call_function_single);
> -
> -/**
>   * smp_call_function_many(): Run a function on a set of other CPUs.
>   * @mask: The set of cpus to run on (only runs on online subset).
>   * @func: The function to run. This must be fast and non-blocking.
> -- 
> 1.8.3.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[PATCH 09/11] watchdog: Simplify a little the IPI call

[Frederic Weisbecker]

In order to remotely restart the watchdog hrtimer, update_timers()
allocates a csd on the stack and pass it to __smp_call_function_single().

There is no partcular need, however, for a specific csd here. Lets
simplify that a little by calling smp_call_function_single()
which can already take care of the csd allocation by itself.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/watchdog.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 4431610..01c6f97 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -505,7 +505,6 @@ static void restart_watchdog_hrtimer(void *info)
 
 static void update_timers(int cpu)
 {
-	struct call_single_data data = {.func = restart_watchdog_hrtimer};
 	/*
 	 * Make sure that perf event counter will adopt to a new
 	 * sampling period. Updating the sampling period directly would
@@ -515,7 +514,7 @@ static void update_timers(int cpu)
 	 * might be late already so we have to restart the timer as well.
 	 */
 	watchdog_nmi_disable(cpu);
-	__smp_call_function_single(cpu, &data, 1);
+	smp_call_function_single(cpu, restart_watchdog_hrtimer, NULL, 1);
 	watchdog_nmi_enable(cpu);
 }
 
-- 
1.8.3.1

Re: [PATCH 09/11] watchdog: Simplify a little the IPI call

[Michal Hocko]

On Sat 08-02-14 17:18:38, Frederic Weisbecker wrote:
> In order to remotely restart the watchdog hrtimer, update_timers()
> allocates a csd on the stack and pass it to __smp_call_function_single().
> 
> There is no partcular need, however, for a specific csd here. Lets
> simplify that a little by calling smp_call_function_single()
> which can already take care of the csd allocation by itself.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Don Zickus <dzickus@redhat.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>

Looks good to me.
Reviewed-by: Michal Hocko <mhocko@suse.cz>

> ---
>  kernel/watchdog.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 4431610..01c6f97 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -505,7 +505,6 @@ static void restart_watchdog_hrtimer(void *info)
>  
>  static void update_timers(int cpu)
>  {
> -	struct call_single_data data = {.func = restart_watchdog_hrtimer};
>  	/*
>  	 * Make sure that perf event counter will adopt to a new
>  	 * sampling period. Updating the sampling period directly would
> @@ -515,7 +514,7 @@ static void update_timers(int cpu)
>  	 * might be late already so we have to restart the timer as well.
>  	 */
>  	watchdog_nmi_disable(cpu);
> -	__smp_call_function_single(cpu, &data, 1);
> +	smp_call_function_single(cpu, restart_watchdog_hrtimer, NULL, 1);
>  	watchdog_nmi_enable(cpu);
>  }
>  
> -- 
> 1.8.3.1
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 09/11] watchdog: Simplify a little the IPI call

[Don Zickus]

On Sat, Feb 08, 2014 at 05:18:38PM +0100, Frederic Weisbecker wrote:
> In order to remotely restart the watchdog hrtimer, update_timers()
> allocates a csd on the stack and pass it to __smp_call_function_single().
> 
> There is no partcular need, however, for a specific csd here. Lets
> simplify that a little by calling smp_call_function_single()
> which can already take care of the csd allocation by itself.

Sounds simple enough.

Acked-by: Don Zickus <dzickus@redhat.com>

> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Don Zickus <dzickus@redhat.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> ---
>  kernel/watchdog.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 4431610..01c6f97 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -505,7 +505,6 @@ static void restart_watchdog_hrtimer(void *info)
>  
>  static void update_timers(int cpu)
>  {
> -	struct call_single_data data = {.func = restart_watchdog_hrtimer};
>  	/*
>  	 * Make sure that perf event counter will adopt to a new
>  	 * sampling period. Updating the sampling period directly would
> @@ -515,7 +514,7 @@ static void update_timers(int cpu)
>  	 * might be late already so we have to restart the timer as well.
>  	 */
>  	watchdog_nmi_disable(cpu);
> -	__smp_call_function_single(cpu, &data, 1);
> +	smp_call_function_single(cpu, restart_watchdog_hrtimer, NULL, 1);
>  	watchdog_nmi_enable(cpu);
>  }
>  
> -- 
> 1.8.3.1
> 

[PATCH 10/11] smp: Remove wait argument from __smp_call_function_single()

[Frederic Weisbecker]

The main point of calling __smp_call_function_single() is to send
an IPI in a pure asynchronous way. By embedding a csd in an object,
a caller can send the IPI without waiting for a previous one to complete
as is required by smp_call_function_single() for example. As such,
sending this kind of IPI can be safe even when irqs are disabled.

This flexibility comes at the expense of the caller who then needs to
synchronize the csd lifecycle by himself and make sure that IPIs on a
single csd are serialized.

This is how __smp_call_function_single() works when wait = 0 and this
usecase is relevant.

Now there don't seem to be any usecase with wait = 1 that can't be
covered by smp_call_function_single() instead, which is safer. Lets look
at the two possible scenario:

1) The user calls __smp_call_function_single(wait = 1) on a csd embedded
   in an object. It looks like a nice and convenient pattern at the first
   sight because we can then retrieve the object from the IPI handler easily.

   But actually it is a waste of memory space in the object since the csd
   can be allocated from the stack by smp_call_function_single(wait = 1)
   and the object can be passed an the IPI argument.

   Besides that, embedding the csd in an object is more error prone
   because the caller must take care of the serialization of the IPIs
   for this csd.

2) The user calls __smp_call_function_single(wait = 1) on a csd that
   is allocated on the stack. It's ok but smp_call_function_single()
   can do it as well and it already takes care of the allocation on the
   stack. Again it's more simple and less error prone.

Therefore, using the underscore prepend API version with wait = 1
is a bad pattern and a sign that the caller can do safer and more
simple.

There was a single user of that which has just been converted.
So lets remove this option to discourage further users.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 block/blk-mq.c            |  2 +-
 block/blk-softirq.c       |  2 +-
 drivers/block/null_blk.c  |  2 +-
 drivers/cpuidle/coupled.c |  2 +-
 include/linux/smp.h       |  2 +-
 kernel/sched/core.c       |  2 +-
 kernel/smp.c              | 19 ++++---------------
 kernel/up.c               |  3 +--
 net/core/dev.c            |  2 +-
 9 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 57039fc..87539d3 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -363,7 +363,7 @@ void blk_mq_end_io(struct request *rq, int error)
 		rq->csd.func = blk_mq_end_io_remote;
 		rq->csd.info = rq;
 		rq->csd.flags = 0;
-		__smp_call_function_single(ctx->cpu, &rq->csd, 0);
+		__smp_call_function_single(ctx->cpu, &rq->csd);
 	} else {
 		__blk_mq_end_io(rq, error);
 	}
diff --git a/block/blk-softirq.c b/block/blk-softirq.c
index b5c37d9..6345b7e 100644
--- a/block/blk-softirq.c
+++ b/block/blk-softirq.c
@@ -70,7 +70,7 @@ static int raise_blk_irq(int cpu, struct request *rq)
 		data->info = rq;
 		data->flags = 0;
 
-		__smp_call_function_single(cpu, data, 0);
+		__smp_call_function_single(cpu, data);
 		return 0;
 	}
 
diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c
index 3107282..c6722dd 100644
--- a/drivers/block/null_blk.c
+++ b/drivers/block/null_blk.c
@@ -255,7 +255,7 @@ static void null_cmd_end_ipi(struct nullb_cmd *cmd)
 	if (llist_add(&cmd->ll_list, &cq->list)) {
 		data->func = null_ipi_cmd_end_io;
 		data->flags = 0;
-		__smp_call_function_single(cpu, data, 0);
+		__smp_call_function_single(cpu, data);
 	}
 
 	put_cpu();
diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
index e952936..0411594 100644
--- a/drivers/cpuidle/coupled.c
+++ b/drivers/cpuidle/coupled.c
@@ -323,7 +323,7 @@ static void cpuidle_coupled_poke(int cpu)
 	struct call_single_data *csd = &per_cpu(cpuidle_coupled_poke_cb, cpu);
 
 	if (!cpumask_test_and_set_cpu(cpu, &cpuidle_coupled_poke_pending))
-		__smp_call_function_single(cpu, csd, 0);
+		__smp_call_function_single(cpu, csd);
 }
 
 /**
diff --git a/include/linux/smp.h b/include/linux/smp.h
index 1e8c721..3664ef3 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -50,7 +50,7 @@ void on_each_cpu_cond(bool (*cond_func)(int cpu, void *info),
 		smp_call_func_t func, void *info, bool wait,
 		gfp_t gfp_flags);
 
-int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait);
+int __smp_call_function_single(int cpu, struct call_single_data *csd);
 
 #ifdef CONFIG_SMP
 
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index b46131e..eba3d84 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -432,7 +432,7 @@ void hrtick_start(struct rq *rq, u64 delay)
 	if (rq == this_rq()) {
 		__hrtick_restart(rq);
 	} else if (!rq->hrtick_csd_pending) {
-		__smp_call_function_single(cpu_of(rq), &rq->hrtick_csd, 0);
+		__smp_call_function_single(cpu_of(rq), &rq->hrtick_csd);
 		rq->hrtick_csd_pending = 1;
 	}
 }
diff --git a/kernel/smp.c b/kernel/smp.c
index fa04ab9..b767631 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -241,29 +241,18 @@ EXPORT_SYMBOL(smp_call_function_single);
  * __smp_call_function_single(): Run a function on a specific CPU
  * @cpu: The CPU to run on.
  * @csd: Pre-allocated and setup data structure
- * @wait: If true, wait until function has completed on specified CPU.
  *
  * Like smp_call_function_single(), but allow caller to pass in a
  * pre-allocated data structure. Useful for embedding @data inside
  * other structures, for instance.
  */
-int __smp_call_function_single(int cpu, struct call_single_data *csd, int wait)
+int __smp_call_function_single(int cpu, struct call_single_data *csd)
 {
 	int err = 0;
-	int this_cpu;
 
-	this_cpu = get_cpu();
-	/*
-	 * Can deadlock when called with interrupts disabled.
-	 * We allow cpu's that are not yet online though, as no one else can
-	 * send smp call function interrupt to this cpu and as such deadlocks
-	 * can't happen.
-	 */
-	WARN_ON_ONCE(cpu_online(this_cpu) && wait && irqs_disabled()
-		     && !oops_in_progress);
-
-	err = generic_exec_single(cpu, csd, csd->func, csd->info, wait);
-	put_cpu();
+	preempt_disable();
+	err = generic_exec_single(cpu, csd, csd->func, csd->info, 0);
+	preempt_enable();
 
 	return err;
 }
diff --git a/kernel/up.c b/kernel/up.c
index cdf03d1..4e199d4 100644
--- a/kernel/up.c
+++ b/kernel/up.c
@@ -22,8 +22,7 @@ int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
 }
 EXPORT_SYMBOL(smp_call_function_single);
 
-int __smp_call_function_single(int cpu, struct call_single_data *csd,
-			       int wait)
+int __smp_call_function_single(int cpu, struct call_single_data *csd)
 {
 	unsigned long flags;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 3721db7..3f659b1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4129,7 +4129,7 @@ static void net_rps_action_and_irq_enable(struct softnet_data *sd)
 
 			if (cpu_online(remsd->cpu))
 				__smp_call_function_single(remsd->cpu,
-							   &remsd->csd, 0);
+							   &remsd->csd);
 			remsd = next;
 		}
 	} else
-- 
1.8.3.1

Re: [PATCH 10/11] smp: Remove wait argument from __smp_call_function_single()

[Christoph Hellwig]

Might be worth to give the function a saner name than just the __-prefix
if you touch all callers anyway.

Re: [PATCH 10/11] smp: Remove wait argument from __smp_call_function_single()

[Frederic Weisbecker]

On Sat, Feb 08, 2014 at 08:42:47AM -0800, Christoph Hellwig wrote:
> Might be worth to give the function a saner name than just the __-prefix
> if you touch all callers anyway.
> 


Good point. How about smp_call_function_single_async() ?

Re: [PATCH 10/11] smp: Remove wait argument from __smp_call_function_single()

[Christoph Hellwig]

On Sat, Feb 08, 2014 at 05:46:36PM +0100, Frederic Weisbecker wrote:
> Good point. How about smp_call_function_single_async() ?

Fine with me.

[PATCH 11/11] smp: Enhance and precise the role & requirements of __smp_call_function_single()

[Frederic Weisbecker]

Be more specific on the description of __smp_call_function_single().
It's actually not that much about just being able to embed a csd
in an object.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/smp.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/kernel/smp.c b/kernel/smp.c
index b767631..24cb936 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -238,13 +238,20 @@ int smp_call_function_single(int cpu, smp_call_func_t func, void *info,
 EXPORT_SYMBOL(smp_call_function_single);
 
 /**
- * __smp_call_function_single(): Run a function on a specific CPU
+ * __smp_call_function_single(): Run an asynchronous function on a
+ * 			         specific CPU.
  * @cpu: The CPU to run on.
  * @csd: Pre-allocated and setup data structure
  *
- * Like smp_call_function_single(), but allow caller to pass in a
- * pre-allocated data structure. Useful for embedding @data inside
- * other structures, for instance.
+ * Like smp_call_function_single(), but the call is asynchonous and
+ * can thus be done from contexts with disabled interrupts.
+ *
+ * The caller passes his own pre-allocated data structure
+ * (ie: embedded in an object) and is responsible for synchronizing it
+ * such that the IPIs performed on the @csd are strictly serialized.
+ *
+ * NOTE: Be careful, there is unfortunately no current debugging facility to
+ * validate the correctness of this serialization.
  */
 int __smp_call_function_single(int cpu, struct call_single_data *csd)
 {
-- 
1.8.3.1

[PATCH 00/11] smp: Single IPI cleanups v2

[Frederic Weisbecker]

Hi,

This version includes:

* Rename __smp_call_function_single to smp_call_function_single() as
  suggested by Christoph.
  
* Acks and reviewed-by added.

* Rebase against -rc4

Thanks.

---
Frederic Weisbecker (6):
  block: Remove useless IPI struct initialization
  smp: Consolidate the various smp_call_function_single() declensions
  smp: Move __smp_call_function_single() below its safe version
  watchdog: Simplify a little the IPI call
  smp: Remove wait argument from __smp_call_function_single()
  smp: Rename __smp_call_function_single() to
    smp_call_function_single_async()

Jan Kara (5):
  block: Stop abusing csd.list for fifo_time
  block: Stop abusing rq->csd.list in blk-softirq
  smp: Iterate functions through llist_for_each_entry_safe()
  smp: Remove unused list_head from csd
  smp: Teach __smp_call_function_single() to check for offline cpus

 block/blk-mq.c            |   2 +-
 block/blk-softirq.c       |  19 ++++---
 block/cfq-iosched.c       |   8 +--
 block/deadline-iosched.c  |   8 +--
 drivers/cpuidle/coupled.c |   2 +-
 include/linux/blkdev.h    |   1 +
 include/linux/elevator.h  |  11 +---
 include/linux/smp.h       |   8 +--
 kernel/sched/core.c       |   2 +-
 kernel/smp.c              | 139 ++++++++++++++++++++++------------------------
 kernel/up.c               |   6 +-
 kernel/watchdog.c         |   3 +-
 net/core/dev.c            |   4 +-
 13 files changed, 98 insertions(+), 115 deletions(-)

-- 
1.8.3.1

[PATCH 09/11] watchdog: Simplify a little the IPI call

[Frederic Weisbecker]

In order to remotely restart the watchdog hrtimer, update_timers()
allocates a csd on the stack and pass it to __smp_call_function_single().

There is no partcular need, however, for a specific csd here. Lets
simplify that a little by calling smp_call_function_single()
which can already take care of the csd allocation by itself.

Acked-by: Don Zickus <dzickus@redhat.com>
Reviewed-by: Michal Hocko <mhocko@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Jens Axboe <axboe@fb.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 kernel/watchdog.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 4431610..01c6f97 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -505,7 +505,6 @@ static void restart_watchdog_hrtimer(void *info)
 
 static void update_timers(int cpu)
 {
-	struct call_single_data data = {.func = restart_watchdog_hrtimer};
 	/*
 	 * Make sure that perf event counter will adopt to a new
 	 * sampling period. Updating the sampling period directly would
@@ -515,7 +514,7 @@ static void update_timers(int cpu)
 	 * might be late already so we have to restart the timer as well.
 	 */
 	watchdog_nmi_disable(cpu);
-	__smp_call_function_single(cpu, &data, 1);
+	smp_call_function_single(cpu, restart_watchdog_hrtimer, NULL, 1);
 	watchdog_nmi_enable(cpu);
 }
 
-- 
1.8.3.1