[Announce] 2.6.29-rc4-rt1

Peter Zijlstra · External Since: Jun 06, 2007 Posts: 205

On Thu, 2009-02-12 at 15:34 +0100, Frederic Weisbecker wrote:
>
> Would you prefer I test this one or wait for everybody agree a final
> version?

It'll work, but its not mergable upstream like this, it greatly
pessimizes the smp_function_call() performance.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Ingo Molnar · External Since: May 15, 2006 Posts: 3112

* Peter Zijlstra <a.p.zijlstra.RemoveThis@chello.nl> wrote:

> On Thu, 2009-02-12 at 15:28 +0100, Frederic Weisbecker wrote:
> > > [ 45.228589] hardirqs last disabled at (0): [<ffffffff8025449d>] copy_process+0x68d/0x1500
> > > [ 45.228602] softirqs last enabled at (0): [<ffffffff8025449d>] copy_process+0x68d/0x1500
> >
> >
> > The reason for which I wanted to send an irqsoff trace is that the above lines are false.
>
> copy_process() has:
>
> #endif
> p->hardirq_enable_ip = 0;
> p->hardirq_enable_event = 0;
> p->hardirq_disable_ip = _THIS_IP_;
> p->hardirq_disable_event = 0;
> p->softirqs_enabled = 1;
> p->softirq_enable_ip = _THIS_IP_;
> p->softirq_enable_event = 0;
> p->softirq_disable_ip = 0;
> p->softirq_disable_event = 0;
> p->hardirq_context = 0;
> p->softirq_context = 0;
> #endif
>
> the sequence count of 0 basically tells you it hasn't been set yet.

maybe we should initialize it to -1 to make this more apparent?

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Peter Zijlstra · External Since: Dec 16, 2006 Posts: 781

On Thu, 2009-02-12 at 15:46 +0100, Ingo Molnar wrote:
> * Peter Zijlstra <a.p.zijlstra RemoveThis @chello.nl> wrote:
>
> > On Thu, 2009-02-12 at 15:28 +0100, Frederic Weisbecker wrote:
> > > > [ 45.228589] hardirqs last disabled at (0): [<ffffffff8025449d>] copy_process+0x68d/0x1500
> > > > [ 45.228602] softirqs last enabled at (0): [<ffffffff8025449d>] copy_process+0x68d/0x1500
> > >
> > >
> > > The reason for which I wanted to send an irqsoff trace is that the above lines are false.
> >
> > copy_process() has:
> >
> > #endif
> > p->hardirq_enable_ip = 0;
> > p->hardirq_enable_event = 0;
> > p->hardirq_disable_ip = _THIS_IP_;
> > p->hardirq_disable_event = 0;
> > p->softirqs_enabled = 1;
> > p->softirq_enable_ip = _THIS_IP_;
> > p->softirq_enable_event = 0;
> > p->softirq_disable_ip = 0;
> > p->softirq_disable_event = 0;
> > p->hardirq_context = 0;
> > p->softirq_context = 0;
> > #endif
> >
> > the sequence count of 0 basically tells you it hasn't been set yet.
>
> maybe we should initialize it to -1 to make this more apparent?

I think the current state makes sense, it reflects the actual state of
copy_process.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Peter Zijlstra · External Since: Dec 16, 2006 Posts: 781

On Thu, 2009-02-12 at 15:50 +0100, Frederic Weisbecker wrote:
>
> > maybe we should initialize it to -1 to make this more apparent?
>
>
> Or why not set p->{hard,soft}irq_disable_ip = NULL ? (or 0, I don't know its type).

It is to reflect the actual state at copy_process(), although I think
here's a little buglet:

ifdef CONFIG_TRACE_IRQFLAGS
p->irq_events = 0;
#ifdef __ARCH_WANT_INTERRUPTS_ON_CTXSW
p->hardirqs_enabled = 1;
#else
p->hardirqs_enabled = 0;
#endif
p->hardirq_enable_ip = 0;
p->hardirq_enable_event = 0;
p->hardirq_disable_ip = _THIS_IP_;
p->hardirq_disable_event = 0;
p->softirqs_enabled = 1;
p->softirq_enable_ip = _THIS_IP_;
p->softirq_enable_event = 0;
p->softirq_disable_ip = 0;
p->softirq_disable_event = 0;
p->hardirq_context = 0;
p->softirq_context = 0;
#endif

I think the hardirq_enable/disable_ip should also depend on
__ARCH_WANT_INTERRUPTS_ON_CTXSW.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Frederic Weisbecker · External Since: Jan 17, 2009 Posts: 267

On Thu, Feb 12, 2009 at 03:46:24PM +0100, Ingo Molnar wrote:
>
> * Peter Zijlstra <a.p.zijlstra.RemoveThis@chello.nl> wrote:
>
> > On Thu, 2009-02-12 at 15:28 +0100, Frederic Weisbecker wrote:
> > > > [ 45.228589] hardirqs last disabled at (0): [<ffffffff8025449d>] copy_process+0x68d/0x1500
> > > > [ 45.228602] softirqs last enabled at (0): [<ffffffff8025449d>] copy_process+0x68d/0x1500
> > >
> > >
> > > The reason for which I wanted to send an irqsoff trace is that the above lines are false.
> >
> > copy_process() has:
> >
> > #endif
> > p->hardirq_enable_ip = 0;
> > p->hardirq_enable_event = 0;
> > p->hardirq_disable_ip = _THIS_IP_;
> > p->hardirq_disable_event = 0;
> > p->softirqs_enabled = 1;
> > p->softirq_enable_ip = _THIS_IP_;
> > p->softirq_enable_event = 0;
> > p->softirq_disable_ip = 0;
> > p->softirq_disable_event = 0;
> > p->hardirq_context = 0;
> > p->softirq_context = 0;
> > #endif
> >
> > the sequence count of 0 basically tells you it hasn't been set yet.

Oh I see now, thanks Peter.

> maybe we should initialize it to -1 to make this more apparent?

Or why not set p->{hard,soft}irq_disable_ip = NULL ? (or 0, I don't know its type).

> Ingo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Steven Rostedt · External Since: Jul 12, 2005 Posts: 581

On Thu, 12 Feb 2009, Peter Zijlstra wrote:
> > > ok, i made it unconditional (not just a PREEMPT_RT hac) and did the
> > > cleanup below on top of it.
> > >
> > > I dont think repeat, queued IPIs are all that interesting from a
> > > performance point of view. If they are, it will all be clearly
> > > bisectable.
> >
> > Right, except I really don't like the smp_call_function_many() slow path
> > that's now the only path.
> >
> > Rusty did that I think, but he also had some idea on how to fix it, I
> > think it boiled down to sticking a count in the call data instead of the
> > full cpumask.
> >
> > So I'd rather we first fix that code, and then remove the kmalloc
> > all-together like you propose here.
>
> Right, I can't see a way around carrying that cpumask, there's just too
> much that can go wrong without it.
>
> So it put in unconditionally, how about this?
>
>
> --
> Subject: generic-smp: remove single ipi fallback for smp_call_function_many()
>
> In preparation of removing the kmalloc() calls from the generic-ipi code
> get rid of the single ipi fallback for smp_call_function_many().
>
> Because we cannot get around carrying the cpumask in the data -- imagine
> 2 such calls with different but overlapping masks -- put in a full mask.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra DeleteThis @chello.nl>
> ---
> kernel/smp.c | 47 ++++++++++++++++++++++++-----------------------
> 1 files changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/kernel/smp.c b/kernel/smp.c
> index bbedbb7..e6658e5 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -26,7 +26,7 @@ struct call_function_data {
> spinlock_t lock;
> unsigned int refs;
> struct rcu_head rcu_head;
> - unsigned long cpumask_bits[];
> + struct cpumask cpumask;
> };
>
> struct call_single_queue {
> @@ -111,13 +111,13 @@ void generic_smp_call_function_interrupt(void)
> list_for_each_entry_rcu(data, &call_function_queue, csd.list) {
> int refs;
>
> - if (!cpumask_test_cpu(cpu, to_cpumask(data->cpumask_bits)))
> + if (!cpumask_test_cpu(cpu, &data->cpumask))
> continue;
>
> data->csd.func(data->csd.info);
>
> spin_lock(&data->lock);
> - cpumask_clear_cpu(cpu, to_cpumask(data->cpumask_bits));
> + cpumask_clear_cpu(cpu, &data->cpumask);
> WARN_ON(data->refs == 0);
> data->refs--;
> refs = data->refs;
> @@ -137,8 +137,10 @@ void generic_smp_call_function_interrupt(void)
> */
> smp_wmb();
> data->csd.flags &= ~CSD_FLAG_WAIT;
> - }
> - if (data->csd.flags & CSD_FLAG_ALLOC)
> + } else if (data->csd.flags & CSD_FLAG_LOCK) {
> + smp_wmb();
> + data->csd.flags &= ~CSD_FLAG_LOCK;
> + } else if (data->csd.flags & CSD_FLAG_ALLOC)
> call_rcu(&data->rcu_head, rcu_free_call_data);
> }
> rcu_read_unlock();
> @@ -302,6 +304,8 @@ void __smp_call_function_single(int cpu, struct call_single_data *data)
> arch_send_call_function_ipi(*(maskp))
> #endif
>
> +static DEFINE_PER_CPU(struct call_function_data, cfd_data);
> +
> /**
> * 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).
> @@ -323,14 +327,14 @@ void smp_call_function_many(const struct cpumask *mask,
> {
> struct call_function_data *data;
> unsigned long flags;
> - int cpu, next_cpu;
> + int cpu, next_cpu, me = smp_processor_id();
>
> /* Can deadlock when called with interrupts disabled */
> WARN_ON(irqs_disabled());
>
> /* So, what's a CPU they want? Ignoring this one. */
> cpu = cpumask_first_and(mask, cpu_online_mask);
> - if (cpu == smp_processor_id())
> + if (cpu == me)
> cpu = cpumask_next_and(cpu, mask, cpu_online_mask);
> /* No online cpus? We're done. */
> if (cpu >= nr_cpu_ids)
> @@ -338,7 +342,7 @@ void smp_call_function_many(const struct cpumask *mask,
>
> /* Do we have another CPU which isn't us? */
> next_cpu = cpumask_next_and(cpu, mask, cpu_online_mask);
> - if (next_cpu == smp_processor_id())
> + if (next_cpu == me)
> next_cpu = cpumask_next_and(next_cpu, mask, cpu_online_mask);
>
> /* Fastpath: do that cpu by itself. */
> @@ -347,27 +351,24 @@ void smp_call_function_many(const struct cpumask *mask,
> return;
> }
>
> - data = kmalloc(sizeof(*data) + cpumask_size(), GFP_ATOMIC);
> - if (unlikely(!data)) {
> - /* Slow path. */
> - for_each_online_cpu(cpu) {
> - if (cpu == smp_processor_id())
> - continue;
> - if (cpumask_test_cpu(cpu, mask))
> - smp_call_function_single(cpu, func, info, wait);
> - }
> - return;
> + data = kmalloc(sizeof(*data), GFP_ATOMIC);
> + if (data)
> + data->csd.flags = CSD_FLAG_ALLOC;
> + else {
> + data = &per_cpu(cfd_data, me);
> + while (data->csd.flags & CSD_FLAG_LOCK)
> + cpu_relax();
> + data->csd.flags = CSD_FLAG_LOCK;

Wont the first CPU that runs the callback unlock this? And then we run the
risk of two back to back callers on the same CPU, having the second
caller possibly corrupt the first.

-- Steve

> }
>
> spin_lock_init(&data->lock);
> - data->csd.flags = CSD_FLAG_ALLOC;
> if (wait)
> data->csd.flags |= CSD_FLAG_WAIT;
> data->csd.func = func;
> data->csd.info = info;
> - cpumask_and(to_cpumask(data->cpumask_bits), mask, cpu_online_mask);
> - cpumask_clear_cpu(smp_processor_id(), to_cpumask(data->cpumask_bits));
> - data->refs = cpumask_weight(to_cpumask(data->cpumask_bits));
> + cpumask_and(&data->cpumask, mask, cpu_online_mask);
> + cpumask_clear_cpu(smp_processor_id(), &data->cpumask);
> + data->refs = cpumask_weight(&data->cpumask);
>
> spin_lock_irqsave(&call_function_lock, flags);
> list_add_tail_rcu(&data->csd.list, &call_function_queue);
> @@ -379,7 +380,7 @@ void smp_call_function_many(const struct cpumask *mask,
> smp_mb();
>
> /* Send a message to all CPUs in the map */
> - arch_send_call_function_ipi_mask(to_cpumask(data->cpumask_bits));
> + arch_send_call_function_ipi_mask(&data->cpumask);
>
> /* optionally wait for the CPUs to complete */
> if (wait)
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Peter Zijlstra · External Since: Jun 06, 2007 Posts: 205

On Thu, 2009-02-12 at 10:43 -0500, Steven Rostedt wrote:
> > + data = kmalloc(sizeof(*data), GFP_ATOMIC);
> > + if (data)
> > + data->csd.flags = CSD_FLAG_ALLOC;
> > + else {
> > + data = &per_cpu(cfd_data, me);
> > + while (data->csd.flags & CSD_FLAG_LOCK)
> > + cpu_relax();
> > + data->csd.flags = CSD_FLAG_LOCK;
>
> Wont the first CPU that runs the callback unlock this? And then we run the
> risk of two back to back callers on the same CPU, having the second
> caller possibly corrupt the first.

No, there's a ref count in there that ensures the last one unlocks it.

But that's still not enough, the global queue is RCU protected.

Suppose you have 4 cpus, and use smp_function_call_mask() to 2 others,
now its possible the 4th is also doing global ipis and is traversing the
global queue.

Therefore, if you remove the cfd when its done, it might be the 4th cpu
is in it trying to iterate to the next entry --> BANG.

The solution used is RCU freeing cfd's. This also means we have to RCU
free the LOCK flag, sadly an RCU grace period is waaaay too long to spin
wait on.

Hence this whole solution is not quite feasible.

There's various alternative solutions, but I'm not quite sure which
makes most sense.

The one I'm currently pondering is using the global queue only for
all-but-self cfd's, this matches the all-but-self ipi APIC case.

For smaller masks we could queue a csd per queue and send single ipis.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Balbir Singh · External Since: Apr 21, 2007 Posts: 137

* Ingo Molnar <mingo.TakeThisOut@elte.hu> [2009-02-12 12:28:54]:

>
> * KAMEZAWA Hiroyuki <kamezawa.hiroyu.TakeThisOut@jp.fujitsu.com> wrote:
>
> > On Thu, 12 Feb 2009 11:21:13 +0100
> > Ingo Molnar <mingo.TakeThisOut@elte.hu> wrote:
> >
> > >
> > > * Ingo Molnar <mingo.TakeThisOut@elte.hu> wrote:
> > >
> > > > Frederic, could you try the patch below?
> > >
> > > Please try v2 below - it might even build Wink

> > >
> > > Ingo
> > >
> > > ------------------->
> > > Subject: rt: res_counter fix
> > > From: Ingo Molnar <mingo.TakeThisOut@elte.hu>
> > > Date: Thu Feb 12 11:11:47 CET 2009
> > >
> > > Frederic Weisbecker reported this warning:
> > >
> > > [ 45.228562] BUG: sleeping function called from invalid context at kernel/rtmutex.c:683
> > > [ 45.228571] in_atomic(): 0, irqs_disabled(): 1, pid: 4290, name: ntpdate
> > > [ 45.228576] INFO: lockdep is turned off.
> > > [ 45.228580] irq event stamp: 0
> > > [ 45.228583] hardirqs last enabled at (0): [<(null)>] (null)
> > > [ 45.228589] hardirqs last disabled at (0): [<ffffffff8025449d>] copy_process+0x68d/0x1500
> > > [ 45.228602] softirqs last enabled at (0): [<ffffffff8025449d>] copy_process+0x68d/0x1500
> > > [ 45.228609] softirqs last disabled at (0): [<(null)>] (null)
> > > [ 45.228617] Pid: 4290, comm: ntpdate Tainted: G W 2.6.29-rc4-rt1-tip #1
> > > [ 45.228622] Call Trace:
> > > [ 45.228632] [<ffffffff8027dfb0>] ? print_irqtrace_events+0xd0/0xe0
> > > [ 45.228639] [<ffffffff8024cd73>] __might_sleep+0x113/0x130
> > > [ 45.228646] [<ffffffff8077c811>] rt_spin_lock+0xa1/0xb0
> > > [ 45.228653] [<ffffffff80296a3d>] res_counter_charge+0x5d/0x130
> > > [ 45.228660] [<ffffffff802fb67f>] __mem_cgroup_try_charge+0x7f/0x180
> > > [ 45.228667] [<ffffffff802fc407>] mem_cgroup_charge_common+0x57/0x90
> > > [ 45.228674] [<ffffffff80212096>] ? ftrace_call+0x5/0x2b
> > > [ 45.228680] [<ffffffff802fc49d>] mem_cgroup_newpage_charge+0x5d/0x60
> > > [ 45.228688] [<ffffffff802d94ce>] __do_fault+0x29e/0x4c0
> > > [ 45.228694] [<ffffffff8077c843>] ? rt_spin_unlock+0x23/0x80
> > > [ 45.228700] [<ffffffff802db8b5>] handle_mm_fault+0x205/0x890
> > > [ 45.228707] [<ffffffff80212096>] ? ftrace_call+0x5/0x2b
> > > [ 45.228714] [<ffffffff8023495e>] do_page_fault+0x11e/0x2a0
> > > [ 45.228720] [<ffffffff8077e5a5>] page_fault+0x25/0x30
> > > [ 45.228727] [<ffffffff8043e1ed>] ? __clear_user+0x3d/0x70
> > > [ 45.228733] [<ffffffff8043e1d1>] ? __clear_user+0x21/0x70
> > >
> > > The reason is the raw IRQ flag use of kernel/res_counter.c.
> > >
> > > The irq flags tricks there seem a bit pointless: it cannot
> > > protect the c->parent linkage because local_irq_save() is
> > > only per CPU.
> > >
> > > So replace it with _nort(). This code needs a second look.
> > >
> > I'm sorry for no knowledge about RT. Could you teach me what
> > local_irq_save_nort() does ?
> >
> > Hmm, how about just replacaing _irq() with preempt_disable()/enable() ?
> > xxx_nort() is better ?
> >
> > AFAIK, these will not be called from irq context. (Added Balbir to CC:)
>
> _nort() will just turn them into NOPs in essence.
>
> The question is, are these local IRQ flags manipulations really needed
> in this code, and if yes, why?

We needed the local IRQ flags, since these counters are updated from
page fault context and from reclaim context with lru_lock held with
IRQ's disabled. I've been thinking about replacing the spin lock with
seq lock, but have not gotten to it yet.

--
Balbir
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Peter Zijlstra · External Since: Dec 16, 2006 Posts: 781

On Thu, 2009-02-12 at 22:28 +0530, Balbir Singh wrote:

> > _nort() will just turn them into NOPs in essence.
> >
> > The question is, are these local IRQ flags manipulations really needed
> > in this code, and if yes, why?
>
> We needed the local IRQ flags, since these counters are updated from
> page fault context and from reclaim context with lru_lock held with
> IRQ's disabled. I've been thinking about replacing the spin lock with
> seq lock, but have not gotten to it yet.

Ah, in that case we can get away with _nort I think, as both those
contexts are preemptable on -rt.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Thomas Gleixner · External Since: May 14, 2006 Posts: 944

We are pleased to announce the first update to our new preempt-rt
series. It fixes the reported bugs and some more.

Download locations:

http://rt.et.redhat.com/download/
http://www.kernel.org/pub/linux/kernel/projects/rt/

Information on the RT patch can be found at:

http://rt.wiki.kernel.org/index.php/Main_Page

to build the 2.6.29-rc4-rt2 tree, the following patches should be
applied:

http://www.kernel.org/pub/linux/kernel/v2.6/testing/linux-2.6.29-rc4.tar.bz2
http://www.kernel.org/pub/linux/kernel/projects/rt/patch-2.6.29-rc4-rt2.bz2

The broken out patches are also available at the same download
locations.

Enjoy !

Thomas, Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

KAMEZAWA Hiroyuki · External Since: May 17, 2006 Posts: 404

On Thu, 12 Feb 2009 22:28:33 +0530
Balbir Singh <balbir.DeleteThis@linux.vnet.ibm.com> wrote:

> * Ingo Molnar <mingo.DeleteThis@elte.hu> [2009-02-12 12:28:54]:
>
> >
> > * KAMEZAWA Hiroyuki <kamezawa.hiroyu.DeleteThis@jp.fujitsu.com> wrote:
> >
> > > On Thu, 12 Feb 2009 11:21:13 +0100
> > > Ingo Molnar <mingo.DeleteThis@elte.hu> wrote:
> > The question is, are these local IRQ flags manipulations really needed
> > in this code, and if yes, why?
>
> We needed the local IRQ flags, since these counters are updated from
> page fault context and from reclaim context with lru_lock held with
> IRQ's disabled. I've been thinking about replacing the spin lock with
> seq lock, but have not gotten to it yet.
>
Hmm ? I can't understand. Why we have to disable IRQ here again ?
And,
- try_to_unmap() is called in shrink_page_list(), there, no zone->lru_lock.
- page fault path doesn't hold zone->lru_lock.

My concern is only shmem. But I think it doesn't call charge() within lock, actually.

Thanks,
-Kame

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

KAMEZAWA Hiroyuki · External Since: May 17, 2006 Posts: 404

On Fri, 13 Feb 2009 08:56:40 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu.RemoveThis@jp.fujitsu.com> wrote:

> On Thu, 12 Feb 2009 22:28:33 +0530
> Balbir Singh <balbir.RemoveThis@linux.vnet.ibm.com> wrote:
>
> > * Ingo Molnar <mingo.RemoveThis@elte.hu> [2009-02-12 12:28:54]:
> >
> > >
> > > * KAMEZAWA Hiroyuki <kamezawa.hiroyu.RemoveThis@jp.fujitsu.com> wrote:
> > >
> > > > On Thu, 12 Feb 2009 11:21:13 +0100
> > > > Ingo Molnar <mingo.RemoveThis@elte.hu> wrote:
> > > The question is, are these local IRQ flags manipulations really needed
> > > in this code, and if yes, why?
> >
> > We needed the local IRQ flags, since these counters are updated from
> > page fault context and from reclaim context with lru_lock held with
> > IRQ's disabled. I've been thinking about replacing the spin lock with
> > seq lock, but have not gotten to it yet.
> >
> Hmm ? I can't understand. Why we have to disable IRQ here again ?
> And,
> - try_to_unmap() is called in shrink_page_list(), there, no zone->lru_lock.
> - page fault path doesn't hold zone->lru_lock.
>
> My concern is only shmem. But I think it doesn't call charge() within lock, actually

Clarification Smile

res_counter_charge() is called from
- page fault
=> under down_read(mmap_sem), lock_page() may be held. IRQ=ENABLED)
- add_to_page_cache
=> under lock_page(), mapping->tree_lock is *not* held, IRQ=DISABLED
- shmem
=> info->lock is held, we use __GFP_NOWAIT here. IRQ=ENABLED
- shmem
=> info->lock is *not* held with GFP_KERNEL here, IRQ=ENABLED.
- migration
=> under lock_page() and mmap_sem, IRQ=ENABLED

res_counter_uncharge() is called from
- page_remove_rmap()//(Only when ANON)
=> anon_vma->lock and pte_lock(),lock_page() can be held. IRQ=ENABLED?
- remove_from_page_cache()
=> lock_page() and mapping->tree_lock is held, IRQ=DISABLED.

Summary:
"Charge" is considered as heavy operation and the call path is placed where the
thread can sleep, AMAP.
"Uncharge" is considered as light operation and call path is under some # of
spinlocks.

Bye,
-Kame

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Frederic Weisbecker · External Since: Jan 17, 2009 Posts: 267

On Thu, Feb 12, 2009 at 11:56:38PM +0100, Thomas Gleixner wrote:
> We are pleased to announce the first update to our new preempt-rt
> series. It fixes the reported bugs and some more.
>
> Download locations:
>
> http://rt.et.redhat.com/download/
> http://www.kernel.org/pub/linux/kernel/projects/rt/
>
> Information on the RT patch can be found at:
>
> http://rt.wiki.kernel.org/index.php/Main_Page
>
> to build the 2.6.29-rc4-rt2 tree, the following patches should be
> applied:
>
> http://www.kernel.org/pub/linux/kernel/v2.6/testing/linux-2.6.29-rc4.tar.bz2
> http://www.kernel.org/pub/linux/kernel/projects/rt/patch-2.6.29-rc4-rt2.bz2
>
> The broken out patches are also available at the same download
> locations.
>
> Enjoy !
>
> Thomas, Ingo

Hi,

It fixes the rt-concerned warning I had.
Now, a new little lockdep info, tell me if you need my config, I changed
it a bit:

[ 0.012874] INFO: trying to register non-static key.
[ 0.012940] the code is fine but needs lockdep annotation.
[ 0.013008] turning off the locking correctness validator.
[ 0.013847] Pid: 0, comm: swapper Not tainted 2.6.29-rc4-rt2-tip #2
[ 0.013917] Call Trace:
[ 0.013981] [<ffffffff80285985>] __lock_acquire+0x1825/0x1b20
[ 0.014053] [<ffffffff802846fb>] ? __lock_acquire+0x59b/0x1b20
[ 0.014125] [<ffffffff80283326>] ? mark_held_locks+0x56/0xa0
[ 0.014197] [<ffffffff80285d1f>] lock_acquire+0x9f/0xe0
[ 0.014267] [<ffffffff80780ea8>] ? rt_spin_lock_slowlock+0x38/0x310
[ 0.014342] [<ffffffff80783211>] __spin_lock_irqsave+0x51/0x70
[ 0.014415] [<ffffffff80780ea8>] ? rt_spin_lock_slowlock+0x38/0x310
[ 0.014489] [<ffffffff80780ea8>] rt_spin_lock_slowlock+0x38/0x310
[ 0.014562] [<ffffffff802836dc>] ? debug_check_no_locks_freed+0x9c/0x160
[ 0.014636] [<ffffffff80781edc>] rt_spin_lock+0x4c/0xa0
[ 0.014706] [<ffffffff80276f51>] compat_down_timeout+0x21/0x60
[ 0.014780] [<ffffffff804910ca>] acpi_os_wait_semaphore+0x4e/0x5d
[ 0.014853] [<ffffffff804aafa9>] acpi_ut_acquire_mutex+0x43/0x87
[ 0.014926] [<ffffffff804a06cd>] acpi_ns_root_initialize+0x28/0x2ab
[ 0.014999] [<ffffffff80449d88>] ? __raw_spin_lock_init+0x38/0x70
[ 0.015073] [<ffffffff80acbd6a>] acpi_initialize_subsystem+0x69/0x91
[ 0.015147] [<ffffffff80acc24a>] acpi_early_init+0x50/0xf5
[ 0.015219] [<ffffffff80aa1ea5>] start_kernel+0x3d2/0x4ca
[ 0.015290] [<ffffffff80aa12b9>] x86_64_start_reservations+0x99/0xb9
[ 0.015362] [<ffffffff80aa13d5>] x86_64_start_kernel+0xfc/0x10b
[ 0.015436] [<ffffffff80aa1140>] ? early_idt_handler+0x0/0x71

Oh and I just tested the tracers.
It seems to run fine for all of them except sysprof.
It passes the self-test but doesn't produce any trace when I manually try.

Not completely sure this is only in -rt so I'm pulling very latest -tip
and will see if I find the same problem there.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Frederic Weisbecker · External Since: Jan 17, 2009 Posts: 267

On Fri, Feb 13, 2009 at 01:48:12AM +0100, Frederic Weisbecker wrote:
> On Thu, Feb 12, 2009 at 11:56:38PM +0100, Thomas Gleixner wrote:
> > We are pleased to announce the first update to our new preempt-rt
> > series. It fixes the reported bugs and some more.
> >
> > Download locations:
> >
> > http://rt.et.redhat.com/download/
> > http://www.kernel.org/pub/linux/kernel/projects/rt/
> >
> > Information on the RT patch can be found at:
> >
> > http://rt.wiki.kernel.org/index.php/Main_Page
> >
> > to build the 2.6.29-rc4-rt2 tree, the following patches should be
> > applied:
> >
> > http://www.kernel.org/pub/linux/kernel/v2.6/testing/linux-2.6.29-rc4.tar.bz2
> > http://www.kernel.org/pub/linux/kernel/projects/rt/patch-2.6.29-rc4-rt2.bz2
> >
> > The broken out patches are also available at the same download
> > locations.
> >
> > Enjoy !
> >
> > Thomas, Ingo
>
>
> Hi,
>
> It fixes the rt-concerned warning I had.
> Now, a new little lockdep info, tell me if you need my config, I changed
> it a bit:
>
> [ 0.012874] INFO: trying to register non-static key.
> [ 0.012940] the code is fine but needs lockdep annotation.
> [ 0.013008] turning off the locking correctness validator.
> [ 0.013847] Pid: 0, comm: swapper Not tainted 2.6.29-rc4-rt2-tip #2
> [ 0.013917] Call Trace:
> [ 0.013981] [<ffffffff80285985>] __lock_acquire+0x1825/0x1b20
> [ 0.014053] [<ffffffff802846fb>] ? __lock_acquire+0x59b/0x1b20
> [ 0.014125] [<ffffffff80283326>] ? mark_held_locks+0x56/0xa0
> [ 0.014197] [<ffffffff80285d1f>] lock_acquire+0x9f/0xe0
> [ 0.014267] [<ffffffff80780ea8>] ? rt_spin_lock_slowlock+0x38/0x310
> [ 0.014342] [<ffffffff80783211>] __spin_lock_irqsave+0x51/0x70
> [ 0.014415] [<ffffffff80780ea8>] ? rt_spin_lock_slowlock+0x38/0x310
> [ 0.014489] [<ffffffff80780ea8>] rt_spin_lock_slowlock+0x38/0x310
> [ 0.014562] [<ffffffff802836dc>] ? debug_check_no_locks_freed+0x9c/0x160
> [ 0.014636] [<ffffffff80781edc>] rt_spin_lock+0x4c/0xa0
> [ 0.014706] [<ffffffff80276f51>] compat_down_timeout+0x21/0x60
> [ 0.014780] [<ffffffff804910ca>] acpi_os_wait_semaphore+0x4e/0x5d
> [ 0.014853] [<ffffffff804aafa9>] acpi_ut_acquire_mutex+0x43/0x87
> [ 0.014926] [<ffffffff804a06cd>] acpi_ns_root_initialize+0x28/0x2ab
> [ 0.014999] [<ffffffff80449d88>] ? __raw_spin_lock_init+0x38/0x70
> [ 0.015073] [<ffffffff80acbd6a>] acpi_initialize_subsystem+0x69/0x91
> [ 0.015147] [<ffffffff80acc24a>] acpi_early_init+0x50/0xf5
> [ 0.015219] [<ffffffff80aa1ea5>] start_kernel+0x3d2/0x4ca
> [ 0.015290] [<ffffffff80aa12b9>] x86_64_start_reservations+0x99/0xb9
> [ 0.015362] [<ffffffff80aa13d5>] x86_64_start_kernel+0xfc/0x10b
> [ 0.015436] [<ffffffff80aa1140>] ? early_idt_handler+0x0/0x71
>
> Oh and I just tested the tracers.
> It seems to run fine for all of them except sysprof.
> It passes the self-test but doesn't produce any trace when I manually try.
>
> Not completely sure this is only in -rt so I'm pulling very latest -tip
> and will see if I find the same problem there.

About sysprof, it's an -rt problem, I don't see it on -tip.
The sysprof hrtimer callback is never called.

I'm digging to see what is happening.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Frederic Weisbecker · External Since: Jan 17, 2009 Posts: 267

On Fri, Feb 13, 2009 at 03:16:26AM +0100, Frederic Weisbecker wrote:
> On Fri, Feb 13, 2009 at 01:48:12AM +0100, Frederic Weisbecker wrote:
> > On Thu, Feb 12, 2009 at 11:56:38PM +0100, Thomas Gleixner wrote:
> > > We are pleased to announce the first update to our new preempt-rt
> > > series. It fixes the reported bugs and some more.
> > >
> > > Download locations:
> > >
> > > http://rt.et.redhat.com/download/
> > > http://www.kernel.org/pub/linux/kernel/projects/rt/
> > >
> > > Information on the RT patch can be found at:
> > >
> > > http://rt.wiki.kernel.org/index.php/Main_Page
> > >
> > > to build the 2.6.29-rc4-rt2 tree, the following patches should be
> > > applied:
> > >
> > > http://www.kernel.org/pub/linux/kernel/v2.6/testing/linux-2.6.29-rc4.tar.bz2
> > > http://www.kernel.org/pub/linux/kernel/projects/rt/patch-2.6.29-rc4-rt2.bz2
> > >
> > > The broken out patches are also available at the same download
> > > locations.
> > >
> > > Enjoy !
> > >
> > > Thomas, Ingo
> >
> >
> > Hi,
> >
> > It fixes the rt-concerned warning I had.
> > Now, a new little lockdep info, tell me if you need my config, I changed
> > it a bit:
> >
> > [ 0.012874] INFO: trying to register non-static key.
> > [ 0.012940] the code is fine but needs lockdep annotation.
> > [ 0.013008] turning off the locking correctness validator.
> > [ 0.013847] Pid: 0, comm: swapper Not tainted 2.6.29-rc4-rt2-tip #2
> > [ 0.013917] Call Trace:
> > [ 0.013981] [<ffffffff80285985>] __lock_acquire+0x1825/0x1b20
> > [ 0.014053] [<ffffffff802846fb>] ? __lock_acquire+0x59b/0x1b20
> > [ 0.014125] [<ffffffff80283326>] ? mark_held_locks+0x56/0xa0
> > [ 0.014197] [<ffffffff80285d1f>] lock_acquire+0x9f/0xe0
> > [ 0.014267] [<ffffffff80780ea8>] ? rt_spin_lock_slowlock+0x38/0x310
> > [ 0.014342] [<ffffffff80783211>] __spin_lock_irqsave+0x51/0x70
> > [ 0.014415] [<ffffffff80780ea8>] ? rt_spin_lock_slowlock+0x38/0x310
> > [ 0.014489] [<ffffffff80780ea8>] rt_spin_lock_slowlock+0x38/0x310
> > [ 0.014562] [<ffffffff802836dc>] ? debug_check_no_locks_freed+0x9c/0x160
> > [ 0.014636] [<ffffffff80781edc>] rt_spin_lock+0x4c/0xa0
> > [ 0.014706] [<ffffffff80276f51>] compat_down_timeout+0x21/0x60
> > [ 0.014780] [<ffffffff804910ca>] acpi_os_wait_semaphore+0x4e/0x5d
> > [ 0.014853] [<ffffffff804aafa9>] acpi_ut_acquire_mutex+0x43/0x87
> > [ 0.014926] [<ffffffff804a06cd>] acpi_ns_root_initialize+0x28/0x2ab
> > [ 0.014999] [<ffffffff80449d88>] ? __raw_spin_lock_init+0x38/0x70
> > [ 0.015073] [<ffffffff80acbd6a>] acpi_initialize_subsystem+0x69/0x91
> > [ 0.015147] [<ffffffff80acc24a>] acpi_early_init+0x50/0xf5
> > [ 0.015219] [<ffffffff80aa1ea5>] start_kernel+0x3d2/0x4ca
> > [ 0.015290] [<ffffffff80aa12b9>] x86_64_start_reservations+0x99/0xb9
> > [ 0.015362] [<ffffffff80aa13d5>] x86_64_start_kernel+0xfc/0x10b
> > [ 0.015436] [<ffffffff80aa1140>] ? early_idt_handler+0x0/0x71
> >
> > Oh and I just tested the tracers.
> > It seems to run fine for all of them except sysprof.
> > It passes the self-test but doesn't produce any trace when I manually try.
> >
> > Not completely sure this is only in -rt so I'm pulling very latest -tip
> > and will see if I find the same problem there.
>
>
> About sysprof, it's an -rt problem, I don't see it on -tip.
> The sysprof hrtimer callback is never called.
>
> I'm digging to see what is happening.

I didn't put my ftrace_printk at the right place.
It doesn't come from hrtimer.
The problem comes from get_irq_regs() which always returns NULL on the
sysprof hrtimer calback, then the trace is immediately dropped
by sysprof.

That's weird, smp_apic_timer_interrupt() calls set_irq_regs() so
it should find it.

Now I'm preempted by my bed.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Ingo Molnar · External Since: May 15, 2006 Posts: 3112

* Frederic Weisbecker <fweisbec.DeleteThis@gmail.com> wrote:

> > > It seems to run fine for all of them except sysprof. It passes the self-test
> > > but doesn't produce any trace when I manually try.
> > >
> > > Not completely sure this is only in -rt so I'm pulling very latest -tip and
> > > will see if I find the same problem there.
> >
> > About sysprof, it's an -rt problem, I don't see it on -tip. The sysprof hrtimer
> > callback is never called.
> >
> > I'm digging to see what is happening.
>
> I didn't put my ftrace_printk at the right place. It doesn't come from hrtimer.
> The problem comes from get_irq_regs() which always returns NULL on the sysprof
> hrtimer calback, then the trace is immediately dropped by sysprof.

Ah, that makes sense - under -rt the default hrtimer execution is to
execute in a softirq context. Could you try the patch below please,
does it fix sysprof?

Ingo

------------------>
Subject: rt: sysprof hrtimer fix
From: Ingo Molnar <mingo.DeleteThis@elte.hu>
Date: Fri Feb 13 08:22:14 CET 2009

Frederic Weisbecker noticed that sysprof does not work under
..29-rt2, and tracked it down to a NULL result that get_irq_regs()
gives to the sysprof plugin.

The reason for the NULL is that it executes in the HRTIMER_SOFTIRQ
context, hence it does not interrupt any real context and thus
there's no IRQ registers to take a look at.

Since the sysprof functionality is atomic, the fix is to move the
sysprof hrtimers to hardirq context.

Reported-by: Frederic Weisbecker <fweisbec.DeleteThis@gmail.com>
Signed-off-by: Ingo Molnar <mingo.DeleteThis@elte.hu>
---
kernel/trace/trace_sysprof.c | 1 +
1 file changed, 1 insertion(+)

Index: tip/kernel/trace/trace_sysprof.c
===================================================================
--- tip.orig/kernel/trace/trace_sysprof.c
+++ tip/kernel/trace/trace_sysprof.c
@@ -202,6 +202,7 @@ static void start_stack_timer(void *unus

hrtimer_init(hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
hrtimer->function = stack_trace_timer_fn;
+ hrtimer->irqsafe = 1;

hrtimer_start(hrtimer, ns_to_ktime(sample_period), HRTIMER_MODE_REL);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Frederic Weisbecker · External Since: Jan 17, 2009 Posts: 267

On Fri, Feb 13, 2009 at 08:26:01AM +0100, Ingo Molnar wrote:
>
> * Frederic Weisbecker <fweisbec DeleteThis @gmail.com> wrote:
>
> > > > It seems to run fine for all of them except sysprof. It passes the self-test
> > > > but doesn't produce any trace when I manually try.
> > > >
> > > > Not completely sure this is only in -rt so I'm pulling very latest -tip and
> > > > will see if I find the same problem there.
> > >
> > > About sysprof, it's an -rt problem, I don't see it on -tip. The sysprof hrtimer
> > > callback is never called.
> > >
> > > I'm digging to see what is happening.
> >
> > I didn't put my ftrace_printk at the right place. It doesn't come from hrtimer.
> > The problem comes from get_irq_regs() which always returns NULL on the sysprof
> > hrtimer calback, then the trace is immediately dropped by sysprof.
>
> Ah, that makes sense - under -rt the default hrtimer execution is to
> execute in a softirq context. Could you try the patch below please,
> does it fix sysprof?
>
> Ingo

Yes! Thanks Smile

Tested-by: Frederic Weisbecker <fweisbec DeleteThis @gmail.com>

So, just a thing.
-rt make the hrtimer's timers running on softirq context because the hrtimer_interrupt
doesn't run as a threaded interrupt, and then it is not preemptible right?

In that case, sysprof will continue to run in hardirq context, as before, and
it will considerably increase the latency. And that matters here.
So I think it is important to put it on the reminder:

-------------------->
Subject: [PATCH] rt: add sysprof to the latency overhead reminder
From: Frederic Weisbecker <fweisbec DeleteThis @gmail.com>

Since sysprof uses the hrtimer on hardirq with interrupts disabled, it
adds a latency overhead.

Add it to the latency reminder.

Signed-off-by: Frederic Weisbecker <fweisbec DeleteThis @gmail.com>
---
--- linux-2.6.29-rc4/init/~main.c 2009-02-13 12:51:54.000000000 +0100
+++ linux-2.6.29-rc4/init/main.c 2009-02-13 12:55:40.000000000 +0100
@@ -904,7 +904,7 @@ static int __init kernel_init(void * unu
WARN_ON(irqs_disabled());
#endif

-#define DEBUG_COUNT (defined(CONFIG_DEBUG_RT_MUTEXES) + defined(CONFIG_IRQSOFF_TRACER) + defined(CONFIG_PREEMPT_TRACER) + defined(CONFIG_STACK_TRACER) + defined(CONFIG_WAKEUP_LATENCY_HIST) + defined(CONFIG_DEBUG_SLAB) + defined(CONFIG_DEBUG_PAGEALLOC) + defined(CONFIG_LOCKDEP) + (defined(CONFIG_FTRACE) - defined(CONFIG_FTRACE_MCOUNT_RECORD)))
+#define DEBUG_COUNT (defined(CONFIG_DEBUG_RT_MUTEXES) + defined(CONFIG_IRQSOFF_TRACER) + defined(CONFIG_PREEMPT_TRACER) + defined(CONFIG_STACK_TRACER) + defined(CONFIG_WAKEUP_LATENCY_HIST) + defined(CONFIG_DEBUG_SLAB) + defined(CONFIG_DEBUG_PAGEALLOC) + defined(CONFIG_LOCKDEP) + defined(CONFIG_SYSPROF_TRACER) + (defined(CONFIG_FTRACE) - defined(CONFIG_FTRACE_MCOUNT_RECORD)))

#if DEBUG_COUNT > 0
printk(KERN_ERR "*****************************************************************************\n");
@@ -924,6 +924,9 @@ static int __init kernel_init(void * unu
#ifdef CONFIG_PREEMPT_TRACER
printk(KERN_ERR "* CONFIG_PREEMPT_TRACER *\n");
#endif
+#ifdef CONFIG_SYSPROF_TRACER
+ printk(KERN_ERR "* CONFIG_SYSPROF_TRACER *\n");
+#endif
#ifdef CONFIG_FTRACE
printk(KERN_ERR "* CONFIG_FTRACE *\n");
#endif

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Ingo Molnar · External Since: May 15, 2006 Posts: 3112

* Frederic Weisbecker <fweisbec.DeleteThis@gmail.com> wrote:

> So, just a thing.
> -rt make the hrtimer's timers running on softirq context because the hrtimer_interrupt
> doesn't run as a threaded interrupt, and then it is not preemptible right?
>
> In that case, sysprof will continue to run in hardirq context, as before, and
> it will considerably increase the latency. And that matters here.
> So I think it is important to put it on the reminder:

hm, not sure. Do you know it numerically how much worst-case overhead it
induces?

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Frederic Weisbecker · External Since: Jan 17, 2009 Posts: 267

On Fri, Feb 13, 2009 at 01:49:28PM +0100, Ingo Molnar wrote:
>
> * Frederic Weisbecker <fweisbec.RemoveThis@gmail.com> wrote:
>
> > So, just a thing.
> > -rt make the hrtimer's timers running on softirq context because the hrtimer_interrupt
> > doesn't run as a threaded interrupt, and then it is not preemptible right?
> >
> > In that case, sysprof will continue to run in hardirq context, as before, and
> > it will considerably increase the latency. And that matters here.
> > So I think it is important to put it on the reminder:
>
> hm, not sure. Do you know it numerically how much worst-case overhead it
> induces?
>
> Ingo

Not at all.
But when I find some time, I will compare some rt average tests with and without sysprof.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Rusty Russell · External Since: May 24, 2006 Posts: 528

On Thursday 12 February 2009 22:39:14 Peter Zijlstra wrote:
> So it put in unconditionally, how about this?
>
>
> --
> Subject: generic-smp: remove single ipi fallback for smp_call_function_many()
>
> In preparation of removing the kmalloc() calls from the generic-ipi code
> get rid of the single ipi fallback for smp_call_function_many().
>
> Because we cannot get around carrying the cpumask in the data -- imagine
> 2 such calls with different but overlapping masks -- put in a full mask.

OK, if you really want this, please just change it to:
unsigned long cpumask_bits[BITS_TO_LONGS(CONFIG_NR_CPUS)];

The 'struct cpumask' will be undefined soon when CONFIG_CPUMASK_OFFSTACK=y,
which will prevent assignment and declaration on stack.

I'd be fascinated to see perf numbers once you kill the kmalloc. Because
this patch will add num_possible_cpus * NR_CPUS/8 bytes to the kernel which
is something we're trying to avoid unless necessary.

Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/