| Message ID | 20201001205141.8885-5-erez.geva.ext@siemens.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | TC-ETF support PTP clocks series | expand |
On Thu, Oct 01 2020 at 22:51, Erez Geva wrote: Fixes should be at the beginning of a patch series and not be hidden somewhere in the middle. > - As all parameters are unsigned. This is not a sentence and this list style does not make that changelog more readable. > - If 'expires' is bigger than 'last_expires' then the left expression > overflows. This would be the most important information and should be clearly spelled out as problem description at the very beginning of the change log. > - It is better to use addition and check both ends of the range. Is better? Either your change is correcting the problem or not. Just better but incorrect does not cut it. But let's look at the problem itself. The check is about: B <= A <= B + C A, B, C are all unsigned. So if B > A then the result is false. Now lets look at the implementation: if (A - B <= C) return; which works correctly due the wonders of unsigned math. For B <= A the check is obviously correct. If B > A then the result of the unsigned subtraction A - B is a very large positive number which is pretty much guaranteed to be larger than C, i.e. the result is false. So while not immediately obvious, it's still correct. Thanks, tglx
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index 2a76a2f5ed88..ebf59ca1faab 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c @@ -632,7 +632,8 @@ void qdisc_watchdog_schedule_range_ns(struct qdisc_watchdog *wd, u64 expires, /* If timer is already set in [expires, expires + delta_ns], * do not reprogram it. */ - if (wd->last_expires - expires <= delta_ns) + if (wd->last_expires >= expires && + wd->last_expires <= expires + delta_ns) return; }
- As all parameters are unsigned. - If 'expires' is bigger than 'last_expires' then the left expression overflows. - It is better to use addition and check both ends of the range. Signed-off-by: Erez Geva <erez.geva.ext@siemens.com> --- net/sched/sch_api.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)