On 11/29, Paul E. McKenney wrote:
>
> On Wed, Nov 29, 2006 at 11:16:46PM +0300, Oleg Nesterov wrote:
> >
> > Hmm... SRCU can't be used from irq, yes. But I think that both versions
> > (spinlock needs _irqsave) can ?
>
> I didn't think you could call wait_event() from irq.
Ah, sorry for confusio
On Wed, Nov 29, 2006 at 11:16:46PM +0300, Oleg Nesterov wrote:
> On 11/29, Paul E. McKenney wrote:
> >
> > 1. The spinlock version will be easier for most people to understand.
> >
> > 2. The atomic version has better read-side overhead -- probably
> > roughly twice as fast on most machines.
On 11/29, Paul E. McKenney wrote:
>
> 1.The spinlock version will be easier for most people to understand.
>
> 2.The atomic version has better read-side overhead -- probably
> roughly twice as fast on most machines.
synchronize_xxx() should be a little bit faster too
> 3.The at
On Mon, Nov 27, 2006 at 07:11:06PM +0300, Oleg Nesterov wrote:
> On 11/26, Paul E. McKenney wrote:
> >
> > Looks pretty good, actually. A few quibbles below. I need to review
> > again after sleeping on it.
>
> Thanks! Please also look at spinlock-based implementation,
>
> http://marc.the
On Mon, Nov 27, 2006 at 04:10:27PM -0500, Alan Stern wrote:
> On Mon, 27 Nov 2006, Oleg Nesterov wrote:
>
> > I still can't relax, another attempt to "prove" this should not be
> > possible on CPUs supported by Linux :)
> >
> > Let's suppose it is possible, then it should also be possible if CPU_1
On Mon, 27 Nov 2006, Oleg Nesterov wrote:
> I still can't relax, another attempt to "prove" this should not be
> possible on CPUs supported by Linux :)
>
> Let's suppose it is possible, then it should also be possible if CPU_1
> does spin_lock() instead of mb() (spin_lock can't be "stronger"), ye
On 11/27, Oleg Nesterov wrote:
>
> so synchronize_xxx() should be
>
> void synchronize_xxx(struct xxx_struct *sp)
> {
> int idx;
>
> smp_mb();
> mutex_lock(&sp->mutex);
>
> idx = sp->completed & 0x1;
> if (atomic_r
On 11/26, Paul E. McKenney wrote:
>
> Looks pretty good, actually. A few quibbles below. I need to review
> again after sleeping on it.
Thanks! Please also look at spinlock-based implementation,
http://marc.theaimsgroup.com/?l=linux-kernel&m=116457701231964
I must admit, Alan was right
On Fri, Nov 24, 2006 at 09:21:53PM +0300, Oleg Nesterov wrote:
> Ok, synchronize_xxx() passed 1 hour rcutorture test on dual P-III.
>
> It behaves the same as srcu but optimized for writers. The fast path
> for synchronize_xxx() is mutex_lock() + atomic_read() + mutex_unlock().
> The slow path is
On Fri, Nov 24, 2006 at 12:49:08AM +0300, Oleg Nesterov wrote:
> On 11/23, Paul E. McKenney wrote:
> >
> >For general use, I believe that this has
> > difficulties with the sequence of events I sent out on November 20th, see:
> >
> > http://marc.theaimsgroup.com/?l=linux
On 11/20, Oleg Nesterov wrote:
>
> So, if we have global A == B == 0,
>
> CPU_0 CPU_1
>
> A = 1; B = 2;
> mb(); mb();
> b = B; a = A;
>
> It could happen that a == b == 0, yes? Isn't this contradicts with definition
> of mb?
I still can
On 11/25, Alan Stern wrote:
>
> Yes, you are right. The corrected routine (including your little
> optimization) looks like this:
>
> void synchronize_xxx(struct xxx_struct *sp)
> {
> int idx;
>
> mutex_lock(&sp->mutex);
> spin_lock(&sp->lock);
> idx = sp->completed & 0x
On Sat, 25 Nov 2006, Oleg Nesterov wrote:
> > void xxx_read_unlock(struct xxx_struct *sp, int idx)
> > {
> > spin_lock(&sp->lock);
>
> It is possible that the memory ops that occur before spin_lock() is not yet
> completed,
>
> > if (--sp->ctr[idx] == 0)
>
> suppose that synchronize_xxx
On 11/19, Paul E. McKenney wrote:
>
> On Mon, Nov 20, 2006 at 12:17:31AM +0300, Oleg Nesterov wrote:
> >
> > It will wait for xxx_read_unlock() on reader's side. And for this reason
> > this idx in fact is not exactly wrong :)
>
> I am not seeing this.
>
> Let's assume sp->completed starts out z
On Mon, Nov 20, 2006 at 12:50:53AM +0300, Oleg Nesterov wrote:
> On 11/19, Paul E. McKenney wrote:
> >
> > On Sun, Nov 19, 2006 at 11:55:16PM +0300, Oleg Nesterov wrote:
> > > So synchronize_xxx() should do
> > >
> > > smp_mb();
> > > idx = sp->completed++ & 0x1;
> > >
> > > for (;;) { ... }
On Mon, Nov 20, 2006 at 12:17:31AM +0300, Oleg Nesterov wrote:
> On 11/19, Alan Stern wrote:
> > On Sun, 19 Nov 2006, Oleg Nesterov wrote:
> >
> > > > What happens if synchronize_xxx manages to execute inbetween
> > > > xxx_read_lock's
> > > >
> > > > idx = sp->completed & 0x1;
> >
On 11/19, Paul E. McKenney wrote:
>
> On Sun, Nov 19, 2006 at 11:55:16PM +0300, Oleg Nesterov wrote:
> > So synchronize_xxx() should do
> >
> > smp_mb();
> > idx = sp->completed++ & 0x1;
> >
> > for (;;) { ... }
> >
> > > You see, there's no way around using synchronize_sc
On Sat, Nov 18, 2006 at 04:00:35PM -0500, Alan Stern wrote:
> On Sat, 18 Nov 2006, Paul E. McKenney wrote:
>
> > > > @@ -94,7 +112,8 @@ void cleanup_srcu_struct(struct srcu_str
> > > > WARN_ON(sum); /* Leakage unless caller handles error. */
> > > > if (sum != 0)
> > > >
On Sat, Nov 18, 2006 at 10:34:26PM +0300, Oleg Nesterov wrote:
> On 11/18, Paul E. McKenney wrote:
> >
> > On Sat, Nov 18, 2006 at 11:15:27AM -0500, Alan Stern wrote:
> > > > + smp_processor_id())->c[idx]++;
> > > > + smp_mb();
> > > > + preempt
On Sat, Nov 18, 2006 at 10:02:17PM +0300, Oleg Nesterov wrote:
> On 11/17, Paul E. McKenney wrote:
> >
> > int srcu_read_lock(struct srcu_struct *sp)
> > {
> > int idx;
> > + struct srcu_struct_array *sap;
> >
> > preempt_disable();
> > idx = sp->completed & 0x1;
> > - barrier();
On Sun, Nov 19, 2006 at 11:55:16PM +0300, Oleg Nesterov wrote:
> On 11/19, Alan Stern wrote:
> >
> > On Sun, 19 Nov 2006, Oleg Nesterov wrote:
> >
> > > int xxx_read_lock(struct xxx_struct *sp)
> > > {
> > > int idx;
> > >
> > > idx = sp->completed & 0x1;
> > > ato
On 11/19, Alan Stern wrote:
> On Sun, 19 Nov 2006, Oleg Nesterov wrote:
>
> > > What happens if synchronize_xxx manages to execute inbetween
> > > xxx_read_lock's
> > >
> > > idx = sp->completed & 0x1;
> > > atomic_inc(sp->ctr + idx);
> > >
> > > statements?
> >
> > Oops. I
On Sat, Nov 18, 2006 at 09:46:24PM +0300, Oleg Nesterov wrote:
> On 11/17, Paul E. McKenney wrote:
> >
> > Oleg, any thoughts about Jens's optimization? He would code something
> > like:
> >
> > if (srcu_readers_active(&my_srcu))
> > synchronize_srcu();
> > else
> >
On Sun, 19 Nov 2006, Oleg Nesterov wrote:
> > What happens if synchronize_xxx manages to execute inbetween
> > xxx_read_lock's
> >
> > idx = sp->completed & 0x1;
> > atomic_inc(sp->ctr + idx);
> >
> > statements?
>
> Oops. I forgot about explicit mb() before sp->complet
On 11/19, Alan Stern wrote:
>
> On Sun, 19 Nov 2006, Oleg Nesterov wrote:
>
> > int xxx_read_lock(struct xxx_struct *sp)
> > {
> > int idx;
> >
> > idx = sp->completed & 0x1;
> > atomic_inc(sp->ctr + idx);
> > smp_mb__after_atomic_inc();
> >
On Sun, 19 Nov 2006, Oleg Nesterov wrote:
> On 11/17, Jens Axboe wrote:
> >
> > It works for me, but the overhead is still large. Before it would take
> > 8-12 jiffies for a synchronize_srcu() to complete without there actually
> > being any reader locks active, now it takes 2-3 jiffies. So it's
>
On Sun, 19 Nov 2006, Oleg Nesterov wrote:
> > Put it this way: If the missing memory barrier in srcu_read_lock() after
> > the atomic_inc call isn't needed, then neither is the existing memory
> > barrier after the per-cpu counter gets incremented.
>
> I disagree. There is another reason for mb()
On 11/17, Jens Axboe wrote:
>
> It works for me, but the overhead is still large. Before it would take
> 8-12 jiffies for a synchronize_srcu() to complete without there actually
> being any reader locks active, now it takes 2-3 jiffies. So it's
> definitely faster, and as suspected the loss of two
On 11/18, Alan Stern wrote:
>
> On Sun, 19 Nov 2006, Oleg Nesterov wrote:
>
> > On 11/18, Alan Stern wrote:
> > >
> > > By the way, I think the fastpath for synchronize_srcu() should be safe,
> > > now that you have added the memory barriers into srcu_read_lock() and
> > > srcu_read_unlock(). Y
On Sun, 19 Nov 2006, Oleg Nesterov wrote:
> On 11/18, Alan Stern wrote:
> >
> > By the way, I think the fastpath for synchronize_srcu() should be safe,
> > now that you have added the memory barriers into srcu_read_lock() and
> > srcu_read_unlock(). You might as well try putting it in.
>
> I s
On 11/18, Alan Stern wrote:
>
> By the way, I think the fastpath for synchronize_srcu() should be safe,
> now that you have added the memory barriers into srcu_read_lock() and
> srcu_read_unlock(). You might as well try putting it in.
I still think the fastpath should do mb() unconditionally to
On Sat, 18 Nov 2006, Paul E. McKenney wrote:
> > > @@ -94,7 +112,8 @@ void cleanup_srcu_struct(struct srcu_str
> > > WARN_ON(sum); /* Leakage unless caller handles error. */
> > > if (sum != 0)
> > > return;
> > > - free_percpu(sp->per_cpu_ref);
> > > + if (sp->per_cpu_ref != NULL)
On 11/18, Paul E. McKenney wrote:
>
> On Sat, Nov 18, 2006 at 11:15:27AM -0500, Alan Stern wrote:
> > > + smp_processor_id())->c[idx]++;
> > > + smp_mb();
> > > + preempt_enable();
> > > + return idx;
> > > + }
> > > + if (mutex_trylock(&sp->mutex)) {
> >
On 11/17, Paul E. McKenney wrote:
>
> int srcu_read_lock(struct srcu_struct *sp)
> {
> int idx;
> + struct srcu_struct_array *sap;
>
> preempt_disable();
> idx = sp->completed & 0x1;
> - barrier(); /* ensure compiler looks -once- at sp->completed. */
> - per_cpu_p
On 11/17, Paul E. McKenney wrote:
>
> Oleg, any thoughts about Jens's optimization? He would code something
> like:
>
> if (srcu_readers_active(&my_srcu))
> synchronize_srcu();
> else
> smp_mb();
Well, this is clearly racy, no? I am not sure, but may be we
On Sat, Nov 18, 2006 at 11:15:27AM -0500, Alan Stern wrote:
> There are a few things I don't like about this patch.
>
> On Fri, 17 Nov 2006, Paul E. McKenney wrote:
>
> > diff -urpNa -X dontdiff linux-2.6.19-rc5/kernel/srcu.c
> > linux-2.6.19-rc5-dsrcu/kernel/srcu.c
> > --- linux-2.6.19-rc5/kern
There are a few things I don't like about this patch.
On Fri, 17 Nov 2006, Paul E. McKenney wrote:
> diff -urpNa -X dontdiff linux-2.6.19-rc5/kernel/srcu.c
> linux-2.6.19-rc5-dsrcu/kernel/srcu.c
> --- linux-2.6.19-rc5/kernel/srcu.c2006-11-17 13:54:17.0 -0800
> +++ linux-2.6.19-rc5-ds
On Fri, Nov 17, 2006 at 08:51:03PM -0800, Andrew Morton wrote:
> On Fri, 17 Nov 2006 23:33:45 -0500 (EST)
> Alan Stern <[EMAIL PROTECTED]> wrote:
>
> > On Fri, 17 Nov 2006, Paul E. McKenney wrote:
> >
> > > > Perhaps a better approach to the initialization problem would be to
> > > > assume
> > >
On Fri, 17 Nov 2006 23:33:45 -0500 (EST)
Alan Stern <[EMAIL PROTECTED]> wrote:
> On Fri, 17 Nov 2006, Paul E. McKenney wrote:
>
> > > Perhaps a better approach to the initialization problem would be to
> > > assume
> > > that either:
> > >
> > > 1. The srcu_struct will be initialized befo
On Fri, 17 Nov 2006, Paul E. McKenney wrote:
> > Perhaps a better approach to the initialization problem would be to assume
> > that either:
> >
> > 1. The srcu_struct will be initialized before it is used, or
> >
> > 2. When it is used before initialization, the system is running
> >
On Fri, Nov 17, 2006 at 02:27:15PM -0500, Alan Stern wrote:
> On Fri, 17 Nov 2006, Paul E. McKenney wrote:
>
> > > It works for me, but the overhead is still large. Before it would take
> > > 8-12 jiffies for a synchronize_srcu() to complete without there actually
> > > being any reader locks acti
On Fri, Nov 17, 2006 at 09:39:45PM +0300, Oleg Nesterov wrote:
> Paul E. McKenney wrote:
> >
> > int srcu_read_lock(struct srcu_struct *sp)
> > {
> > @@ -112,11 +126,24 @@ int srcu_read_lock(struct srcu_struct *s
> >
> > preempt_disable();
> > idx = sp->completed & 0x1;
> > - barrier(
On Fri, 17 Nov 2006, Paul E. McKenney wrote:
> > It works for me, but the overhead is still large. Before it would take
> > 8-12 jiffies for a synchronize_srcu() to complete without there actually
> > being any reader locks active, now it takes 2-3 jiffies. So it's
> > definitely faster, and as su
On Fri, Nov 17, 2006 at 10:29:25AM +0100, Jens Axboe wrote:
> On Thu, Nov 16 2006, Paul E. McKenney wrote:
> > On Thu, Nov 16, 2006 at 10:06:25PM -0500, Alan Stern wrote:
> > > On Thu, 16 Nov 2006, Linus Torvalds wrote:
> > >
> > > >
> > > >
> > > > On Thu, 16 Nov 2006, Alan Stern wrote:
> > > >
Paul E. McKenney wrote:
>
> int srcu_read_lock(struct srcu_struct *sp)
> {
> @@ -112,11 +126,24 @@ int srcu_read_lock(struct srcu_struct *s
>
> preempt_disable();
> idx = sp->completed & 0x1;
> - barrier(); /* ensure compiler looks -once- at sp->completed. */
> - per_cpu_pt
On Thu, Nov 16 2006, Paul E. McKenney wrote:
> On Thu, Nov 16, 2006 at 10:06:25PM -0500, Alan Stern wrote:
> > On Thu, 16 Nov 2006, Linus Torvalds wrote:
> >
> > >
> > >
> > > On Thu, 16 Nov 2006, Alan Stern wrote:
> > > > On Thu, 16 Nov 2006, Linus Torvalds wrote:
> > > > >
> > > > > Paul, it w
On Thu, Nov 16, 2006 at 10:06:25PM -0500, Alan Stern wrote:
> On Thu, 16 Nov 2006, Linus Torvalds wrote:
>
> >
> >
> > On Thu, 16 Nov 2006, Alan Stern wrote:
> > > On Thu, 16 Nov 2006, Linus Torvalds wrote:
> > > >
> > > > Paul, it would be _really_ nice to have some way to just initialize
> >
On Thu, 16 Nov 2006, Linus Torvalds wrote:
>
>
> On Thu, 16 Nov 2006, Alan Stern wrote:
> > On Thu, 16 Nov 2006, Linus Torvalds wrote:
> > >
> > > Paul, it would be _really_ nice to have some way to just initialize
> > > that SRCU thing statically. This kind of crud is just crazy.
> >
> > I lo
On Thu, Nov 16, 2006 at 01:47:48PM -0800, Linus Torvalds wrote:
>
>
> On Thu, 16 Nov 2006, Thomas Gleixner wrote:
> >
> > Here is the i386/sparc fixup
>
> Gag me with a volvo.
No can do -- my wife drives a Ford and my car is a bicycle.
> This is disgusting, but I would actually prefer the fol
On Thu, 16 Nov 2006, Alan Stern wrote:
> On Thu, 16 Nov 2006, Linus Torvalds wrote:
> >
> > Paul, it would be _really_ nice to have some way to just initialize
> > that SRCU thing statically. This kind of crud is just crazy.
>
> I looked into this back when SRCU was first added. It's essential
On Thu, 16 Nov 2006, Linus Torvalds wrote:
> - it makes it clear that this should be fixed, preferably by just having
>some way to initialize SRCU structs staticalyl. If we get that, the fix
>is to just replace the horrible "initialize by hand" with a static
>initializer once and f
On Thu, 16 Nov 2006, Thomas Gleixner wrote:
>
> Here is the i386/sparc fixup
Gag me with a volvo.
This is disgusting, but I would actually prefer the following version over
the patches I've seen, because
- it doesn't end up having any architecture-specific parts
- it doesn't use the new "
On Thu, 2006-11-16 at 13:20 -0800, Andrew Morton wrote:
> >
> > -core_initcall(cpufreq_tsc);
> > +/*
> > + * init_cpufreq_transition_notifier_list() should execute first,
> > + * which is a core_initcall, so mark this one core_initcall_sync:
> > + */
> > +core_initcall_sync(cpufreq_tsc);
>
> Wou
On Thu, 16 Nov 2006 21:15:32 +0100
Ingo Molnar <[EMAIL PROTECTED]> wrote:
>
> * Thomas Gleixner <[EMAIL PROTECTED]> wrote:
>
> > [PATCH] cpufreq: make the transition_notifier chain use SRCU
> > (b4dfdbb3c707474a2254c5b4d7e62be31a4b7da9)
> >
> > breaks cpu frequency notification users, which reg
On Thu, 2006-11-16 at 21:15 +0100, Ingo Molnar wrote:
> From: Ingo Molnar <[EMAIL PROTECTED]>
> Subject: [patch] cpufreq: mark cpufreq_tsc() as core_initcall_sync
>
> init_cpufreq_transition_notifier_list() should execute first, which is a
> core_initcall, so mark cpufreq_tsc() core_initcall_syn
On Thu, 2006-11-16 at 21:15 +0100, Ingo Molnar wrote:
> From: Ingo Molnar <[EMAIL PROTECTED]>
> Subject: [patch] cpufreq: mark cpufreq_tsc() as core_initcall_sync
>
> init_cpufreq_transition_notifier_list() should execute first, which is a
> core_initcall, so mark cpufreq_tsc() core_initcall_sync
56 matches
Mail list logo
