| Message ID | 20241115-converge-secs-to-jiffies-v2-0-911fb7595e79@linux.microsoft.com |
|---|---|
| Headers | show |
| Series | Converge on using secs_to_jiffies() | expand |
On 11/15/2024 1:29 PM, Easwar Hariharan wrote: > On 11/15/2024 1:26 PM, Easwar Hariharan wrote: >> This is a series that follows up on my previous series to introduce >> secs_to_jiffies() and convert a few initial users.[1] In the review for >> that series, Anna-Maria requested converting other users with >> Coccinelle. This is part 1 that converts users of msecs_to_jiffies() >> that use the multiply pattern of either of: >> - msecs_to_jiffies(N*1000), or >> - msecs_to_jiffies(N*MSEC_PER_SEC) >> >> The entire conversion is made with Coccinelle in the script added in >> patch 2. Some changes suggested by Coccinelle have been deferred to >> later parts that will address other possible variant patterns. >> >> CC: Anna-Maria Behnsen <anna-maria@linutronix.de> >> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com> >> >> [1] https://lore.kernel.org/all/20241030-open-coded-timeouts-v3-0-9ba123facf88@linux.microsoft.com/ >> [2] https://lore.kernel.org/all/8734kngfni.fsf@somnus/ >> >> --- >> Changes in v2: >> - EDITME: describe what is new in this series revision. >> - EDITME: use bulletpoints and terse descriptions. >> - Link to v1: https://lore.kernel.org/r/20241115-converge-secs-to-jiffies-v1-0-19aadc34941b@linux.microsoft.com >> > > Apologies, I missed out on editing the changelog here. v1 included a > patch that's already been accepted, there are no other changes in v2. > > Thanks, > Easwar How do you expect this series to land since it overlaps a large number of maintainer trees? Do you have a maintainer who has volunteered to take the series and the maintainers should just ack? Or do you want the maintainers to take the individual patches that are applicable to them? /jeff
Hi Easwar, On Fri, 15 Nov 2024 21:26:18 +0000 Easwar Hariharan <eahariha@linux.microsoft.com> wrote: > > static const unsigned int sctp_timeouts[SCTP_CONNTRACK_MAX] = { > - [SCTP_CONNTRACK_CLOSED] = 10 SECS, > - [SCTP_CONNTRACK_COOKIE_WAIT] = 3 SECS, > - [SCTP_CONNTRACK_COOKIE_ECHOED] = 3 SECS, > - [SCTP_CONNTRACK_ESTABLISHED] = 210 SECS, > - [SCTP_CONNTRACK_SHUTDOWN_SENT] = 3 SECS, > - [SCTP_CONNTRACK_SHUTDOWN_RECD] = 3 SECS, > - [SCTP_CONNTRACK_SHUTDOWN_ACK_SENT] = 3 SECS, > - [SCTP_CONNTRACK_HEARTBEAT_SENT] = 30 SECS, > + [SCTP_CONNTRACK_CLOSED] = secs_to_jiffies(10), > + [SCTP_CONNTRACK_COOKIE_WAIT] = secs_to_jiffies(3), > + [SCTP_CONNTRACK_COOKIE_ECHOED] = secs_to_jiffies(3), > + [SCTP_CONNTRACK_ESTABLISHED] = secs_to_jiffies(210), > + [SCTP_CONNTRACK_SHUTDOWN_SENT] = secs_to_jiffies(3), > + [SCTP_CONNTRACK_SHUTDOWN_RECD] = secs_to_jiffies(3), > + [SCTP_CONNTRACK_SHUTDOWN_ACK_SENT] = secs_to_jiffies(3), > + [SCTP_CONNTRACK_HEARTBEAT_SENT] = secs_to_jiffies(3), You have changed this last timeout from 30 seconds to 3 (presumably just a copy and paste error).
Le 15/11/2024 à 22:26, Easwar Hariharan a écrit : > [Vous ne recevez pas souvent de courriers de eahariha@linux.microsoft.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > Changes made with the following Coccinelle rules: > > @@ constant C; @@ > > - msecs_to_jiffies(C * 1000) > + secs_to_jiffies(C) > > @@ constant C; @@ > > - msecs_to_jiffies(C * MSEC_PER_SEC) > + secs_to_jiffies(C) > > Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com> > --- > samples/livepatch/livepatch-callbacks-busymod.c | 2 +- > samples/livepatch/livepatch-shadow-fix1.c | 2 +- > samples/livepatch/livepatch-shadow-mod.c | 10 +++++----- > 3 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/samples/livepatch/livepatch-callbacks-busymod.c b/samples/livepatch/livepatch-callbacks-busymod.c > index 378e2d40271a9717d09eff51d3d3612c679736fc..d0fd801a7c21b7d7939c29d83f9d993badcc9aba 100644 > --- a/samples/livepatch/livepatch-callbacks-busymod.c > +++ b/samples/livepatch/livepatch-callbacks-busymod.c > @@ -45,7 +45,7 @@ static int livepatch_callbacks_mod_init(void) > { > pr_info("%s\n", __func__); > schedule_delayed_work(&work, > - msecs_to_jiffies(1000 * 0)); > + secs_to_jiffies(0)); Using secs_to_jiffies() is pointless, 0 is universal, should become schedule_delayed_work(&work, 0); > return 0; > } > > diff --git a/samples/livepatch/livepatch-shadow-fix1.c b/samples/livepatch/livepatch-shadow-fix1.c > index 6701641bf12d454a770e49abeeb0dea92560e55e..948ea1f5760fed2fa27baf478c97cf98ad5c99a8 100644 > --- a/samples/livepatch/livepatch-shadow-fix1.c > +++ b/samples/livepatch/livepatch-shadow-fix1.c > @@ -73,7 +73,7 @@ static struct dummy *livepatch_fix1_dummy_alloc(void) > return NULL; > > d->jiffies_expire = jiffies + > - msecs_to_jiffies(1000 * EXPIRE_PERIOD); > + secs_to_jiffies(EXPIRE_PERIOD); > > /* > * Patch: save the extra memory location into a SV_LEAK shadow > diff --git a/samples/livepatch/livepatch-shadow-mod.c b/samples/livepatch/livepatch-shadow-mod.c > index 7e753b0d2fa611524c9e2adbe02c8fa3e9b6015e..79296e6ccb119f521e86a121623855d841c9fc5e 100644 > --- a/samples/livepatch/livepatch-shadow-mod.c > +++ b/samples/livepatch/livepatch-shadow-mod.c > @@ -102,7 +102,7 @@ static __used noinline struct dummy *dummy_alloc(void) > return NULL; > > d->jiffies_expire = jiffies + > - msecs_to_jiffies(1000 * EXPIRE_PERIOD); > + secs_to_jiffies(EXPIRE_PERIOD); Should fit on a single line, avoid multiple lines when possible. > > /* Oops, forgot to save leak! */ > leak = kzalloc(sizeof(*leak), GFP_KERNEL); > @@ -153,7 +153,7 @@ static void alloc_work_func(struct work_struct *work) > mutex_unlock(&dummy_list_mutex); > > schedule_delayed_work(&alloc_dwork, > - msecs_to_jiffies(1000 * ALLOC_PERIOD)); > + secs_to_jiffies(ALLOC_PERIOD)); Should fit on a single line, avoid multiple lines when possible. > } > > /* > @@ -185,15 +185,15 @@ static void cleanup_work_func(struct work_struct *work) > mutex_unlock(&dummy_list_mutex); > > schedule_delayed_work(&cleanup_dwork, > - msecs_to_jiffies(1000 * CLEANUP_PERIOD)); > + secs_to_jiffies(CLEANUP_PERIOD)); Should fit on a single line, avoid multiple lines when possible. > } > > static int livepatch_shadow_mod_init(void) > { > schedule_delayed_work(&alloc_dwork, > - msecs_to_jiffies(1000 * ALLOC_PERIOD)); > + secs_to_jiffies(ALLOC_PERIOD)); Should fit on a single line, avoid multiple lines when possible. > schedule_delayed_work(&cleanup_dwork, > - msecs_to_jiffies(1000 * CLEANUP_PERIOD)); > + secs_to_jiffies(CLEANUP_PERIOD)); Should fit on a single line, avoid multiple lines when possible. > > return 0; > } > > -- > 2.34.1 >
Le 15/11/2024 à 22:26, Easwar Hariharan a écrit : > [Vous ne recevez pas souvent de courriers de eahariha@linux.microsoft.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > This is a series that follows up on my previous series to introduce > secs_to_jiffies() and convert a few initial users.[1] In the review for > that series, Anna-Maria requested converting other users with > Coccinelle. This is part 1 that converts users of msecs_to_jiffies() > that use the multiply pattern of either of: > - msecs_to_jiffies(N*1000), or > - msecs_to_jiffies(N*MSEC_PER_SEC) You should provide a reference to the accepted commit that adds secs_to_jiffies: Commit b35108a51cf7 ("jiffies: Define secs_to_jiffies()") > > The entire conversion is made with Coccinelle in the script added in > patch 2. Some changes suggested by Coccinelle have been deferred to > later parts that will address other possible variant patterns. > > CC: Anna-Maria Behnsen <anna-maria@linutronix.de> > Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com> > > [1] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20241030-open-coded-timeouts-v3-0-9ba123facf88%40linux.microsoft.com%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C121622b159564a010cac08dd05bc32da%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638673028056187739%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=oW04hGIpfjRo8qcX0GaGdHE1xiApgoOtgAuWQXFgWR4%3D&reserved=0 > [2] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F8734kngfni.fsf%40somnus%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C121622b159564a010cac08dd05bc32da%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638673028056211741%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=UDn89U6oUNFiRj3K5fvNEIuiwmwEGfJ2XhPn43z8%2BhA%3D&reserved=0 > > --- > Changes in v2: > - EDITME: describe what is new in this series revision. > - EDITME: use bulletpoints and terse descriptions. > - Link to v1: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fr%2F20241115-converge-secs-to-jiffies-v1-0-19aadc34941b%40linux.microsoft.com&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C121622b159564a010cac08dd05bc32da%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638673028056225723%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=reWzZOiSyn%2FA5qxcXAoUqNGedJ1K%2FM%2BuCgEKwXusuU8%3D&reserved=0 > > --- > Easwar Hariharan (21): > netfilter: conntrack: Cleanup timeout definitions > coccinelle: misc: Add secs_to_jiffies script > arm: pxa: Convert timeouts to use secs_to_jiffies() > s390: kernel: Convert timeouts to use secs_to_jiffies() > powerpc/papr_scm: Convert timeouts to secs_to_jiffies() > mm: kmemleak: Convert timeouts to secs_to_jiffies() > accel/habanalabs: Convert timeouts to secs_to_jiffies() > drm/xe: Convert timeout to secs_to_jiffies() > drm/etnaviv: Convert timeouts to secs_to_jiffies() > scsi: lpfc: Convert timeouts to secs_to_jiffies() > scsi: arcmsr: Convert timeouts to secs_to_jiffies() > scsi: pm8001: Convert timeouts to secs_to_jiffies() > xen/blkback: Convert timeouts to secs_to_jiffies() > gve: Convert timeouts to secs_to_jiffies() > wifi: ath11k: Convert timeouts to secs_to_jiffies() > Bluetooth: MGMT: Convert timeouts to secs_to_jiffies() > staging: vc04_services: Convert timeouts to secs_to_jiffies() > ceph: Convert timeouts to secs_to_jiffies() > livepatch: Convert timeouts to secs_to_jiffies() > ALSA: line6: Convert timeouts to secs_to_jiffies() > nfp: Convert timeouts to secs_to_jiffies() > > arch/arm/mach-pxa/sharpsl_pm.c | 6 +++--- > arch/powerpc/platforms/pseries/papr_scm.c | 2 +- > arch/s390/kernel/lgr.c | 3 ++- > arch/s390/kernel/time.c | 4 ++-- > arch/s390/kernel/topology.c | 2 +- > drivers/accel/habanalabs/common/device.c | 2 +- > drivers/accel/habanalabs/common/habanalabs_drv.c | 3 +-- > drivers/block/xen-blkback/blkback.c | 2 +- > drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 2 +- > drivers/gpu/drm/xe/xe_device.c | 2 +- > drivers/net/ethernet/google/gve/gve_tx_dqo.c | 6 ++---- > drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 +- > drivers/net/wireless/ath/ath11k/debugfs.c | 2 +- > drivers/scsi/arcmsr/arcmsr_hba.c | 2 +- > drivers/scsi/lpfc/lpfc_init.c | 18 +++++++++--------- > drivers/scsi/lpfc/lpfc_nportdisc.c | 8 ++++---- > drivers/scsi/lpfc/lpfc_nvme.c | 2 +- > drivers/scsi/lpfc/lpfc_sli.c | 4 ++-- > drivers/scsi/lpfc/lpfc_vmid.c | 2 +- > drivers/scsi/pm8001/pm8001_init.c | 2 +- > .../vc04_services/bcm2835-audio/bcm2835-vchiq.c | 2 +- > fs/ceph/quota.c | 2 +- > mm/kmemleak.c | 4 ++-- > net/bluetooth/mgmt.c | 2 +- > net/netfilter/nf_conntrack_proto_sctp.c | 21 ++++++++------------- > samples/livepatch/livepatch-callbacks-busymod.c | 2 +- > samples/livepatch/livepatch-shadow-fix1.c | 2 +- > samples/livepatch/livepatch-shadow-mod.c | 10 +++++----- > scripts/coccinelle/misc/secs_to_jiffies.cocci | 21 +++++++++++++++++++++ > sound/usb/line6/toneport.c | 2 +- > 30 files changed, 79 insertions(+), 65 deletions(-) > --- > base-commit: 2d5404caa8c7bb5c4e0435f94b28834ae5456623 > change-id: 20241112-converge-secs-to-jiffies-d99d1016bd11 > > Best regards, > -- > Easwar Hariharan <eahariha@linux.microsoft.com> >
On Sat, Nov 16, 2024 at 11:06:55AM +0100, Christophe Leroy wrote: > > diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c > > index 9e297f88adc5d97d4dc7b267b0bfebd58e5cf193..9e8086ec66e0f0e555ac27933854c06cfcf91a04 100644 > > --- a/arch/powerpc/platforms/pseries/papr_scm.c > > +++ b/arch/powerpc/platforms/pseries/papr_scm.c > > @@ -543,7 +543,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p) > > > > /* Jiffies offset for which the health data is assumed to be same */ > > cache_timeout = p->lasthealth_jiffies + > > - msecs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL * 1000); > > + secs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL); > > Wouldn't it now fit on a single line ? > Some maintainers still prefer to put a line break at 80 characters. It's kind of a nightmare for an automated script like this to figure out everyone's preferences. In this particular file, there are some lines which go over 80 characters so sure. Earlier in the patchset one of these introduced a line break that wasn't there before so I think maybe Coccinelle is applying the 80 character line break rule? There are sometimes where the 80 character rule really hurts readability, but here it doesn't make any difference. regards, dan carpenter
[removed most non-list recipients, it's just too much] On 11/15/24 10:26 PM, Easwar Hariharan wrote: > This is a series that follows up on my previous series to introduce > secs_to_jiffies() and convert a few initial users.[1] In the review for > that series, Anna-Maria requested converting other users with > Coccinelle. This is part 1 that converts users of msecs_to_jiffies() > that use the multiply pattern of either of: > - msecs_to_jiffies(N*1000), or > - msecs_to_jiffies(N*MSEC_PER_SEC) > > The entire conversion is made with Coccinelle in the script added in > patch 2. Some changes suggested by Coccinelle have been deferred to > later parts that will address other possible variant patterns. > > CC: Anna-Maria Behnsen <anna-maria@linutronix.de> > Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com> > > [1] https://lore.kernel.org/all/20241030-open-coded-timeouts-v3-0-9ba123facf88@linux.microsoft.com/ > [2] https://lore.kernel.org/all/8734kngfni.fsf@somnus/ > > --- > Changes in v2: > - EDITME: describe what is new in this series revision. > - EDITME: use bulletpoints and terse descriptions. > - Link to v1: https://lore.kernel.org/r/20241115-converge-secs-to-jiffies-v1-0-19aadc34941b@linux.microsoft.com that is not a proper changelog, you were supposed to edit those placeholder entries; please look around for examples There is also just too much recipients. Please split up your patches into smaller pieces. You will also learn the process on a smaller sample. And definitively please wait for 48h before reposting such big series. Regarding code - you could also convert msecs_to_jiffies(const * HZ), there are 10 that are greppable. > > --- > Easwar Hariharan (21): > netfilter: conntrack: Cleanup timeout definitions > coccinelle: misc: Add secs_to_jiffies script > arm: pxa: Convert timeouts to use secs_to_jiffies() > s390: kernel: Convert timeouts to use secs_to_jiffies() > powerpc/papr_scm: Convert timeouts to secs_to_jiffies() > mm: kmemleak: Convert timeouts to secs_to_jiffies() > accel/habanalabs: Convert timeouts to secs_to_jiffies() > drm/xe: Convert timeout to secs_to_jiffies() > drm/etnaviv: Convert timeouts to secs_to_jiffies() > scsi: lpfc: Convert timeouts to secs_to_jiffies() > scsi: arcmsr: Convert timeouts to secs_to_jiffies() > scsi: pm8001: Convert timeouts to secs_to_jiffies() > xen/blkback: Convert timeouts to secs_to_jiffies() > gve: Convert timeouts to secs_to_jiffies() > wifi: ath11k: Convert timeouts to secs_to_jiffies() > Bluetooth: MGMT: Convert timeouts to secs_to_jiffies() > staging: vc04_services: Convert timeouts to secs_to_jiffies() > ceph: Convert timeouts to secs_to_jiffies() > livepatch: Convert timeouts to secs_to_jiffies() > ALSA: line6: Convert timeouts to secs_to_jiffies() > nfp: Convert timeouts to secs_to_jiffies() > > arch/arm/mach-pxa/sharpsl_pm.c | 6 +++--- > arch/powerpc/platforms/pseries/papr_scm.c | 2 +- > arch/s390/kernel/lgr.c | 3 ++- > arch/s390/kernel/time.c | 4 ++-- > arch/s390/kernel/topology.c | 2 +- > drivers/accel/habanalabs/common/device.c | 2 +- > drivers/accel/habanalabs/common/habanalabs_drv.c | 3 +-- > drivers/block/xen-blkback/blkback.c | 2 +- > drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 2 +- > drivers/gpu/drm/xe/xe_device.c | 2 +- > drivers/net/ethernet/google/gve/gve_tx_dqo.c | 6 ++---- > drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 +- > drivers/net/wireless/ath/ath11k/debugfs.c | 2 +- > drivers/scsi/arcmsr/arcmsr_hba.c | 2 +- > drivers/scsi/lpfc/lpfc_init.c | 18 +++++++++--------- > drivers/scsi/lpfc/lpfc_nportdisc.c | 8 ++++---- > drivers/scsi/lpfc/lpfc_nvme.c | 2 +- > drivers/scsi/lpfc/lpfc_sli.c | 4 ++-- > drivers/scsi/lpfc/lpfc_vmid.c | 2 +- > drivers/scsi/pm8001/pm8001_init.c | 2 +- > .../vc04_services/bcm2835-audio/bcm2835-vchiq.c | 2 +- > fs/ceph/quota.c | 2 +- > mm/kmemleak.c | 4 ++-- > net/bluetooth/mgmt.c | 2 +- > net/netfilter/nf_conntrack_proto_sctp.c | 21 ++++++++------------- > samples/livepatch/livepatch-callbacks-busymod.c | 2 +- > samples/livepatch/livepatch-shadow-fix1.c | 2 +- > samples/livepatch/livepatch-shadow-mod.c | 10 +++++----- > scripts/coccinelle/misc/secs_to_jiffies.cocci | 21 +++++++++++++++++++++ > sound/usb/line6/toneport.c | 2 +- > 30 files changed, 79 insertions(+), 65 deletions(-) > --- > base-commit: 2d5404caa8c7bb5c4e0435f94b28834ae5456623 > change-id: 20241112-converge-secs-to-jiffies-d99d1016bd11 > > Best regards,
On 11/29/2024 4:57 AM, Przemek Kitszel wrote: > > [removed most non-list recipients, it's just too much] > > On 11/15/24 10:26 PM, Easwar Hariharan wrote: <snip> >> >> --- >> Changes in v2: >> - EDITME: describe what is new in this series revision. >> - EDITME: use bulletpoints and terse descriptions. >> - Link to v1: https://lore.kernel.org/r/20241115-converge-secs-to- >> jiffies-v1-0-19aadc34941b@linux.microsoft.com > > that is not a proper changelog, you were supposed to edit those > placeholder entries; please look around for examples > > There is also just too much recipients. Please split up your patches > into smaller pieces. You will also learn the process on a smaller > sample. > > And definitively please wait for 48h before reposting such big series. Yes, sorry, I sent out a v2 in a moment of panic on including the already accepted patch in v1. I failed to edit the changelog in that same panic. I'll try to not panic and do better in the future. > > Regarding code - you could also convert msecs_to_jiffies(const * HZ), > there are 10 that are greppable. > Those seem to be mistakes. const*HZ is a seconds-denominated timeout, being passed to msecs_to_jiffies() which will treat it as a millisecond-denominated timeout resulting in an excessively long timeout. I suppose that's better than a too-short timeout, and apparently it's been working fine all along since hardware responds before the too-long timeout expires. Half of them are in drivers/scsi/arcmsr/arcmsr_hba.c and the pattern has apparently been there since 2010. Thanks, Easwar
Le 09/12/2024 à 13:01, Przemek Kitszel a écrit : > On 12/6/24 9:58 PM, Easwar Hariharan wrote: >> On 11/29/2024 4:57 AM, Przemek Kitszel wrote: >>> >>> [removed most non-list recipients, it's just too much] >>> >>> On 11/15/24 10:26 PM, Easwar Hariharan wrote: >> <snip> > >>> >>> Regarding code - you could also convert msecs_to_jiffies(const * HZ), >>> there are 10 that are greppable. >>> >> >> Those seem to be mistakes. const*HZ is a seconds-denominated timeout, >> being passed to msecs_to_jiffies() which will treat it as a >> millisecond-denominated timeout resulting in an excessively long >> timeout. I suppose that's better than a too-short timeout, and >> apparently it's been working fine all along since hardware responds >> before the too-long timeout expires. Half of them are in >> drivers/scsi/arcmsr/arcmsr_hba.c and the pattern has apparently been >> there since 2010. > > my point was that, the default value of HZ is 1000, and most of the code > that is just `$value*HZ` was meant as "$value seconds, in ms unit". I can't follow you here. The default value of HZ is 250 as far as I can see. Regardless, HZ is the number of jiffies per second, nothing else. > > Same for HZ/const, HZ/2 being 500ms. > > HZ is awful in that it is not 1s but 1/s, but it was easy to abuse the > value in simple context. Why is that awful ? HZ is a nice macro that gives you the number of ticks per second, so that you are able to easily calculate the number of ticks for a given duration, regardless of the configured number of ticks per second. Christophe
This is a series that follows up on my previous series to introduce secs_to_jiffies() and convert a few initial users.[1] In the review for that series, Anna-Maria requested converting other users with Coccinelle. This is part 1 that converts users of msecs_to_jiffies() that use the multiply pattern of either of: - msecs_to_jiffies(N*1000), or - msecs_to_jiffies(N*MSEC_PER_SEC) The entire conversion is made with Coccinelle in the script added in patch 2. Some changes suggested by Coccinelle have been deferred to later parts that will address other possible variant patterns. CC: Anna-Maria Behnsen <anna-maria@linutronix.de> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com> [1] https://lore.kernel.org/all/20241030-open-coded-timeouts-v3-0-9ba123facf88@linux.microsoft.com/ [2] https://lore.kernel.org/all/8734kngfni.fsf@somnus/ --- Changes in v2: - EDITME: describe what is new in this series revision. - EDITME: use bulletpoints and terse descriptions. - Link to v1: https://lore.kernel.org/r/20241115-converge-secs-to-jiffies-v1-0-19aadc34941b@linux.microsoft.com --- Easwar Hariharan (21): netfilter: conntrack: Cleanup timeout definitions coccinelle: misc: Add secs_to_jiffies script arm: pxa: Convert timeouts to use secs_to_jiffies() s390: kernel: Convert timeouts to use secs_to_jiffies() powerpc/papr_scm: Convert timeouts to secs_to_jiffies() mm: kmemleak: Convert timeouts to secs_to_jiffies() accel/habanalabs: Convert timeouts to secs_to_jiffies() drm/xe: Convert timeout to secs_to_jiffies() drm/etnaviv: Convert timeouts to secs_to_jiffies() scsi: lpfc: Convert timeouts to secs_to_jiffies() scsi: arcmsr: Convert timeouts to secs_to_jiffies() scsi: pm8001: Convert timeouts to secs_to_jiffies() xen/blkback: Convert timeouts to secs_to_jiffies() gve: Convert timeouts to secs_to_jiffies() wifi: ath11k: Convert timeouts to secs_to_jiffies() Bluetooth: MGMT: Convert timeouts to secs_to_jiffies() staging: vc04_services: Convert timeouts to secs_to_jiffies() ceph: Convert timeouts to secs_to_jiffies() livepatch: Convert timeouts to secs_to_jiffies() ALSA: line6: Convert timeouts to secs_to_jiffies() nfp: Convert timeouts to secs_to_jiffies() arch/arm/mach-pxa/sharpsl_pm.c | 6 +++--- arch/powerpc/platforms/pseries/papr_scm.c | 2 +- arch/s390/kernel/lgr.c | 3 ++- arch/s390/kernel/time.c | 4 ++-- arch/s390/kernel/topology.c | 2 +- drivers/accel/habanalabs/common/device.c | 2 +- drivers/accel/habanalabs/common/habanalabs_drv.c | 3 +-- drivers/block/xen-blkback/blkback.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 2 +- drivers/gpu/drm/xe/xe_device.c | 2 +- drivers/net/ethernet/google/gve/gve_tx_dqo.c | 6 ++---- drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 +- drivers/net/wireless/ath/ath11k/debugfs.c | 2 +- drivers/scsi/arcmsr/arcmsr_hba.c | 2 +- drivers/scsi/lpfc/lpfc_init.c | 18 +++++++++--------- drivers/scsi/lpfc/lpfc_nportdisc.c | 8 ++++---- drivers/scsi/lpfc/lpfc_nvme.c | 2 +- drivers/scsi/lpfc/lpfc_sli.c | 4 ++-- drivers/scsi/lpfc/lpfc_vmid.c | 2 +- drivers/scsi/pm8001/pm8001_init.c | 2 +- .../vc04_services/bcm2835-audio/bcm2835-vchiq.c | 2 +- fs/ceph/quota.c | 2 +- mm/kmemleak.c | 4 ++-- net/bluetooth/mgmt.c | 2 +- net/netfilter/nf_conntrack_proto_sctp.c | 21 ++++++++------------- samples/livepatch/livepatch-callbacks-busymod.c | 2 +- samples/livepatch/livepatch-shadow-fix1.c | 2 +- samples/livepatch/livepatch-shadow-mod.c | 10 +++++----- scripts/coccinelle/misc/secs_to_jiffies.cocci | 21 +++++++++++++++++++++ sound/usb/line6/toneport.c | 2 +- 30 files changed, 79 insertions(+), 65 deletions(-) --- base-commit: 2d5404caa8c7bb5c4e0435f94b28834ae5456623 change-id: 20241112-converge-secs-to-jiffies-d99d1016bd11 Best regards,