> -----Original Message----- > From: Easwar Hariharan <eahariha@xxxxxxxxxxxxxxxxxxx> > Sent: Thursday, October 31, 2024 1:06 PM > To: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>; KY Srinivasan > <kys@xxxxxxxxxxxxx>; Wei Liu <wei.liu@xxxxxxxxxx>; Dexuan Cui > <decui@xxxxxxxxxxxxx>; linux-hyperv@xxxxxxxxxxxxxxx; Anna-Maria Behnsen > <anna-maria@xxxxxxxxxxxxx>; Thomas Gleixner <tglx@xxxxxxxxxxxxx>; Geert > Uytterhoeven <geert@xxxxxxxxxxxxxx>; Marcel Holtmann > <marcel@xxxxxxxxxxxx>; Johan Hedberg <johan.hedberg@xxxxxxxxx>; Luiz > Augusto von Dentz <luiz.dentz@xxxxxxxxx>; linux- > bluetooth@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Praveen Kumar > <kumarpraveen@xxxxxxxxxxxxxxxxxxx>; Naman Jain > <namjain@xxxxxxxxxxxxxxxxxxx> > Cc: eahariha@xxxxxxxxxxxxxxxxxxx; Michael Kelley <mhklinux@xxxxxxxxxxx>; > Von Dentz, Luiz <luiz.von.dentz@xxxxxxxxx> > 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@xxxxxxxxxxxxxxxxxxx> > >> Sent: Wednesday, October 30, 2024 1:48 PM > >> To: KY Srinivasan <kys@xxxxxxxxxxxxx>; Haiyang Zhang > >> <haiyangz@xxxxxxxxxxxxx>; Wei Liu <wei.liu@xxxxxxxxxx>; Dexuan Cui > >> <decui@xxxxxxxxxxxxx>; linux-hyperv@xxxxxxxxxxxxxxx; Anna-Maria > Behnsen > >> <anna-maria@xxxxxxxxxxxxx>; Thomas Gleixner <tglx@xxxxxxxxxxxxx>; > Geert > >> Uytterhoeven <geert@xxxxxxxxxxxxxx>; Marcel Holtmann > >> <marcel@xxxxxxxxxxxx>; Johan Hedberg <johan.hedberg@xxxxxxxxx>; Luiz > >> Augusto von Dentz <luiz.dentz@xxxxxxxxx>; linux- > >> bluetooth@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Praveen Kumar > >> <kumarpraveen@xxxxxxxxxxxxxxxxxxx>; Naman Jain > >> <namjain@xxxxxxxxxxxxxxxxxxx> > >> Cc: Michael Kelley <mhklinux@xxxxxxxxxxx>; Easwar Hariharan > >> <eahariha@xxxxxxxxxxxxxxxxxxx>; Von Dentz, Luiz > >> <luiz.von.dentz@xxxxxxxxx> > >> 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@xxxxxxxxxxxxx> > >> TO: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx> > >> TO: Wei Liu <wei.liu@xxxxxxxxxx> > >> TO: Dexuan Cui <decui@xxxxxxxxxxxxx> > >> TO: linux-hyperv@xxxxxxxxxxxxxxx > >> TO: Anna-Maria Behnsen <anna-maria@xxxxxxxxxxxxx> > >> TO: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > >> TO: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> > >> TO: Marcel Holtmann <marcel@xxxxxxxxxxxx> > >> TO: Johan Hedberg <johan.hedberg@xxxxxxxxx> > >> TO: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> > >> TO: linux-bluetooth@xxxxxxxxxxxxxxx > >> TO: linux-kernel@xxxxxxxxxxxxxxx > >> Suggested-by: Michael Kelley <mhklinux@xxxxxxxxxxx> > >> Reviewed-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> > >> Signed-off-by: Easwar Hariharan <eahariha@xxxxxxxxxxxxxxxxxxx> > >> --- > >> 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
