| Message ID | 20231107112023.676016-7-faizal.abdul.rahim@linux.intel.com (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | qbv cycle time extension/truncation | expand |
On Tue, Nov 07, 2023 at 06:20:22AM -0500, Faizal Rahim wrote: > Fix the issue of prematurely setting q->current_entry to NULL in the > setup_first_end_time() function when a new admin schedule arrives > while the oper schedule is still running but hasn't transitioned yet. > This premature setting causes problems because any reference to > q->current_entry, such as in taprio_dequeue(), will result in NULL > during this period, which is incorrect. q->current_entry should remain > valid until the currently running entry expires. > > To address this issue, only set q->current_entry to NULL when there is > no oper schedule currently running. > > Fixes: 5a781ccbd19e ("tc: Add support for configuring the taprio scheduler") The "Fixes" tag represents the commit where the code started becoming a problem, not when the code that you're changing was first introduced. I find it hard to believe that the problem was in commit 5a781ccbd19e ("tc: Add support for configuring the taprio scheduler"), because we didn't support admin -> oper dynamic schedules back then. > Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com> > --- > net/sched/sch_taprio.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c > index 01b114edec30..c60e9e7ac193 100644 > --- a/net/sched/sch_taprio.c > +++ b/net/sched/sch_taprio.c > @@ -1375,7 +1375,8 @@ static void setup_first_end_time(struct taprio_sched *q, > first->gate_close_time[tc] = ktime_add_ns(base, first->gate_duration[tc]); > } > > - rcu_assign_pointer(q->current_entry, NULL); > + if (!hrtimer_active(&q->advance_timer)) > + rcu_assign_pointer(q->current_entry, NULL); > } > > static void taprio_start_sched(struct Qdisc *sch, > -- > 2.25.1 >
On 9/11/2023 7:55 pm, Vladimir Oltean wrote: > On Tue, Nov 07, 2023 at 06:20:22AM -0500, Faizal Rahim wrote: >> Fix the issue of prematurely setting q->current_entry to NULL in the >> setup_first_end_time() function when a new admin schedule arrives >> while the oper schedule is still running but hasn't transitioned yet. >> This premature setting causes problems because any reference to >> q->current_entry, such as in taprio_dequeue(), will result in NULL >> during this period, which is incorrect. q->current_entry should remain >> valid until the currently running entry expires. >> >> To address this issue, only set q->current_entry to NULL when there is >> no oper schedule currently running. >> >> Fixes: 5a781ccbd19e ("tc: Add support for configuring the taprio scheduler") > > The "Fixes" tag represents the commit where the code started becoming a > problem, not when the code that you're changing was first introduced. > > I find it hard to believe that the problem was in commit 5a781ccbd19e > ("tc: Add support for configuring the taprio scheduler"), because we > didn't support admin -> oper dynamic schedules back then. > My bad, will update accordingly. >> Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com> >> --- >> net/sched/sch_taprio.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c >> index 01b114edec30..c60e9e7ac193 100644 >> --- a/net/sched/sch_taprio.c >> +++ b/net/sched/sch_taprio.c >> @@ -1375,7 +1375,8 @@ static void setup_first_end_time(struct taprio_sched *q, >> first->gate_close_time[tc] = ktime_add_ns(base, first->gate_duration[tc]); >> } >> >> - rcu_assign_pointer(q->current_entry, NULL); >> + if (!hrtimer_active(&q->advance_timer)) >> + rcu_assign_pointer(q->current_entry, NULL); >> } >> >> static void taprio_start_sched(struct Qdisc *sch, >> -- >> 2.25.1 >>
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c index 01b114edec30..c60e9e7ac193 100644 --- a/net/sched/sch_taprio.c +++ b/net/sched/sch_taprio.c @@ -1375,7 +1375,8 @@ static void setup_first_end_time(struct taprio_sched *q, first->gate_close_time[tc] = ktime_add_ns(base, first->gate_duration[tc]); } - rcu_assign_pointer(q->current_entry, NULL); + if (!hrtimer_active(&q->advance_timer)) + rcu_assign_pointer(q->current_entry, NULL); } static void taprio_start_sched(struct Qdisc *sch,
Fix the issue of prematurely setting q->current_entry to NULL in the setup_first_end_time() function when a new admin schedule arrives while the oper schedule is still running but hasn't transitioned yet. This premature setting causes problems because any reference to q->current_entry, such as in taprio_dequeue(), will result in NULL during this period, which is incorrect. q->current_entry should remain valid until the currently running entry expires. To address this issue, only set q->current_entry to NULL when there is no oper schedule currently running. Fixes: 5a781ccbd19e ("tc: Add support for configuring the taprio scheduler") Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com> --- net/sched/sch_taprio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)