The rcu_tasks_percpu structure's->lazy_timer is queued only when
the rcu_tasks structure's->lazy_jiffies is not equal to zero in
call_rcu_tasks_generic(), if the lazy_timer callback is invoked,
that means the lazy_jiffes is not equal to zero, this commit
therefore remove lazy_jiffies check in call_rcu_tasks_generic_timer().
Signed-off-by: Zqiang <[email protected]>
---
kernel/rcu/tasks.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index b1254cf3c210..439e0b9a2656 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -299,7 +299,7 @@ static void call_rcu_tasks_generic_timer(struct timer_list *tlp)
rtp = rtpcp->rtpp;
raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
- if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
+ if (!rcu_segcblist_empty(&rtpcp->cblist)) {
if (!rtpcp->urgent_gp)
rtpcp->urgent_gp = 1;
needwake = true;
--
2.17.1
On Tue, Mar 12, 2024 at 07:35:24PM +0800, Zqiang wrote:
> The rcu_tasks_percpu structure's->lazy_timer is queued only when
> the rcu_tasks structure's->lazy_jiffies is not equal to zero in
> call_rcu_tasks_generic(), if the lazy_timer callback is invoked,
> that means the lazy_jiffes is not equal to zero, this commit
> therefore remove lazy_jiffies check in call_rcu_tasks_generic_timer().
>
> Signed-off-by: Zqiang <[email protected]>
> ---
> kernel/rcu/tasks.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index b1254cf3c210..439e0b9a2656 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -299,7 +299,7 @@ static void call_rcu_tasks_generic_timer(struct timer_list *tlp)
>
> rtp = rtpcp->rtpp;
> raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> - if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
> + if (!rcu_segcblist_empty(&rtpcp->cblist)) {
Good eyes!
But did you test with something like a WARN_ON_ONCE(rtp->lazy_jiffies)?
Thanx, Paul
> if (!rtpcp->urgent_gp)
> rtpcp->urgent_gp = 1;
> needwake = true;
> --
> 2.17.1
>
>
> On Tue, Mar 12, 2024 at 07:35:24PM +0800, Zqiang wrote:
> > The rcu_tasks_percpu structure's->lazy_timer is queued only when
> > the rcu_tasks structure's->lazy_jiffies is not equal to zero in
> > call_rcu_tasks_generic(), if the lazy_timer callback is invoked,
> > that means the lazy_jiffes is not equal to zero, this commit
> > therefore remove lazy_jiffies check in call_rcu_tasks_generic_timer().
> >
> > Signed-off-by: Zqiang <[email protected]>
> > ---
> > kernel/rcu/tasks.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > index b1254cf3c210..439e0b9a2656 100644
> > --- a/kernel/rcu/tasks.h
> > +++ b/kernel/rcu/tasks.h
> > @@ -299,7 +299,7 @@ static void call_rcu_tasks_generic_timer(struct timer_list *tlp)
> >
> > rtp = rtpcp->rtpp;
> > raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> > - if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
> > + if (!rcu_segcblist_empty(&rtpcp->cblist)) {
>
> Good eyes!
>
> But did you test with something like a WARN_ON_ONCE(rtp->lazy_jiffies)?
Hi, Paul
+ if (!rcu_segcblist_empty(&rtpcp->cblist) &&
!WARN_ON_ONCE(!rtp->lazy_jiffies))
I've done tests like this:
1. runqemu nographic kvm slirp qemuparams="-smp 4 -m 2048M -drive
file=$PWD/share.img,if=virtio"
bootparams="rcupdate.rcu_tasks_trace_lazy_ms=0" -d
2. insmod torture.ko
insmod rcutorture.ko torture_type=tasks-tracing fwd_progress=4
3. bpftrace -e 't:timer:timer_expire_entry /args->function ==
kaddr("call_rcu_tasks_generic_timer")/
{
printf("comm:%s,cpu:%d,stack:%s,func:%s\n", comm, cpu, kstack,
ksym(args->function)); }'
The call_rcu_tasks_generic_timer() has never been executed.
Thanks
Zqiang
>
> Thanx, Paul
>
> > if (!rtpcp->urgent_gp)
> > rtpcp->urgent_gp = 1;
> > needwake = true;
> > --
> > 2.17.1
> >
On Wed, Mar 13, 2024 at 12:49:50PM +0800, Z qiang wrote:
> >
> > On Tue, Mar 12, 2024 at 07:35:24PM +0800, Zqiang wrote:
> > > The rcu_tasks_percpu structure's->lazy_timer is queued only when
> > > the rcu_tasks structure's->lazy_jiffies is not equal to zero in
> > > call_rcu_tasks_generic(), if the lazy_timer callback is invoked,
> > > that means the lazy_jiffes is not equal to zero, this commit
> > > therefore remove lazy_jiffies check in call_rcu_tasks_generic_timer().
> > >
> > > Signed-off-by: Zqiang <[email protected]>
> > > ---
> > > kernel/rcu/tasks.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > index b1254cf3c210..439e0b9a2656 100644
> > > --- a/kernel/rcu/tasks.h
> > > +++ b/kernel/rcu/tasks.h
> > > @@ -299,7 +299,7 @@ static void call_rcu_tasks_generic_timer(struct timer_list *tlp)
> > >
> > > rtp = rtpcp->rtpp;
> > > raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> > > - if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
> > > + if (!rcu_segcblist_empty(&rtpcp->cblist)) {
> >
> > Good eyes!
> >
> > But did you test with something like a WARN_ON_ONCE(rtp->lazy_jiffies)?
>
> Hi, Paul
>
> + if (!rcu_segcblist_empty(&rtpcp->cblist) &&
> !WARN_ON_ONCE(!rtp->lazy_jiffies))
>
> I've done tests like this:
>
> 1. runqemu nographic kvm slirp qemuparams="-smp 4 -m 2048M -drive
> file=$PWD/share.img,if=virtio"
> bootparams="rcupdate.rcu_tasks_trace_lazy_ms=0" -d
>
> 2. insmod torture.ko
> insmod rcutorture.ko torture_type=tasks-tracing fwd_progress=4
>
> 3. bpftrace -e 't:timer:timer_expire_entry /args->function ==
> kaddr("call_rcu_tasks_generic_timer")/
> {
> printf("comm:%s,cpu:%d,stack:%s,func:%s\n", comm, cpu, kstack,
> ksym(args->function)); }'
>
> The call_rcu_tasks_generic_timer() has never been executed.
Very good!
Then if we get a couple of acks or reviews from the others acknowledging
that if they ever make ->lazy_jiffies be changeable at runtime, they
will remember to do something to adjust this logic appropriately, I will
take it. ;-)
Thanx, Paul
> Thanks
> Zqiang
>
>
> >
> > Thanx, Paul
> >
> > > if (!rtpcp->urgent_gp)
> > > rtpcp->urgent_gp = 1;
> > > needwake = true;
> > > --
> > > 2.17.1
> > >
>
> On Wed, Mar 13, 2024 at 12:49:50PM +0800, Z qiang wrote:
> > >
> > > On Tue, Mar 12, 2024 at 07:35:24PM +0800, Zqiang wrote:
> > > > The rcu_tasks_percpu structure's->lazy_timer is queued only when
> > > > the rcu_tasks structure's->lazy_jiffies is not equal to zero in
> > > > call_rcu_tasks_generic(), if the lazy_timer callback is invoked,
> > > > that means the lazy_jiffes is not equal to zero, this commit
> > > > therefore remove lazy_jiffies check in call_rcu_tasks_generic_timer().
> > > >
> > > > Signed-off-by: Zqiang <[email protected]>
> > > > ---
> > > > kernel/rcu/tasks.h | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > > index b1254cf3c210..439e0b9a2656 100644
> > > > --- a/kernel/rcu/tasks.h
> > > > +++ b/kernel/rcu/tasks.h
> > > > @@ -299,7 +299,7 @@ static void call_rcu_tasks_generic_timer(struct timer_list *tlp)
> > > >
> > > > rtp = rtpcp->rtpp;
> > > > raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> > > > - if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
> > > > + if (!rcu_segcblist_empty(&rtpcp->cblist)) {
> > >
> > > Good eyes!
> > >
> > > But did you test with something like a WARN_ON_ONCE(rtp->lazy_jiffies)?
> >
> > Hi, Paul
> >
> > + if (!rcu_segcblist_empty(&rtpcp->cblist) &&
> > !WARN_ON_ONCE(!rtp->lazy_jiffies))
> >
> > I've done tests like this:
> >
> > 1. runqemu nographic kvm slirp qemuparams="-smp 4 -m 2048M -drive
> > file=$PWD/share.img,if=virtio"
> > bootparams="rcupdate.rcu_tasks_trace_lazy_ms=0" -d
> >
> > 2. insmod torture.ko
> > insmod rcutorture.ko torture_type=tasks-tracing fwd_progress=4
> >
> > 3. bpftrace -e 't:timer:timer_expire_entry /args->function ==
> > kaddr("call_rcu_tasks_generic_timer")/
> > {
> > printf("comm:%s,cpu:%d,stack:%s,func:%s\n", comm, cpu, kstack,
> > ksym(args->function)); }'
> >
> > The call_rcu_tasks_generic_timer() has never been executed.
>
> Very good!
>
> Then if we get a couple of acks or reviews from the others acknowledging
> that if they ever make ->lazy_jiffies be changeable at runtime, they
> will remember to do something to adjust this logic appropriately, I will
> take it. ;-)
>
Hi, Paul
Would it be better to modify it as follows? set needwake not
depend on lazy_jiffies, if ->lazy_jiffies be changed at runtime,
and set it to zero, will miss the chance to wake up gp kthreads.
diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index e7ac9138a4fd..aea2b71af36c 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -299,11 +299,12 @@ static void call_rcu_tasks_generic_timer(struct
timer_list *tlp)
rtp = rtpcp->rtpp;
raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
- if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
+ if (!rcu_segcblist_empty(&rtpcp->cblist)) {
if (!rtpcp->urgent_gp)
rtpcp->urgent_gp = 1;
needwake = true;
- mod_timer(&rtpcp->lazy_timer, rcu_tasks_lazy_time(rtp));
+ if (rtp->lazy_jiffies)
+ mod_timer(&rtpcp->lazy_timer, rcu_tasks_lazy_time(rtp));
}
raw_spin_unlock_irqrestore_rcu_node(rtpcp, flags);
if (needwake)
Thanks
Zqiang
> Thanx, Paul
>
> > Thanks
> > Zqiang
> >
> >
> > >
> > > Thanx, Paul
> > >
> > > > if (!rtpcp->urgent_gp)
> > > > rtpcp->urgent_gp = 1;
> > > > needwake = true;
> > > > --
> > > > 2.17.1
> > > >
On Sun, Mar 17, 2024 at 10:31:22PM +0800, Z qiang wrote:
> > On Wed, Mar 13, 2024 at 12:49:50PM +0800, Z qiang wrote:
> > > > On Tue, Mar 12, 2024 at 07:35:24PM +0800, Zqiang wrote:
> > > > > The rcu_tasks_percpu structure's->lazy_timer is queued only when
> > > > > the rcu_tasks structure's->lazy_jiffies is not equal to zero in
> > > > > call_rcu_tasks_generic(), if the lazy_timer callback is invoked,
> > > > > that means the lazy_jiffes is not equal to zero, this commit
> > > > > therefore remove lazy_jiffies check in call_rcu_tasks_generic_timer().
> > > > >
> > > > > Signed-off-by: Zqiang <[email protected]>
> > > > > ---
> > > > > kernel/rcu/tasks.h | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > > > index b1254cf3c210..439e0b9a2656 100644
> > > > > --- a/kernel/rcu/tasks.h
> > > > > +++ b/kernel/rcu/tasks.h
> > > > > @@ -299,7 +299,7 @@ static void call_rcu_tasks_generic_timer(struct timer_list *tlp)
> > > > >
> > > > > rtp = rtpcp->rtpp;
> > > > > raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> > > > > - if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
> > > > > + if (!rcu_segcblist_empty(&rtpcp->cblist)) {
> > > >
> > > > Good eyes!
> > > >
> > > > But did you test with something like a WARN_ON_ONCE(rtp->lazy_jiffies)?
> > >
> > > Hi, Paul
> > >
> > > + if (!rcu_segcblist_empty(&rtpcp->cblist) &&
> > > !WARN_ON_ONCE(!rtp->lazy_jiffies))
> > >
> > > I've done tests like this:
> > >
> > > 1. runqemu nographic kvm slirp qemuparams="-smp 4 -m 2048M -drive
> > > file=$PWD/share.img,if=virtio"
> > > bootparams="rcupdate.rcu_tasks_trace_lazy_ms=0" -d
> > >
> > > 2. insmod torture.ko
> > > insmod rcutorture.ko torture_type=tasks-tracing fwd_progress=4
> > >
> > > 3. bpftrace -e 't:timer:timer_expire_entry /args->function ==
> > > kaddr("call_rcu_tasks_generic_timer")/
> > > {
> > > printf("comm:%s,cpu:%d,stack:%s,func:%s\n", comm, cpu, kstack,
> > > ksym(args->function)); }'
> > >
> > > The call_rcu_tasks_generic_timer() has never been executed.
> >
> > Very good!
> >
> > Then if we get a couple of acks or reviews from the others acknowledging
> > that if they ever make ->lazy_jiffies be changeable at runtime, they
> > will remember to do something to adjust this logic appropriately, I will
> > take it. ;-)
> >
>
> Hi, Paul
>
> Would it be better to modify it as follows? set needwake not
> depend on lazy_jiffies, if ->lazy_jiffies be changed at runtime,
> and set it to zero, will miss the chance to wake up gp kthreads.
>
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index e7ac9138a4fd..aea2b71af36c 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -299,11 +299,12 @@ static void call_rcu_tasks_generic_timer(struct
> timer_list *tlp)
>
> rtp = rtpcp->rtpp;
> raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> - if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
> + if (!rcu_segcblist_empty(&rtpcp->cblist)) {
> if (!rtpcp->urgent_gp)
> rtpcp->urgent_gp = 1;
> needwake = true;
> - mod_timer(&rtpcp->lazy_timer, rcu_tasks_lazy_time(rtp));
> + if (rtp->lazy_jiffies)
> + mod_timer(&rtpcp->lazy_timer, rcu_tasks_lazy_time(rtp));
> }
> raw_spin_unlock_irqrestore_rcu_node(rtpcp, flags);
> if (needwake)
Interesting approach!
But how does that avoid defeating laziness by doing the wakeup early?
Thanx, Paul
> Thanks
> Zqiang
>
>
> > Thanx, Paul
> >
> > > Thanks
> > > Zqiang
> > >
> > >
> > > >
> > > > Thanx, Paul
> > > >
> > > > > if (!rtpcp->urgent_gp)
> > > > > rtpcp->urgent_gp = 1;
> > > > > needwake = true;
> > > > > --
> > > > > 2.17.1
> > > > >
>
> On Sun, Mar 17, 2024 at 10:31:22PM +0800, Z qiang wrote:
> > > On Wed, Mar 13, 2024 at 12:49:50PM +0800, Z qiang wrote:
> > > > > On Tue, Mar 12, 2024 at 07:35:24PM +0800, Zqiang wrote:
> > > > > > The rcu_tasks_percpu structure's->lazy_timer is queued only when
> > > > > > the rcu_tasks structure's->lazy_jiffies is not equal to zero in
> > > > > > call_rcu_tasks_generic(), if the lazy_timer callback is invoked,
> > > > > > that means the lazy_jiffes is not equal to zero, this commit
> > > > > > therefore remove lazy_jiffies check in call_rcu_tasks_generic_timer().
> > > > > >
> > > > > > Signed-off-by: Zqiang <[email protected]>
> > > > > > ---
> > > > > > kernel/rcu/tasks.h | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > > > > index b1254cf3c210..439e0b9a2656 100644
> > > > > > --- a/kernel/rcu/tasks.h
> > > > > > +++ b/kernel/rcu/tasks.h
> > > > > > @@ -299,7 +299,7 @@ static void call_rcu_tasks_generic_timer(struct timer_list *tlp)
> > > > > >
> > > > > > rtp = rtpcp->rtpp;
> > > > > > raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> > > > > > - if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
> > > > > > + if (!rcu_segcblist_empty(&rtpcp->cblist)) {
> > > > >
> > > > > Good eyes!
> > > > >
> > > > > But did you test with something like a WARN_ON_ONCE(rtp->lazy_jiffies)?
> > > >
> > > > Hi, Paul
> > > >
> > > > + if (!rcu_segcblist_empty(&rtpcp->cblist) &&
> > > > !WARN_ON_ONCE(!rtp->lazy_jiffies))
> > > >
> > > > I've done tests like this:
> > > >
> > > > 1. runqemu nographic kvm slirp qemuparams="-smp 4 -m 2048M -drive
> > > > file=$PWD/share.img,if=virtio"
> > > > bootparams="rcupdate.rcu_tasks_trace_lazy_ms=0" -d
> > > >
> > > > 2. insmod torture.ko
> > > > insmod rcutorture.ko torture_type=tasks-tracing fwd_progress=4
> > > >
> > > > 3. bpftrace -e 't:timer:timer_expire_entry /args->function ==
> > > > kaddr("call_rcu_tasks_generic_timer")/
> > > > {
> > > > printf("comm:%s,cpu:%d,stack:%s,func:%s\n", comm, cpu, kstack,
> > > > ksym(args->function)); }'
> > > >
> > > > The call_rcu_tasks_generic_timer() has never been executed.
> > >
> > > Very good!
> > >
> > > Then if we get a couple of acks or reviews from the others acknowledging
> > > that if they ever make ->lazy_jiffies be changeable at runtime, they
> > > will remember to do something to adjust this logic appropriately, I will
> > > take it. ;-)
> > >
> >
> > Hi, Paul
> >
> > Would it be better to modify it as follows? set needwake not
> > depend on lazy_jiffies, if ->lazy_jiffies be changed at runtime,
> > and set it to zero, will miss the chance to wake up gp kthreads.
> >
> > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > index e7ac9138a4fd..aea2b71af36c 100644
> > --- a/kernel/rcu/tasks.h
> > +++ b/kernel/rcu/tasks.h
> > @@ -299,11 +299,12 @@ static void call_rcu_tasks_generic_timer(struct
> > timer_list *tlp)
> >
> > rtp = rtpcp->rtpp;
> > raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> > - if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
> > + if (!rcu_segcblist_empty(&rtpcp->cblist)) {
> > if (!rtpcp->urgent_gp)
> > rtpcp->urgent_gp = 1;
> > needwake = true;
> > - mod_timer(&rtpcp->lazy_timer, rcu_tasks_lazy_time(rtp));
> > + if (rtp->lazy_jiffies)
> > + mod_timer(&rtpcp->lazy_timer, rcu_tasks_lazy_time(rtp));
> > }
> > raw_spin_unlock_irqrestore_rcu_node(rtpcp, flags);
> > if (needwake)
>
> Interesting approach!
>
> But how does that avoid defeating laziness by doing the wakeup early?
Hello, Paul
Does this mean that if cblist is empty, we will miss the opportunity to
enqueue the timer again? If so, we can move mod_timer() outside
the if condition.
or I didn't understand your question?
Thanks
Zqiang
>
> Thanx, Paul
>
> > Thanks
> > Zqiang
> >
> >
> > > Thanx, Paul
> > >
> > > > Thanks
> > > > Zqiang
> > > >
> > > >
> > > > >
> > > > > Thanx, Paul
> > > > >
> > > > > > if (!rtpcp->urgent_gp)
> > > > > > rtpcp->urgent_gp = 1;
> > > > > > needwake = true;
> > > > > > --
> > > > > > 2.17.1
> > > > > >
On Mon, Mar 18, 2024 at 10:35:44AM +0800, Z qiang wrote:
> > On Sun, Mar 17, 2024 at 10:31:22PM +0800, Z qiang wrote:
> > > > On Wed, Mar 13, 2024 at 12:49:50PM +0800, Z qiang wrote:
> > > > > > On Tue, Mar 12, 2024 at 07:35:24PM +0800, Zqiang wrote:
> > > > > > > The rcu_tasks_percpu structure's->lazy_timer is queued only when
> > > > > > > the rcu_tasks structure's->lazy_jiffies is not equal to zero in
> > > > > > > call_rcu_tasks_generic(), if the lazy_timer callback is invoked,
> > > > > > > that means the lazy_jiffes is not equal to zero, this commit
> > > > > > > therefore remove lazy_jiffies check in call_rcu_tasks_generic_timer().
> > > > > > >
> > > > > > > Signed-off-by: Zqiang <[email protected]>
> > > > > > > ---
> > > > > > > kernel/rcu/tasks.h | 2 +-
> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > > > > > index b1254cf3c210..439e0b9a2656 100644
> > > > > > > --- a/kernel/rcu/tasks.h
> > > > > > > +++ b/kernel/rcu/tasks.h
> > > > > > > @@ -299,7 +299,7 @@ static void call_rcu_tasks_generic_timer(struct timer_list *tlp)
> > > > > > >
> > > > > > > rtp = rtpcp->rtpp;
> > > > > > > raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> > > > > > > - if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
> > > > > > > + if (!rcu_segcblist_empty(&rtpcp->cblist)) {
> > > > > >
> > > > > > Good eyes!
> > > > > >
> > > > > > But did you test with something like a WARN_ON_ONCE(rtp->lazy_jiffies)?
> > > > >
> > > > > Hi, Paul
> > > > >
> > > > > + if (!rcu_segcblist_empty(&rtpcp->cblist) &&
> > > > > !WARN_ON_ONCE(!rtp->lazy_jiffies))
> > > > >
> > > > > I've done tests like this:
> > > > >
> > > > > 1. runqemu nographic kvm slirp qemuparams="-smp 4 -m 2048M -drive
> > > > > file=$PWD/share.img,if=virtio"
> > > > > bootparams="rcupdate.rcu_tasks_trace_lazy_ms=0" -d
> > > > >
> > > > > 2. insmod torture.ko
> > > > > insmod rcutorture.ko torture_type=tasks-tracing fwd_progress=4
> > > > >
> > > > > 3. bpftrace -e 't:timer:timer_expire_entry /args->function ==
> > > > > kaddr("call_rcu_tasks_generic_timer")/
> > > > > {
> > > > > printf("comm:%s,cpu:%d,stack:%s,func:%s\n", comm, cpu, kstack,
> > > > > ksym(args->function)); }'
> > > > >
> > > > > The call_rcu_tasks_generic_timer() has never been executed.
> > > >
> > > > Very good!
> > > >
> > > > Then if we get a couple of acks or reviews from the others acknowledging
> > > > that if they ever make ->lazy_jiffies be changeable at runtime, they
> > > > will remember to do something to adjust this logic appropriately, I will
> > > > take it. ;-)
> > > >
> > >
> > > Hi, Paul
> > >
> > > Would it be better to modify it as follows? set needwake not
> > > depend on lazy_jiffies, if ->lazy_jiffies be changed at runtime,
> > > and set it to zero, will miss the chance to wake up gp kthreads.
> > >
> > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> > > index e7ac9138a4fd..aea2b71af36c 100644
> > > --- a/kernel/rcu/tasks.h
> > > +++ b/kernel/rcu/tasks.h
> > > @@ -299,11 +299,12 @@ static void call_rcu_tasks_generic_timer(struct
> > > timer_list *tlp)
> > >
> > > rtp = rtpcp->rtpp;
> > > raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> > > - if (!rcu_segcblist_empty(&rtpcp->cblist) && rtp->lazy_jiffies) {
> > > + if (!rcu_segcblist_empty(&rtpcp->cblist)) {
> > > if (!rtpcp->urgent_gp)
> > > rtpcp->urgent_gp = 1;
> > > needwake = true;
> > > - mod_timer(&rtpcp->lazy_timer, rcu_tasks_lazy_time(rtp));
> > > + if (rtp->lazy_jiffies)
> > > + mod_timer(&rtpcp->lazy_timer, rcu_tasks_lazy_time(rtp));
> > > }
> > > raw_spin_unlock_irqrestore_rcu_node(rtpcp, flags);
> > > if (needwake)
> >
> > Interesting approach!
> >
> > But how does that avoid defeating laziness by doing the wakeup early?
>
> Hello, Paul
>
> Does this mean that if cblist is empty, we will miss the opportunity to
> enqueue the timer again? If so, we can move mod_timer() outside
> the if condition.
> or I didn't understand your question?
Never mind! I was getting confused, and forgetting that this code has
already seen a timeout.
Let's see what others think.
Thanx, Paul
> Thanks
> Zqiang
>
>
> >
> > Thanx, Paul
> >
> > > Thanks
> > > Zqiang
> > >
> > >
> > > > Thanx, Paul
> > > >
> > > > > Thanks
> > > > > Zqiang
> > > > >
> > > > >
> > > > > >
> > > > > > Thanx, Paul
> > > > > >
> > > > > > > if (!rtpcp->urgent_gp)
> > > > > > > rtpcp->urgent_gp = 1;
> > > > > > > needwake = true;
> > > > > > > --
> > > > > > > 2.17.1
> > > > > > >