Re: call_rcu from trace_preempt

2015-06-17 Thread Paul E. McKenney
On Wed, Jun 17, 2015 at 04:58:48PM -0700, Alexei Starovoitov wrote: > On 6/17/15 2:36 PM, Paul E. McKenney wrote: > >Well, you do need to have something in each element to allow them to be > >tracked. You could indeed use llist_add() to maintain the per-CPU list, > >and then use llist_del_all() bu

Re: call_rcu from trace_preempt

2015-06-17 Thread Alexei Starovoitov
On 6/17/15 2:36 PM, Paul E. McKenney wrote: Well, you do need to have something in each element to allow them to be tracked. You could indeed use llist_add() to maintain the per-CPU list, and then use llist_del_all() bulk-remove all the elements from the per-CPU list. You can then pass each ele

Re: call_rcu from trace_preempt

2015-06-17 Thread Paul E. McKenney
On Wed, Jun 17, 2015 at 01:53:17PM -0700, Alexei Starovoitov wrote: > On 6/17/15 1:37 PM, Paul E. McKenney wrote: > >On Wed, Jun 17, 2015 at 11:39:29AM -0700, Alexei Starovoitov wrote: > >>On 6/17/15 2:05 AM, Daniel Wagner wrote: > >Steven's suggestion deferring the work via irq_work results in

Re: call_rcu from trace_preempt

2015-06-17 Thread Alexei Starovoitov
On 6/17/15 1:37 PM, Paul E. McKenney wrote: On Wed, Jun 17, 2015 at 11:39:29AM -0700, Alexei Starovoitov wrote: On 6/17/15 2:05 AM, Daniel Wagner wrote: Steven's suggestion deferring the work via irq_work results in the same stack trace. (Now I get cold feets, without the nice heat from the CPU

Re: call_rcu from trace_preempt

2015-06-17 Thread Paul E. McKenney
On Wed, Jun 17, 2015 at 11:39:29AM -0700, Alexei Starovoitov wrote: > On 6/17/15 2:05 AM, Daniel Wagner wrote: > >>>Steven's suggestion deferring the work via irq_work results in the same > >>>stack trace. (Now I get cold feets, without the nice heat from the CPU > >>>busy looping...) > >That one s

Re: call_rcu from trace_preempt

2015-06-17 Thread Alexei Starovoitov
On 6/17/15 2:05 AM, Daniel Wagner wrote: >Steven's suggestion deferring the work via irq_work results in the same >stack trace. (Now I get cold feets, without the nice heat from the CPU >busy looping...) That one still not working. It also makes the system really really slow. I guess I still do

Re: call_rcu from trace_preempt

2015-06-17 Thread Daniel Wagner
On 06/17/2015 10:11 AM, Daniel Wagner wrote: > On 06/16/2015 07:20 PM, Alexei Starovoitov wrote: >> On 6/16/15 5:38 AM, Daniel Wagner wrote: >>> static int free_thread(void *arg) >>> +{ >>> +unsigned long flags; >>> +struct htab_elem *l; >>> + >>> +while (!kthread_should_stop()) { >>> +

Re: call_rcu from trace_preempt

2015-06-17 Thread Daniel Wagner
On 06/16/2015 07:20 PM, Alexei Starovoitov wrote: > On 6/16/15 5:38 AM, Daniel Wagner wrote: >> static int free_thread(void *arg) >> +{ >> +unsigned long flags; >> +struct htab_elem *l; >> + >> +while (!kthread_should_stop()) { >> +spin_lock_irqsave(&elem_freelist_lock, flags);

Re: call_rcu from trace_preempt

2015-06-16 Thread Steven Rostedt
On Tue, 16 Jun 2015 18:04:39 -0700 Alexei Starovoitov wrote: > > you mean similar to what rb_wakeups() and friends are doing? > makes sense. starting to study it... Yes, I meant those functions. But yours would be slightly different. As it would be the one calling the irq work that would be sett

Re: call_rcu from trace_preempt

2015-06-16 Thread Alexei Starovoitov
On 6/16/15 5:47 PM, Steven Rostedt wrote: Do what I do in tracing. Use a bit (per cpu?) test. Add the element to the list (that will be a cmpxchg, but I'm not sure you can avoid it), then check the bit to see if the irq work is already been activated. If not, then activate the irq work and set t

Re: call_rcu from trace_preempt

2015-06-16 Thread Steven Rostedt
On Tue, 16 Jun 2015 17:33:24 -0700 Alexei Starovoitov wrote: > On 6/16/15 10:37 AM, Steven Rostedt wrote: > >>> + kfree(l); > >> > > >> >that's not right, since such thread defeats rcu protection of lookup. > >> >We need either kfree_rcu/call_rcu or synchronize_rcu. > >> >Obviousl

Re: call_rcu from trace_preempt

2015-06-16 Thread Alexei Starovoitov
On 6/16/15 10:37 AM, Steven Rostedt wrote: + kfree(l); > >that's not right, since such thread defeats rcu protection of lookup. >We need either kfree_rcu/call_rcu or synchronize_rcu. >Obviously the former is preferred that's why I'm still digging into it. >Probably a thread

Re: call_rcu from trace_preempt

2015-06-16 Thread Paul E. McKenney
On Tue, Jun 16, 2015 at 03:29:25PM -0400, Steven Rostedt wrote: > On Tue, 16 Jun 2015 12:20:55 -0700 > "Paul E. McKenney" wrote: > > > > I'm fine with it. But doesn't Acked-by go above Signed-off-by? > > > > I have done it both ways, usually in time order. > > You're either a Big-endian, or a L

Re: call_rcu from trace_preempt

2015-06-16 Thread Steven Rostedt
On Tue, 16 Jun 2015 12:20:55 -0700 "Paul E. McKenney" wrote: > > I'm fine with it. But doesn't Acked-by go above Signed-off-by? > > I have done it both ways, usually in time order. You're either a Big-endian, or a Little-endian? You can't be both! -- Steve -- To unsubscribe from this list: sen

Re: call_rcu from trace_preempt

2015-06-16 Thread Paul E. McKenney
On Tue, Jun 16, 2015 at 02:57:44PM -0400, Steven Rostedt wrote: > On Tue, 16 Jun 2015 10:39:42 -0700 > "Paul E. McKenney" wrote: > > > > >>If rcu_is_watching() and __rcu_is_watching() are both marked as > > > >>notrace, it makes sense to use preempt_disable/enable_notrace() as it > > > >>otherwis

Re: call_rcu from trace_preempt

2015-06-16 Thread Steven Rostedt
On Tue, 16 Jun 2015 10:39:42 -0700 "Paul E. McKenney" wrote: > > >>If rcu_is_watching() and __rcu_is_watching() are both marked as > > >>notrace, it makes sense to use preempt_disable/enable_notrace() as it > > >>otherwise defeats the purpose of the notrace markers on rcu_is_watching. > > And __

Re: call_rcu from trace_preempt

2015-06-16 Thread Paul E. McKenney
On Tue, Jun 16, 2015 at 10:14:08AM -0700, Alexei Starovoitov wrote: > On 6/16/15 9:05 AM, Paul E. McKenney wrote: > >On Tue, Jun 16, 2015 at 11:37:38AM -0400, Steven Rostedt wrote: > >>On Tue, 16 Jun 2015 05:27:33 -0700 > >>"Paul E. McKenney" wrote: > >> > >>>On Mon, Jun 15, 2015 at 10:45:05PM -07

Re: call_rcu from trace_preempt

2015-06-16 Thread Steven Rostedt
On Tue, 16 Jun 2015 10:20:05 -0700 Alexei Starovoitov wrote: > On 6/16/15 5:38 AM, Daniel Wagner wrote: > > static int free_thread(void *arg) > > +{ > > + unsigned long flags; > > + struct htab_elem *l; > > + > > + while (!kthread_should_stop()) { > > + spin_lock_irqsave(&elem_fre

Re: call_rcu from trace_preempt

2015-06-16 Thread Alexei Starovoitov
On 6/16/15 5:38 AM, Daniel Wagner wrote: static int free_thread(void *arg) +{ + unsigned long flags; + struct htab_elem *l; + + while (!kthread_should_stop()) { + spin_lock_irqsave(&elem_freelist_lock, flags); + while (!list_empty(&elem_freelist)) { +

Re: call_rcu from trace_preempt

2015-06-16 Thread Alexei Starovoitov
On 6/16/15 9:05 AM, Paul E. McKenney wrote: On Tue, Jun 16, 2015 at 11:37:38AM -0400, Steven Rostedt wrote: On Tue, 16 Jun 2015 05:27:33 -0700 "Paul E. McKenney" wrote: On Mon, Jun 15, 2015 at 10:45:05PM -0700, Alexei Starovoitov wrote: On 6/15/15 7:14 PM, Paul E. McKenney wrote: Why do yo

Re: call_rcu from trace_preempt

2015-06-16 Thread Daniel Wagner
On 06/16/2015 06:07 PM, Paul E. McKenney wrote: On Tue, Jun 16, 2015 at 11:43:42AM -0400, Steven Rostedt wrote: On Tue, 16 Jun 2015 07:16:26 -0700 "Paul E. McKenney" wrote: Just for the record: Using a thread for freeing the memory is curing the problem without the need to modify rcu_is_watch

Re: call_rcu from trace_preempt

2015-06-16 Thread Daniel Wagner
On 06/16/2015 05:41 PM, Steven Rostedt wrote: On Tue, 16 Jun 2015 14:38:53 +0200 Daniel Wagner wrote: *map, void *key) if (l) { hlist_del_rcu(&l->hash_node); htab->count--; - kfree_rcu(l, rcu); + /* kfree_rcu(l, rcu); */ So t

Re: call_rcu from trace_preempt

2015-06-16 Thread Paul E. McKenney
On Tue, Jun 16, 2015 at 11:43:42AM -0400, Steven Rostedt wrote: > On Tue, 16 Jun 2015 07:16:26 -0700 > "Paul E. McKenney" wrote: > > > > Just for the record: Using a thread for freeing the memory is curing the > > > problem without the need to modify rcu_is_watching. > > > > I must confess to li

Re: call_rcu from trace_preempt

2015-06-16 Thread Paul E. McKenney
On Tue, Jun 16, 2015 at 11:37:38AM -0400, Steven Rostedt wrote: > On Tue, 16 Jun 2015 05:27:33 -0700 > "Paul E. McKenney" wrote: > > > On Mon, Jun 15, 2015 at 10:45:05PM -0700, Alexei Starovoitov wrote: > > > On 6/15/15 7:14 PM, Paul E. McKenney wrote: > > > > > > > >Why do you believe that it is

Re: call_rcu from trace_preempt

2015-06-16 Thread Steven Rostedt
On Tue, 16 Jun 2015 11:41:51 -0400 Steven Rostedt wrote: > > if (l) { > > hlist_del_rcu(&l->hash_node); > > htab->count--; > > - kfree_rcu(l, rcu); > > + /* kfree_rcu(l, rcu); */ > > So this kfree_rcu() is only being used to defer a free, and has n

Re: call_rcu from trace_preempt

2015-06-16 Thread Steven Rostedt
On Tue, 16 Jun 2015 07:16:26 -0700 "Paul E. McKenney" wrote: > > Just for the record: Using a thread for freeing the memory is curing the > > problem without the need to modify rcu_is_watching. > > I must confess to liking this approach better than guaranteeing full-up > reentrancy in call_rcu()

Re: call_rcu from trace_preempt

2015-06-16 Thread Steven Rostedt
On Tue, 16 Jun 2015 14:38:53 +0200 Daniel Wagner wrote: > diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c > index 83c209d..8d73be3 100644 > --- a/kernel/bpf/hashtab.c > +++ b/kernel/bpf/hashtab.c > @@ -13,6 +13,8 @@ > #include > #include > #include > +#include > +#include > > s

Re: call_rcu from trace_preempt

2015-06-16 Thread Steven Rostedt
On Tue, 16 Jun 2015 05:27:33 -0700 "Paul E. McKenney" wrote: > On Mon, Jun 15, 2015 at 10:45:05PM -0700, Alexei Starovoitov wrote: > > On 6/15/15 7:14 PM, Paul E. McKenney wrote: > > > > > >Why do you believe that it is better to fix it within call_rcu()? > > > > found it: > > diff --git a/kerne

Re: call_rcu from trace_preempt

2015-06-16 Thread Paul E. McKenney
On Tue, Jun 16, 2015 at 02:38:53PM +0200, Daniel Wagner wrote: > On 06/16/2015 02:27 PM, Paul E. McKenney wrote: > > On Mon, Jun 15, 2015 at 10:45:05PM -0700, Alexei Starovoitov wrote: > >> On 6/15/15 7:14 PM, Paul E. McKenney wrote: > >>> > >>> Why do you believe that it is better to fix it within

Re: call_rcu from trace_preempt

2015-06-16 Thread Daniel Wagner
On 06/16/2015 02:27 PM, Paul E. McKenney wrote: > On Mon, Jun 15, 2015 at 10:45:05PM -0700, Alexei Starovoitov wrote: >> On 6/15/15 7:14 PM, Paul E. McKenney wrote: >>> >>> Why do you believe that it is better to fix it within call_rcu()? >> >> found it: >> diff --git a/kernel/rcu/tree.c b/kernel/r

Re: call_rcu from trace_preempt

2015-06-16 Thread Paul E. McKenney
On Mon, Jun 15, 2015 at 10:45:05PM -0700, Alexei Starovoitov wrote: > On 6/15/15 7:14 PM, Paul E. McKenney wrote: > > > >Why do you believe that it is better to fix it within call_rcu()? > > found it: > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index 8cf7304b2867..a3be09d482ae 100644 >

Re: call_rcu from trace_preempt

2015-06-15 Thread Daniel Wagner
On 06/16/2015 08:46 AM, Alexei Starovoitov wrote: > On 6/15/15 11:34 PM, Daniel Wagner wrote: >> On 06/16/2015 08:25 AM, Alexei Starovoitov wrote: >>> On 6/15/15 11:06 PM, Daniel Wagner wrote: > with the above 'fix' the trace.patch is now passing. It still crashes for me with the original

Re: call_rcu from trace_preempt

2015-06-15 Thread Alexei Starovoitov
On 6/15/15 11:34 PM, Daniel Wagner wrote: On 06/16/2015 08:25 AM, Alexei Starovoitov wrote: On 6/15/15 11:06 PM, Daniel Wagner wrote: with the above 'fix' the trace.patch is now passing. It still crashes for me with the original test program [ 145.908013] [] ? __rcu_reclaim+0x101/0x3d0 [ 1

Re: call_rcu from trace_preempt

2015-06-15 Thread Daniel Wagner
On 06/16/2015 08:25 AM, Alexei Starovoitov wrote: > On 6/15/15 11:06 PM, Daniel Wagner wrote: >>> with the above 'fix' the trace.patch is now passing. >> It still crashes for me with the original test program >> >> [ 145.908013] [] ? __rcu_reclaim+0x101/0x3d0 >> [ 145.908013] [] ? rcu_barrier_f

Re: call_rcu from trace_preempt

2015-06-15 Thread Alexei Starovoitov
On 6/15/15 11:06 PM, Daniel Wagner wrote: with the above 'fix' the trace.patch is now passing. It still crashes for me with the original test program [ 145.908013] [] ? __rcu_reclaim+0x101/0x3d0 [ 145.908013] [] ? rcu_barrier_func+0x250/0x250 [ 145.908013] [] ? trace_hardirqs_on_caller+0x

Re: call_rcu from trace_preempt

2015-06-15 Thread Daniel Wagner
On 06/16/2015 07:45 AM, Alexei Starovoitov wrote: > On 6/15/15 7:14 PM, Paul E. McKenney wrote: >> >> Why do you believe that it is better to fix it within call_rcu()? > > found it: > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index 8cf7304b2867..a3be09d482ae 100644 > --- a/kernel/rcu/

Re: call_rcu from trace_preempt

2015-06-15 Thread Alexei Starovoitov
On 6/15/15 7:14 PM, Paul E. McKenney wrote: Why do you believe that it is better to fix it within call_rcu()? found it: diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 8cf7304b2867..a3be09d482ae 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -935,9 +935,9 @@ bool notrace rc

Re: call_rcu from trace_preempt

2015-06-15 Thread Paul E. McKenney
On Mon, Jun 15, 2015 at 06:09:56PM -0700, Alexei Starovoitov wrote: > On 6/15/15 4:07 PM, Paul E. McKenney wrote: > > > >Oh... One important thing is that both call_rcu() and kfree_rcu() > >use per-CPU variables, managing a per-CPU linked list. This is why > >they disable interrupts. If you do a

Re: call_rcu from trace_preempt

2015-06-15 Thread Alexei Starovoitov
On 6/15/15 4:07 PM, Paul E. McKenney wrote: Oh... One important thing is that both call_rcu() and kfree_rcu() use per-CPU variables, managing a per-CPU linked list. This is why they disable interrupts. If you do another call_rcu() in the middle of the first one in just the wrong place, you wi

Re: call_rcu from trace_preempt

2015-06-15 Thread Paul E. McKenney
On Mon, Jun 15, 2015 at 03:24:29PM -0700, Alexei Starovoitov wrote: > Hi Paul, > > I've been debugging the issue reported by Daniel: > http://thread.gmane.org/gmane.linux.kernel/1974304/focus=1974304 > and it seems I narrowed it down to recursive call_rcu. By "recursive call_rcu()", you mean invo