linux-kernel - RE: [PATCH v3 1/2] jiffies: Define secs_to

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 linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
 <SA0PR21MB18673610EE997A0D62048679CA552@SA0PR21MB1867.namprd21.prod.outlook.com>
Date: Thu, 31 Oct 2024 17:28:47 +0000
From: Haiyang Zhang <haiyangz@...rosoft.com>
To: Easwar Hariharan <eahariha@...ux.microsoft.com>, KY Srinivasan
	<kys@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, Dexuan Cui
	<decui@...rosoft.com>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>, Anna-Maria Behnsen
	<anna-maria@...utronix.de>, Thomas Gleixner <tglx@...utronix.de>, Geert
 Uytterhoeven <geert@...ux-m68k.org>, Marcel Holtmann <marcel@...tmann.org>,
	Johan Hedberg <johan.hedberg@...il.com>, Luiz Augusto von Dentz
	<luiz.dentz@...il.com>, "linux-bluetooth@...r.kernel.org"
	<linux-bluetooth@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Praveen Kumar
	<kumarpraveen@...ux.microsoft.com>, Naman Jain <namjain@...ux.microsoft.com>
CC: Michael Kelley <mhklinux@...look.com>, "Von Dentz, Luiz"
	<luiz.von.dentz@...el.com>
Subject: RE: [PATCH v3 1/2] jiffies: Define secs_to_jiffies()



> -----Original Message-----
> From: Easwar Hariharan <eahariha@...ux.microsoft.com>
> Sent: Thursday, October 31, 2024 1:06 PM
> To: Haiyang Zhang <haiyangz@...rosoft.com>; KY Srinivasan
> <kys@...rosoft.com>; Wei Liu <wei.liu@...nel.org>; Dexuan Cui
> <decui@...rosoft.com>; linux-hyperv@...r.kernel.org; Anna-Maria Behnsen
> <anna-maria@...utronix.de>; Thomas Gleixner <tglx@...utronix.de>; Geert
> Uytterhoeven <geert@...ux-m68k.org>; Marcel Holtmann
> <marcel@...tmann.org>; Johan Hedberg <johan.hedberg@...il.com>; Luiz
> Augusto von Dentz <luiz.dentz@...il.com>; linux-
> bluetooth@...r.kernel.org; linux-kernel@...r.kernel.org; Praveen Kumar
> <kumarpraveen@...ux.microsoft.com>; Naman Jain
> <namjain@...ux.microsoft.com>
> Cc: eahariha@...ux.microsoft.com; Michael Kelley <mhklinux@...look.com>;
> Von Dentz, Luiz <luiz.von.dentz@...el.com>
> Subject: Re: [PATCH v3 1/2] jiffies: Define secs_to_jiffies()
>
> On 10/31/2024 8:54 AM, Haiyang Zhang wrote:
> >
> >
> >> -----Original Message-----
> >> From: Easwar Hariharan <eahariha@...ux.microsoft.com>
> >> Sent: Wednesday, October 30, 2024 1:48 PM
> >> To: KY Srinivasan <kys@...rosoft.com>; Haiyang Zhang
> >> <haiyangz@...rosoft.com>; Wei Liu <wei.liu@...nel.org>; Dexuan Cui
> >> <decui@...rosoft.com>; linux-hyperv@...r.kernel.org; Anna-Maria
> Behnsen
> >> <anna-maria@...utronix.de>; Thomas Gleixner <tglx@...utronix.de>;
> Geert
> >> Uytterhoeven <geert@...ux-m68k.org>; Marcel Holtmann
> >> <marcel@...tmann.org>; Johan Hedberg <johan.hedberg@...il.com>; Luiz
> >> Augusto von Dentz <luiz.dentz@...il.com>; linux-
> >> bluetooth@...r.kernel.org; linux-kernel@...r.kernel.org; Praveen Kumar
> >> <kumarpraveen@...ux.microsoft.com>; Naman Jain
> >> <namjain@...ux.microsoft.com>
> >> Cc: Michael Kelley <mhklinux@...look.com>; Easwar Hariharan
> >> <eahariha@...ux.microsoft.com>; Von Dentz, Luiz
> >> <luiz.von.dentz@...el.com>
> >> Subject: [PATCH v3 1/2] jiffies: Define secs_to_jiffies()
> >>
> >> secs_to_jiffies() is defined in hci_event.c and cannot be reused by
> >> other call sites. Hoist it into the core code to allow conversion of
> the
> >> ~1150 usages of msecs_to_jiffies() that either:
> >> - use a multiplier value of 1000 or equivalently MSEC_PER_SEC, or
> >> - have timeouts that are denominated in seconds (i.e. end in 000)
> >>
> >> This will also allow conversion of yet more sites that use (sec * HZ)
> >> directly, and improve their readability.
> >>
> >> TO: K. Y. Srinivasan <kys@...rosoft.com>
> >> TO: Haiyang Zhang <haiyangz@...rosoft.com>
> >> TO: Wei Liu <wei.liu@...nel.org>
> >> TO: Dexuan Cui <decui@...rosoft.com>
> >> TO: linux-hyperv@...r.kernel.org
> >> TO: Anna-Maria Behnsen <anna-maria@...utronix.de>
> >> TO: Thomas Gleixner <tglx@...utronix.de>
> >> TO: Geert Uytterhoeven <geert@...ux-m68k.org>
> >> TO: Marcel Holtmann <marcel@...tmann.org>
> >> TO: Johan Hedberg <johan.hedberg@...il.com>
> >> TO: Luiz Augusto von Dentz <luiz.dentz@...il.com>
> >> TO: linux-bluetooth@...r.kernel.org
> >> TO: linux-kernel@...r.kernel.org
> >> Suggested-by: Michael Kelley <mhklinux@...look.com>
> >> Reviewed-by: Luiz Augusto von Dentz <luiz.von.dentz@...el.com>
> >> Signed-off-by: Easwar Hariharan <eahariha@...ux.microsoft.com>
> >> ---
> >>  include/linux/jiffies.h   | 12 ++++++++++++
> >>  net/bluetooth/hci_event.c |  2 --
> >>  2 files changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
> >> index
> >>
> 1220f0fbe5bf9fb6c559b4efd603db3e97db9b65..e17c220ed56e587fd55fb9cf4a133a5
> >> 3588af940 100644
> >> --- a/include/linux/jiffies.h
> >> +++ b/include/linux/jiffies.h
> >> @@ -526,6 +526,18 @@ static __always_inline unsigned long
> >> msecs_to_jiffies(const unsigned int m)
> >>    }
> >>  }
> >>
> >> +/**
> >> + * secs_to_jiffies: - convert seconds to jiffies
> >> + * @_secs: time in seconds
> >> + *
> >> + * Conversion is done by simple multiplication with HZ
> >> + * secs_to_jiffies() is defined as a macro rather than a static
> inline
> >> + * function due to its potential application in struct initializers.
> >> + *
> >> + * Return: jiffies value
> >> + */
> >> +#define secs_to_jiffies(_secs) ((_secs) * HZ)
> >> +
> >>  extern unsigned long __usecs_to_jiffies(const unsigned int u);
> >>  #if !(USEC_PER_SEC % HZ)
> >>  static inline unsigned long _usecs_to_jiffies(const unsigned int u)
> >> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> >> index
> >>
> 0bbad90ddd6f87e87c03859bae48a7901d39b634..7b35c58bbbeb79f2b50a02212771fb2
> >> 83ba5643d 100644
> >> --- a/net/bluetooth/hci_event.c
> >> +++ b/net/bluetooth/hci_event.c
> >> @@ -42,8 +42,6 @@
> >>  #define ZERO_KEY "\x00\x00\x00\x00\x00\x00\x00\x00" \
> >>             "\x00\x00\x00\x00\x00\x00\x00\x00"
> >>
> >> -#define secs_to_jiffies(_secs) msecs_to_jiffies((_secs) * 1000)
> >> -
> >>  /* Handle HCI Event packets */
> >>
> >>  static void *hci_ev_skb_pull(struct hci_dev *hdev, struct sk_buff
> *skb,
> >>
> >> --
> >> 2.34.1
> >
> > All looks good.
> > But can you consider naming the macro as s2jiffy()? Just
> > to be shorter, so after adopting this macro we don't have
> > to split some lines for over 80 characters:)
> >
> > Thanks,
> > - Haiyang
> >
>
> Thanks for the review! The patch introducing the macro has already been
> accepted into timers/core in tip[1], so unfortunately I can't make that
> change anymore. For readability considerations, I also find it better to
> match the remaining APIs in the jiffies family, i.e. msecs_to_jiffies(),
> nsecs_to_jiffies(), usecs_to_jiffies().
>
> [1]
> https://git.ker/
> nel.org%2Ftip%2Fb35108a51cf7bab58d7eace1267d7965978bcdb8&data=05%7C02%7Ch
> aiyangz%40microsoft.com%7C7d5db079ed124f62ac2a08dcf9ce4b20%7C72f988bf86f1
> 41af91ab2d7cd011db47%7C1%7C0%7C638659911651280804%7CUnknown%7CTWFpbGZsb3d
> 8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFp
> bCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=hz5UtwU9CLw068z4tpr9kPMANntwX58De
> A5dXi9pqSg%3D&reserved=0
>

Then, that's fine.

Thanks
- Haiyang