On Fri, May 19, 2017 at 03:53:31PM +0800, Aaron Lu wrote:
> Inter-Processor-Interrupt(IPI) is needed when a page is unmapped and the
> process' mm_cpumask() shows the process has ever run on other CPUs. page
> migration, page reclaim all need IPIs. The number of IPI needed to send
> to different CPUs is especially large for multi-threaded workload since
> mm_cpumask() is per process.
>
> For smp_call_function_many(), whenever a CPU queues a CSD to a target
> CPU, it will send an IPI to let the target CPU to handle the work.
> This isn't necessary - we need only send IPI when queueing a CSD
> to an empty call_single_queue.
>
> The reason:
> flush_smp_call_function_queue() that is called upon a CPU receiving an
> IPI will empty the queue and then handle all of the CSDs there. So if
> the target CPU's call_single_queue is not empty, we know that:
> i. An IPI for the target CPU has already been sent by 'previous queuers';
> ii. flush_smp_call_function_queue() hasn't emptied that CPU's queue yet.
> Thus, it's safe for us to just queue our CSD there without sending an
> addtional IPI. And for the 'previous queuers', we can limit it to the
> first queuer.
>
> To demonstrate the effect of this patch, a multi-thread workload that
> spawns 80 threads to equally consume 100G memory is used. This is tested
> on a 2 node broadwell-EP which has 44cores/88threads and 32G memory. So
> after 32G memory is used up, page reclaiming starts to happen a lot.
>
> With this patch, IPI number dropped 88% and throughput increased about
> 15% for the above workload.
>
> Signed-off-by: Aaron Lu <[email protected]>
Seems fine to me, I'll queue it.
> @@ -434,6 +442,7 @@ void smp_call_function_many(const struct cpumask *mask,
> if (unlikely(!cpumask_weight(cfd->cpumask)))
> return;
>
> + cpumask_clear(cfd->cpumask_ipi);
> for_each_cpu(cpu, cfd->cpumask) {
> struct call_single_data *csd = per_cpu_ptr(cfd->csd, cpu);
>
> @@ -442,11 +451,12 @@ void smp_call_function_many(const struct cpumask *mask,
> csd->flags |= CSD_FLAG_SYNCHRONOUS;
> csd->func = func;
> csd->info = info;
> - llist_add(&csd->llist, &per_cpu(call_single_queue, cpu));
> + if (llist_add(&csd->llist, &per_cpu(call_single_queue, cpu)))
> + cpumask_set_cpu(cpu, cfd->cpumask_ipi);
> }
But looking at this I wonder why cpumask_{set,clear}_cpu() are atomic
ops while most other cpumask ops are not.
This seems to suggest we want __cpumask_{set,clear}_cpu() and use those
here. Because those LOCK prefixes sure are pointless.
>
> /* Send a message to all CPUs in the map */
> - arch_send_call_function_ipi_mask(cfd->cpumask);
> + arch_send_call_function_ipi_mask(cfd->cpumask_ipi);
>
> if (wait) {
> for_each_cpu(cpu, cfd->cpumask) {
Something like so on top I suppose.
Anybody?
---
include/linux/cpumask.h | 11 +++++++++++
kernel/smp.c | 4 ++--
2 files changed, 13 insertions(+), 2 deletions(-)
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -293,6 +293,12 @@ static inline void cpumask_set_cpu(unsig
set_bit(cpumask_check(cpu), cpumask_bits(dstp));
}
+static inline void __cpumask_set_cpu(unsigned int cpu, struct cpumask *dstp)
+{
+ __set_bit(cpumask_check(cpu), cpumask_bits(dstp));
+}
+
+
/**
* cpumask_clear_cpu - clear a cpu in a cpumask
* @cpu: cpu number (< nr_cpu_ids)
@@ -303,6 +309,11 @@ static inline void cpumask_clear_cpu(int
clear_bit(cpumask_check(cpu), cpumask_bits(dstp));
}
+static inline void __cpumask_clear_cpu(int cpu, struct cpumask *dstp)
+{
+ __clear_bit(cpumask_check(cpu), cpumask_bits(dstp));
+}
+
/**
* cpumask_test_cpu - test for a cpu in a cpumask
* @cpu: cpu number (< nr_cpu_ids)
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -436,7 +436,7 @@ void smp_call_function_many(const struct
cfd = this_cpu_ptr(&cfd_data);
cpumask_and(cfd->cpumask, mask, cpu_online_mask);
- cpumask_clear_cpu(this_cpu, cfd->cpumask);
+ __cpumask_clear_cpu(this_cpu, cfd->cpumask);
/* Some callers race with other cpus changing the passed mask */
if (unlikely(!cpumask_weight(cfd->cpumask)))
@@ -452,7 +452,7 @@ void smp_call_function_many(const struct
csd->func = func;
csd->info = info;
if (llist_add(&csd->llist, &per_cpu(call_single_queue, cpu)))
- cpumask_set_cpu(cpu, cfd->cpumask_ipi);
+ __cpumask_set_cpu(cpu, cfd->cpumask_ipi);
}
/* Send a message to all CPUs in the map */
