> -----Original Message-----
> From: Easwar Hariharan <eahar...@linux.microsoft.com>
> Sent: Thursday, October 31, 2024 1:06 PM
> To: Haiyang Zhang <haiya...@microsoft.com>; KY Srinivasan
> <k...@microsoft.com>; Wei Liu <wei....@kernel.org>; Dexuan Cui
> <de...@microsoft.com>; linux-hyperv@vger.kernel.org; Anna-Maria Behnsen
> <anna-ma...@linutronix.de>; Thomas Gleixner <t...@linutronix.de>; Geert
> Uytterhoeven <ge...@linux-m68k.org>; Marcel Holtmann
> <mar...@holtmann.org>; Johan Hedberg <johan.hedb...@gmail.com>; Luiz
> Augusto von Dentz <luiz.de...@gmail.com>; linux-
> blueto...@vger.kernel.org; linux-ker...@vger.kernel.org; Praveen Kumar
> <kumarprav...@linux.microsoft.com>; Naman Jain
> <namj...@linux.microsoft.com>
> Cc: eahar...@linux.microsoft.com; Michael Kelley <mhkli...@outlook.com>;
> Von Dentz, Luiz <luiz.von.de...@intel.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 <eahar...@linux.microsoft.com>
> >> Sent: Wednesday, October 30, 2024 1:48 PM
> >> To: KY Srinivasan <k...@microsoft.com>; Haiyang Zhang
> >> <haiya...@microsoft.com>; Wei Liu <wei....@kernel.org>; Dexuan Cui
> >> <de...@microsoft.com>; linux-hyperv@vger.kernel.org; Anna-Maria
> Behnsen
> >> <anna-ma...@linutronix.de>; Thomas Gleixner <t...@linutronix.de>;
> Geert
> >> Uytterhoeven <ge...@linux-m68k.org>; Marcel Holtmann
> >> <mar...@holtmann.org>; Johan Hedberg <johan.hedb...@gmail.com>; Luiz
> >> Augusto von Dentz <luiz.de...@gmail.com>; linux-
> >> blueto...@vger.kernel.org; linux-ker...@vger.kernel.org; Praveen Kumar
> >> <kumarprav...@linux.microsoft.com>; Naman Jain
> >> <namj...@linux.microsoft.com>
> >> Cc: Michael Kelley <mhkli...@outlook.com>; Easwar Hariharan
> >> <eahar...@linux.microsoft.com>; Von Dentz, Luiz
> >> <luiz.von.de...@intel.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 <k...@microsoft.com>
> >> TO: Haiyang Zhang <haiya...@microsoft.com>
> >> TO: Wei Liu <wei....@kernel.org>
> >> TO: Dexuan Cui <de...@microsoft.com>
> >> TO: linux-hyperv@vger.kernel.org
> >> TO: Anna-Maria Behnsen <anna-ma...@linutronix.de>
> >> TO: Thomas Gleixner <t...@linutronix.de>
> >> TO: Geert Uytterhoeven <ge...@linux-m68k.org>
> >> TO: Marcel Holtmann <mar...@holtmann.org>
> >> TO: Johan Hedberg <johan.hedb...@gmail.com>
> >> TO: Luiz Augusto von Dentz <luiz.de...@gmail.com>
> >> TO: linux-blueto...@vger.kernel.org
> >> TO: linux-ker...@vger.kernel.org
> >> Suggested-by: Michael Kelley <mhkli...@outlook.com>
> >> Reviewed-by: Luiz Augusto von Dentz <luiz.von.de...@intel.com>
> >> Signed-off-by: Easwar Hariharan <eahar...@linux.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
