lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 23 Mar 2022 11:40:26 -0700 From: Josh Poimboeuf <jpoimboe@...hat.com> To: Artem Savkov <asavkov@...hat.com> Cc: tglx@...utronix.de, netdev@...r.kernel.org, davem@...emloft.net, yoshfuji@...ux-ipv6.org, dsahern@...nel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH 1/2] timer: introduce upper bound timers On Wed, Mar 23, 2022 at 12:16:41PM +0100, Artem Savkov wrote: > Add TIMER_UPPER_BOUND flag which allows creation of timers that would > expire at most at specified time or earlier. > > This was previously discussed here: > https://lore.kernel.org/all/20210302001054.4qgrvnkltvkgikzr@treble/T/#u > > Suggested-by: Josh Poimboeuf <jpoimboe@...hat.com> > Signed-off-by: Artem Savkov <asavkov@...hat.com> Thanks Artem, this approach looks good to me. Some kernel style nits... The commit log could use some more background and motivation for the change: a) describe the current timer bound functionality and why it works for most cases b) describe the type of situation where it doesn't work and why c) describe the solution > --- > include/linux/timer.h | 3 ++- > kernel/time/timer.c | 36 ++++++++++++++++++++++-------------- > 2 files changed, 24 insertions(+), 15 deletions(-) > > diff --git a/include/linux/timer.h b/include/linux/timer.h > index fda13c9d1256..9b0963f49528 100644 > --- a/include/linux/timer.h > +++ b/include/linux/timer.h > @@ -67,7 +67,8 @@ struct timer_list { > #define TIMER_DEFERRABLE 0x00080000 > #define TIMER_PINNED 0x00100000 > #define TIMER_IRQSAFE 0x00200000 > -#define TIMER_INIT_FLAGS (TIMER_DEFERRABLE | TIMER_PINNED | TIMER_IRQSAFE) > +#define TIMER_UPPER_BOUND 0x00400000 > +#define TIMER_INIT_FLAGS (TIMER_DEFERRABLE | TIMER_PINNED | TIMER_IRQSAFE | TIMER_UPPER_BOUND) > #define TIMER_ARRAYSHIFT 22 > #define TIMER_ARRAYMASK 0xFFC00000 This new flag needs a comment above, where the other user flags are also commented. > } > return idx; > } > @@ -607,7 +613,8 @@ static void internal_add_timer(struct timer_base *base, struct timer_list *timer > unsigned long bucket_expiry; > unsigned int idx; > > - idx = calc_wheel_index(timer->expires, base->clk, &bucket_expiry); > + idx = calc_wheel_index(timer->expires, base->clk, &bucket_expiry, > + timer->flags & TIMER_UPPER_BOUND); Here and elsewhere, to follow kernel convention, the 2nd line needs spaces after the tabs to align it with the previous line. That makes it more obvious that the 2nd line is just another function argument. Like: idx = calc_wheel_index(timer->expires, base->clk, &bucket_expiry, timer->flags & TIMER_UPPER_BOUND); -- Josh
Powered by blists - more mailing lists