* [PATCH v4 0/2] Converge on using secs_to_jiffies() part two
@ 2025-03-01 4:22 Easwar Hariharan
2025-03-01 4:22 ` [PATCH v4 1/2] rbd: convert timeouts to secs_to_jiffies() Easwar Hariharan
2025-03-01 4:22 ` [PATCH v4 2/2] libceph: " Easwar Hariharan
0 siblings, 2 replies; 7+ messages in thread
From: Easwar Hariharan @ 2025-03-01 4:22 UTC (permalink / raw)
To: Andrew Morton, Christophe JAILLET, Daniel Vacek, Ilya Dryomov,
Dongsheng Yang, Jens Axboe, Xiubo Li
Cc: ceph-devel, linux-block, linux-kernel, Easwar Hariharan
This is the second series (part 1*) that converts users of msecs_to_jiffies() that
either use the multiply pattern of either of:
- msecs_to_jiffies(N*1000) or
- msecs_to_jiffies(N*MSEC_PER_SEC)
where N is a constant or an expression, to avoid the multiplication.
The conversion is made with Coccinelle with the secs_to_jiffies() script
in scripts/coccinelle/misc. Attention is paid to what the best change
can be rather than restricting to what the tool provides.
Andrew has kindly agreed to take the series through mm.git modulo the
patches maintainers want to pick through their own trees.
This series is based on next-20250225
Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
* https://lore.kernel.org/all/20241210-converge-secs-to-jiffies-v3-0-ddfefd7e9f2a@linux.microsoft.com/
---
Changes in v4:
- Restore correct range checks for rbd and libceph (Christophe J)
- Link to v3: https://lore.kernel.org/r/20250225-converge-secs-to-jiffies-part-two-v3-0-a43967e36c88@linux.microsoft.com
Changes in v3:
- Change commit message prefix from libata: zpodd to ata: libata-zpodd: in patch 8 (Damien)
- Split up overly long line in patch 9 (Christoph)
- Fixup unnecessary line break in patch 14 (Ilpo)
- Combine v1 and v2
- Fix some additional hunks in patch 2 (scsi: lpfc) which the more concise script missed
- msecs_to_jiffies -> msecs_to_jiffies() in commit messages throughout
- Bug in secs_to_jiffies() uncovered by LKP merged in 6.14-rc2: bb2784d9ab4958 ("jiffies: Cast to unsigned long in secs_to_jiffies() conversion")
- Link to v2: https://lore.kernel.org/r/20250203-converge-secs-to-jiffies-part-two-v2-0-d7058a01fd0e@linux.microsoft.com
Changes in v2:
- Remove unneeded range checks in rbd and libceph. While there, convert some timeouts that should have been fixed in part 1. (Ilya)
- Fixup secs_to_jiffies.cocci to be a bit more verbose
- Link to v1: https://lore.kernel.org/r/20250128-converge-secs-to-jiffies-part-two-v1-0-9a6ecf0b2308@linux.microsoft.com
---
Easwar Hariharan (2):
rbd: convert timeouts to secs_to_jiffies()
libceph: convert timeouts to secs_to_jiffies()
drivers/block/rbd.c | 8 ++++----
include/linux/ceph/libceph.h | 12 ++++++------
net/ceph/ceph_common.c | 18 ++++++++----------
net/ceph/osd_client.c | 3 +--
4 files changed, 19 insertions(+), 22 deletions(-)
---
base-commit: 0226d0ce98a477937ed295fb7df4cc30b46fc304
change-id: 20241217-converge-secs-to-jiffies-part-two-f44017aa6f67
Best regards,
--
Easwar Hariharan <eahariha@linux.microsoft.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v4 1/2] rbd: convert timeouts to secs_to_jiffies() 2025-03-01 4:22 [PATCH v4 0/2] Converge on using secs_to_jiffies() part two Easwar Hariharan @ 2025-03-01 4:22 ` Easwar Hariharan 2025-03-02 17:05 ` Alex Elder 2025-03-01 4:22 ` [PATCH v4 2/2] libceph: " Easwar Hariharan 1 sibling, 1 reply; 7+ messages in thread From: Easwar Hariharan @ 2025-03-01 4:22 UTC (permalink / raw) To: Andrew Morton, Christophe JAILLET, Daniel Vacek, Ilya Dryomov, Dongsheng Yang, Jens Axboe, Xiubo Li Cc: ceph-devel, linux-block, linux-kernel, Easwar Hariharan Commit b35108a51cf7 ("jiffies: Define secs_to_jiffies()") introduced secs_to_jiffies(). As the value here is a multiple of 1000, use secs_to_jiffies() instead of msecs_to_jiffies() to avoid the multiplication This is converted using scripts/coccinelle/misc/secs_to_jiffies.cocci with the following Coccinelle rules: @depends on patch@ expression E; @@ -msecs_to_jiffies(E * 1000) +secs_to_jiffies(E) @depends on patch@ expression E; @@ -msecs_to_jiffies(E * MSEC_PER_SEC) +secs_to_jiffies(E) Change the check for range to check against HZ. Acked-by: Ilya Dryomov <idryomov@gmail.com> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com> --- drivers/block/rbd.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index faafd7ff43d6ef53110ab3663cc7ac322214cc8c..1c406b17f3cee741b7bdd9f742958b3f1d5b1bbe 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -108,7 +108,7 @@ static int atomic_dec_return_safe(atomic_t *v) #define RBD_OBJ_PREFIX_LEN_MAX 64 #define RBD_NOTIFY_TIMEOUT 5 /* seconds */ -#define RBD_RETRY_DELAY msecs_to_jiffies(1000) +#define RBD_RETRY_DELAY secs_to_jiffies(1) /* Feature bits */ @@ -4162,7 +4162,7 @@ static void rbd_acquire_lock(struct work_struct *work) dout("%s rbd_dev %p requeuing lock_dwork\n", __func__, rbd_dev); mod_delayed_work(rbd_dev->task_wq, &rbd_dev->lock_dwork, - msecs_to_jiffies(2 * RBD_NOTIFY_TIMEOUT * MSEC_PER_SEC)); + secs_to_jiffies(2 * RBD_NOTIFY_TIMEOUT)); } } @@ -6283,9 +6283,9 @@ static int rbd_parse_param(struct fs_parameter *param, break; case Opt_lock_timeout: /* 0 is "wait forever" (i.e. infinite timeout) */ - if (result.uint_32 > INT_MAX / 1000) + if (result.uint32 > INT_MAX / HZ) goto out_of_range; - opt->lock_timeout = msecs_to_jiffies(result.uint_32 * 1000); + opt->lock_timeout = secs_to_jiffies(result.uint_32); break; case Opt_pool_ns: kfree(pctx->spec->pool_ns); -- 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] rbd: convert timeouts to secs_to_jiffies() 2025-03-01 4:22 ` [PATCH v4 1/2] rbd: convert timeouts to secs_to_jiffies() Easwar Hariharan @ 2025-03-02 17:05 ` Alex Elder 2025-03-03 18:13 ` Easwar Hariharan 0 siblings, 1 reply; 7+ messages in thread From: Alex Elder @ 2025-03-02 17:05 UTC (permalink / raw) To: Easwar Hariharan, Andrew Morton, Christophe JAILLET, Daniel Vacek, Ilya Dryomov, Dongsheng Yang, Jens Axboe, Xiubo Li Cc: ceph-devel, linux-block, linux-kernel On 2/28/25 10:22 PM, Easwar Hariharan wrote: > Commit b35108a51cf7 ("jiffies: Define secs_to_jiffies()") introduced > secs_to_jiffies(). As the value here is a multiple of 1000, use > secs_to_jiffies() instead of msecs_to_jiffies() to avoid the multiplication > > This is converted using scripts/coccinelle/misc/secs_to_jiffies.cocci with > the following Coccinelle rules: > > @depends on patch@ expression E; @@ > > -msecs_to_jiffies(E * 1000) > +secs_to_jiffies(E) > > @depends on patch@ expression E; @@ > > -msecs_to_jiffies(E * MSEC_PER_SEC) > +secs_to_jiffies(E) > > Change the check for range to check against HZ. > > Acked-by: Ilya Dryomov <idryomov@gmail.com> > Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com> I think what you've done in the last hunk below should not be done that way. I also suggest something to the (related, but not part of this series) secs_to_jiffies() implementation. > --- > drivers/block/rbd.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > index faafd7ff43d6ef53110ab3663cc7ac322214cc8c..1c406b17f3cee741b7bdd9f742958b3f1d5b1bbe 100644 > --- a/drivers/block/rbd.c > +++ b/drivers/block/rbd.c > @@ -108,7 +108,7 @@ static int atomic_dec_return_safe(atomic_t *v) > #define RBD_OBJ_PREFIX_LEN_MAX 64 > > #define RBD_NOTIFY_TIMEOUT 5 /* seconds */ > -#define RBD_RETRY_DELAY msecs_to_jiffies(1000) > +#define RBD_RETRY_DELAY secs_to_jiffies(1) > > /* Feature bits */ > > @@ -4162,7 +4162,7 @@ static void rbd_acquire_lock(struct work_struct *work) > dout("%s rbd_dev %p requeuing lock_dwork\n", __func__, > rbd_dev); > mod_delayed_work(rbd_dev->task_wq, &rbd_dev->lock_dwork, > - msecs_to_jiffies(2 * RBD_NOTIFY_TIMEOUT * MSEC_PER_SEC)); > + secs_to_jiffies(2 * RBD_NOTIFY_TIMEOUT)); > } > } > > @@ -6283,9 +6283,9 @@ static int rbd_parse_param(struct fs_parameter *param, > break; > case Opt_lock_timeout: > /* 0 is "wait forever" (i.e. infinite timeout) */ > - if (result.uint_32 > INT_MAX / 1000) Previously, the above line was verifying that the multiplication done below would not overflow. It was unrelated to whatever msecs_to_jiffies() did. > + if (result.uint32 > INT_MAX / HZ) Here you are assuming something about what secs_to_jiffies() does. It's a very reasonable assumption, but you are encoding this in unrelated code, which you shouldn't do. Just do the direct conversion as you've done above: if (result.uint32 > INT_MAX) > goto out_of_range; > - opt->lock_timeout = msecs_to_jiffies(result.uint_32 * 1000); > + opt->lock_timeout = secs_to_jiffies(result.uint_32); Unfortunately, secs_to_jiffies() does not implement the clamp operation that msecs_to_jiffies() does. If you look at __msecs_to_jiffies() you see that the unsigned value provided is limited to MAX_JIFFY_OFFSET if it's negative when interpreted as a signed int (i.e., if its high bit is set). I think the secs_to_jiffies() implementation could benefit from the use of an overflow check. This might not be exactly right, but it gives the idea: #define secs_to_jiffies(_secs) \ ({ \ unsigned long _result; \ \ if (check_mul_overflow(_secs, HZ, &_result)) \ _result = MAX_JIFFY_OFFSET; \ (_result); \ }) -Alex > break; > case Opt_pool_ns: > kfree(pctx->spec->pool_ns); > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] rbd: convert timeouts to secs_to_jiffies() 2025-03-02 17:05 ` Alex Elder @ 2025-03-03 18:13 ` Easwar Hariharan 2025-03-03 18:18 ` Jens Axboe 0 siblings, 1 reply; 7+ messages in thread From: Easwar Hariharan @ 2025-03-03 18:13 UTC (permalink / raw) To: Alex Elder, Andrew Morton, Jens Axboe Cc: Christophe JAILLET, Daniel Vacek, Ilya Dryomov, Dongsheng Yang, Xiubo Li, eahariha, ceph-devel, linux-block, linux-kernel On 3/2/2025 9:05 AM, Alex Elder wrote: > On 2/28/25 10:22 PM, Easwar Hariharan wrote: >> Commit b35108a51cf7 ("jiffies: Define secs_to_jiffies()") introduced >> secs_to_jiffies(). As the value here is a multiple of 1000, use >> secs_to_jiffies() instead of msecs_to_jiffies() to avoid the multiplication >> >> This is converted using scripts/coccinelle/misc/secs_to_jiffies.cocci with >> the following Coccinelle rules: >> >> @depends on patch@ expression E; @@ >> >> -msecs_to_jiffies(E * 1000) >> +secs_to_jiffies(E) >> >> @depends on patch@ expression E; @@ >> >> -msecs_to_jiffies(E * MSEC_PER_SEC) >> +secs_to_jiffies(E) >> >> Change the check for range to check against HZ. >> >> Acked-by: Ilya Dryomov <idryomov@gmail.com> >> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com> > > I think what you've done in the last hunk below should not be > done that way. I also suggest something to the (related, but > not part of this series) secs_to_jiffies() implementation. > >> --- >> drivers/block/rbd.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c >> index faafd7ff43d6ef53110ab3663cc7ac322214cc8c..1c406b17f3cee741b7bdd9f742958b3f1d5b1bbe 100644 >> --- a/drivers/block/rbd.c >> +++ b/drivers/block/rbd.c >> @@ -108,7 +108,7 @@ static int atomic_dec_return_safe(atomic_t *v) >> #define RBD_OBJ_PREFIX_LEN_MAX 64 >> #define RBD_NOTIFY_TIMEOUT 5 /* seconds */ >> -#define RBD_RETRY_DELAY msecs_to_jiffies(1000) >> +#define RBD_RETRY_DELAY secs_to_jiffies(1) >> /* Feature bits */ >> @@ -4162,7 +4162,7 @@ static void rbd_acquire_lock(struct work_struct *work) >> dout("%s rbd_dev %p requeuing lock_dwork\n", __func__, >> rbd_dev); >> mod_delayed_work(rbd_dev->task_wq, &rbd_dev->lock_dwork, >> - msecs_to_jiffies(2 * RBD_NOTIFY_TIMEOUT * MSEC_PER_SEC)); >> + secs_to_jiffies(2 * RBD_NOTIFY_TIMEOUT)); >> } >> } >> @@ -6283,9 +6283,9 @@ static int rbd_parse_param(struct fs_parameter *param, >> break; >> case Opt_lock_timeout: >> /* 0 is "wait forever" (i.e. infinite timeout) */ >> - if (result.uint_32 > INT_MAX / 1000) > > Previously, the above line was verifying that the multiplication > done below would not overflow. It was unrelated to whatever > msecs_to_jiffies() did. > >> + if (result.uint32 > INT_MAX / HZ) > > Here you are assuming something about what secs_to_jiffies() > does. It's a very reasonable assumption, but you are encoding > this in unrelated code, which you shouldn't do. > > Just do the direct conversion as you've done above: > > if (result.uint32 > INT_MAX) > >> goto out_of_range; >> - opt->lock_timeout = msecs_to_jiffies(result.uint_32 * 1000); >> + opt->lock_timeout = secs_to_jiffies(result.uint_32); > > Unfortunately, secs_to_jiffies() does not implement the clamp > operation that msecs_to_jiffies() does. If you look at > __msecs_to_jiffies() you see that the unsigned value provided > is limited to MAX_JIFFY_OFFSET if it's negative when interpreted > as a signed int (i.e., if its high bit is set). > > I think the secs_to_jiffies() implementation could benefit > from the use of an overflow check. This might not be > exactly right, but it gives the idea: Fair enough, let me explore this. @Jens, @Andrew, can you drop the rbd and libceph patches from your respective trees while I work this out? Thanks, Easwar (he/him) > > #define secs_to_jiffies(_secs) \ > ({ \ > unsigned long _result; \ > \ > if (check_mul_overflow(_secs, HZ, &_result)) \ > _result = MAX_JIFFY_OFFSET; \ > (_result); \ > }) > > -Alex > > >> break; >> case Opt_pool_ns: >> kfree(pctx->spec->pool_ns); >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/2] rbd: convert timeouts to secs_to_jiffies() 2025-03-03 18:13 ` Easwar Hariharan @ 2025-03-03 18:18 ` Jens Axboe 0 siblings, 0 replies; 7+ messages in thread From: Jens Axboe @ 2025-03-03 18:18 UTC (permalink / raw) To: Easwar Hariharan, Alex Elder, Andrew Morton Cc: Christophe JAILLET, Daniel Vacek, Ilya Dryomov, Dongsheng Yang, Xiubo Li, ceph-devel, linux-block, linux-kernel On 3/3/25 11:13 AM, Easwar Hariharan wrote: > @Jens, @Andrew, can you drop the rbd and libceph patches from your respective > trees while I work this out? Sure... -- Jens Axboe ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 2/2] libceph: convert timeouts to secs_to_jiffies() 2025-03-01 4:22 [PATCH v4 0/2] Converge on using secs_to_jiffies() part two Easwar Hariharan 2025-03-01 4:22 ` [PATCH v4 1/2] rbd: convert timeouts to secs_to_jiffies() Easwar Hariharan @ 2025-03-01 4:22 ` Easwar Hariharan 2025-03-02 17:09 ` Alex Elder 1 sibling, 1 reply; 7+ messages in thread From: Easwar Hariharan @ 2025-03-01 4:22 UTC (permalink / raw) To: Andrew Morton, Christophe JAILLET, Daniel Vacek, Ilya Dryomov, Dongsheng Yang, Jens Axboe, Xiubo Li Cc: ceph-devel, linux-block, linux-kernel, Easwar Hariharan Commit b35108a51cf7 ("jiffies: Define secs_to_jiffies()") introduced secs_to_jiffies(). As the value here is a multiple of 1000, use secs_to_jiffies() instead of msecs_to_jiffies() to avoid the multiplication This is converted using scripts/coccinelle/misc/secs_to_jiffies.cocci with the following Coccinelle rules: @depends on patch@ expression E; @@ -msecs_to_jiffies(E * 1000) +secs_to_jiffies(E) @depends on patch@ expression E; @@ -msecs_to_jiffies(E * MSEC_PER_SEC) +secs_to_jiffies(E) Change the checks for range to check against HZ. Acked-by: Ilya Dryomov <idryomov@gmail.com> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com> --- include/linux/ceph/libceph.h | 12 ++++++------ net/ceph/ceph_common.c | 18 ++++++++---------- net/ceph/osd_client.c | 3 +-- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/include/linux/ceph/libceph.h b/include/linux/ceph/libceph.h index 733e7f93db66a7a29a4a8eba97e9ebf2c49da1f9..5f57128ef0c7d018341c15cc59288aa47edec646 100644 --- a/include/linux/ceph/libceph.h +++ b/include/linux/ceph/libceph.h @@ -72,15 +72,15 @@ struct ceph_options { /* * defaults */ -#define CEPH_MOUNT_TIMEOUT_DEFAULT msecs_to_jiffies(60 * 1000) -#define CEPH_OSD_KEEPALIVE_DEFAULT msecs_to_jiffies(5 * 1000) -#define CEPH_OSD_IDLE_TTL_DEFAULT msecs_to_jiffies(60 * 1000) +#define CEPH_MOUNT_TIMEOUT_DEFAULT secs_to_jiffies(60) +#define CEPH_OSD_KEEPALIVE_DEFAULT secs_to_jiffies(5) +#define CEPH_OSD_IDLE_TTL_DEFAULT secs_to_jiffies(60) #define CEPH_OSD_REQUEST_TIMEOUT_DEFAULT 0 /* no timeout */ #define CEPH_READ_FROM_REPLICA_DEFAULT 0 /* read from primary */ -#define CEPH_MONC_HUNT_INTERVAL msecs_to_jiffies(3 * 1000) -#define CEPH_MONC_PING_INTERVAL msecs_to_jiffies(10 * 1000) -#define CEPH_MONC_PING_TIMEOUT msecs_to_jiffies(30 * 1000) +#define CEPH_MONC_HUNT_INTERVAL secs_to_jiffies(3) +#define CEPH_MONC_PING_INTERVAL secs_to_jiffies(10) +#define CEPH_MONC_PING_TIMEOUT secs_to_jiffies(30) #define CEPH_MONC_HUNT_BACKOFF 2 #define CEPH_MONC_HUNT_MAX_MULT 10 diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c index 4c6441536d55b6323f4b9d93b5d4837cd4ec880c..ee701b39960e1c9778db91936ac7503467ee1162 100644 --- a/net/ceph/ceph_common.c +++ b/net/ceph/ceph_common.c @@ -527,29 +527,27 @@ int ceph_parse_param(struct fs_parameter *param, struct ceph_options *opt, case Opt_osdkeepalivetimeout: /* 0 isn't well defined right now, reject it */ - if (result.uint_32 < 1 || result.uint_32 > INT_MAX / 1000) + if (result.uint_32 < 1 || result.uint32 > INT_MAX / HZ) goto out_of_range; - opt->osd_keepalive_timeout = - msecs_to_jiffies(result.uint_32 * 1000); + opt->osd_keepalive_timeout = secs_to_jiffies(result.uint_32); break; case Opt_osd_idle_ttl: /* 0 isn't well defined right now, reject it */ - if (result.uint_32 < 1 || result.uint_32 > INT_MAX / 1000) + if (result.uint_32 < 1 || result.uint32 > INT_MAX / HZ) goto out_of_range; - opt->osd_idle_ttl = msecs_to_jiffies(result.uint_32 * 1000); + opt->osd_idle_ttl = secs_to_jiffies(result.uint_32); break; case Opt_mount_timeout: /* 0 is "wait forever" (i.e. infinite timeout) */ - if (result.uint_32 > INT_MAX / 1000) + if (result.uint32 > INT_MAX / HZ) goto out_of_range; - opt->mount_timeout = msecs_to_jiffies(result.uint_32 * 1000); + opt->mount_timeout = secs_to_jiffies(result.uint_32); break; case Opt_osd_request_timeout: /* 0 is "wait forever" (i.e. infinite timeout) */ - if (result.uint_32 > INT_MAX / 1000) + if (result.uint32 > INT_MAX / HZ) goto out_of_range; - opt->osd_request_timeout = - msecs_to_jiffies(result.uint_32 * 1000); + opt->osd_request_timeout = secs_to_jiffies(result.uint_32); break; case Opt_share: diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c index b24afec241382b60d775dd12a6561fa23a7eca45..ba61a48b4388c2eceb5b7a299906e7f90191dd5d 100644 --- a/net/ceph/osd_client.c +++ b/net/ceph/osd_client.c @@ -4989,8 +4989,7 @@ int ceph_osdc_notify(struct ceph_osd_client *osdc, linger_submit(lreq); ret = linger_reg_commit_wait(lreq); if (!ret) - ret = linger_notify_finish_wait(lreq, - msecs_to_jiffies(2 * timeout * MSEC_PER_SEC)); + ret = linger_notify_finish_wait(lreq, secs_to_jiffies(2 * timeout)); else dout("lreq %p failed to initiate notify %d\n", lreq, ret); -- 2.43.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 2/2] libceph: convert timeouts to secs_to_jiffies() 2025-03-01 4:22 ` [PATCH v4 2/2] libceph: " Easwar Hariharan @ 2025-03-02 17:09 ` Alex Elder 0 siblings, 0 replies; 7+ messages in thread From: Alex Elder @ 2025-03-02 17:09 UTC (permalink / raw) To: Easwar Hariharan, Andrew Morton, Christophe JAILLET, Daniel Vacek, Ilya Dryomov, Dongsheng Yang, Jens Axboe, Xiubo Li Cc: ceph-devel, linux-block, linux-kernel On 2/28/25 10:22 PM, Easwar Hariharan wrote: > Commit b35108a51cf7 ("jiffies: Define secs_to_jiffies()") introduced > secs_to_jiffies(). As the value here is a multiple of 1000, use > secs_to_jiffies() instead of msecs_to_jiffies() to avoid the multiplication > > This is converted using scripts/coccinelle/misc/secs_to_jiffies.cocci with > the following Coccinelle rules: > > @depends on patch@ expression E; @@ > > -msecs_to_jiffies(E * 1000) > +secs_to_jiffies(E) > > @depends on patch@ expression E; @@ > > -msecs_to_jiffies(E * MSEC_PER_SEC) > +secs_to_jiffies(E) > > Change the checks for range to check against HZ. > > Acked-by: Ilya Dryomov <idryomov@gmail.com> > Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com> Much of this is fine, but I have the same comment on the changes where you bring HZ into the checks, which is wrong. Just convert the code more directly, factoring out 1000 *only*. > --- > include/linux/ceph/libceph.h | 12 ++++++------ > net/ceph/ceph_common.c | 18 ++++++++---------- > net/ceph/osd_client.c | 3 +-- > 3 files changed, 15 insertions(+), 18 deletions(-) > > diff --git a/include/linux/ceph/libceph.h b/include/linux/ceph/libceph.h > index 733e7f93db66a7a29a4a8eba97e9ebf2c49da1f9..5f57128ef0c7d018341c15cc59288aa47edec646 100644 > --- a/include/linux/ceph/libceph.h > +++ b/include/linux/ceph/libceph.h > @@ -72,15 +72,15 @@ struct ceph_options { > /* > * defaults > */ > -#define CEPH_MOUNT_TIMEOUT_DEFAULT msecs_to_jiffies(60 * 1000) > -#define CEPH_OSD_KEEPALIVE_DEFAULT msecs_to_jiffies(5 * 1000) > -#define CEPH_OSD_IDLE_TTL_DEFAULT msecs_to_jiffies(60 * 1000) > +#define CEPH_MOUNT_TIMEOUT_DEFAULT secs_to_jiffies(60) > +#define CEPH_OSD_KEEPALIVE_DEFAULT secs_to_jiffies(5) > +#define CEPH_OSD_IDLE_TTL_DEFAULT secs_to_jiffies(60) > #define CEPH_OSD_REQUEST_TIMEOUT_DEFAULT 0 /* no timeout */ > #define CEPH_READ_FROM_REPLICA_DEFAULT 0 /* read from primary */ > > -#define CEPH_MONC_HUNT_INTERVAL msecs_to_jiffies(3 * 1000) > -#define CEPH_MONC_PING_INTERVAL msecs_to_jiffies(10 * 1000) > -#define CEPH_MONC_PING_TIMEOUT msecs_to_jiffies(30 * 1000) > +#define CEPH_MONC_HUNT_INTERVAL secs_to_jiffies(3) > +#define CEPH_MONC_PING_INTERVAL secs_to_jiffies(10) > +#define CEPH_MONC_PING_TIMEOUT secs_to_jiffies(30) > #define CEPH_MONC_HUNT_BACKOFF 2 > #define CEPH_MONC_HUNT_MAX_MULT 10 > > diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c > index 4c6441536d55b6323f4b9d93b5d4837cd4ec880c..ee701b39960e1c9778db91936ac7503467ee1162 100644 > --- a/net/ceph/ceph_common.c > +++ b/net/ceph/ceph_common.c > @@ -527,29 +527,27 @@ int ceph_parse_param(struct fs_parameter *param, struct ceph_options *opt, > > case Opt_osdkeepalivetimeout: > /* 0 isn't well defined right now, reject it */ > - if (result.uint_32 < 1 || result.uint_32 > INT_MAX / 1000) > + if (result.uint_32 < 1 || result.uint32 > INT_MAX / HZ) if (!result.uint32 || result.uint32 > INT_MAX) > goto out_of_range; > - opt->osd_keepalive_timeout = > - msecs_to_jiffies(result.uint_32 * 1000); > + opt->osd_keepalive_timeout = secs_to_jiffies(result.uint_32); > break; > case Opt_osd_idle_ttl: > /* 0 isn't well defined right now, reject it */ > - if (result.uint_32 < 1 || result.uint_32 > INT_MAX / 1000) > + if (result.uint_32 < 1 || result.uint32 > INT_MAX / HZ) if (!result.uint32 || result.uint32 > INT_MAX) > goto out_of_range; > - opt->osd_idle_ttl = msecs_to_jiffies(result.uint_32 * 1000); > + opt->osd_idle_ttl = secs_to_jiffies(result.uint_32); > break; > case Opt_mount_timeout: > /* 0 is "wait forever" (i.e. infinite timeout) */ > - if (result.uint_32 > INT_MAX / 1000) And so on. -Alex > + if (result.uint32 > INT_MAX / HZ) > goto out_of_range; > - opt->mount_timeout = msecs_to_jiffies(result.uint_32 * 1000); > + opt->mount_timeout = secs_to_jiffies(result.uint_32); > break; > case Opt_osd_request_timeout: > /* 0 is "wait forever" (i.e. infinite timeout) */ > - if (result.uint_32 > INT_MAX / 1000) > + if (result.uint32 > INT_MAX / HZ) > goto out_of_range; > - opt->osd_request_timeout = > - msecs_to_jiffies(result.uint_32 * 1000); > + opt->osd_request_timeout = secs_to_jiffies(result.uint_32); > break; > > case Opt_share: > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > index b24afec241382b60d775dd12a6561fa23a7eca45..ba61a48b4388c2eceb5b7a299906e7f90191dd5d 100644 > --- a/net/ceph/osd_client.c > +++ b/net/ceph/osd_client.c > @@ -4989,8 +4989,7 @@ int ceph_osdc_notify(struct ceph_osd_client *osdc, > linger_submit(lreq); > ret = linger_reg_commit_wait(lreq); > if (!ret) > - ret = linger_notify_finish_wait(lreq, > - msecs_to_jiffies(2 * timeout * MSEC_PER_SEC)); > + ret = linger_notify_finish_wait(lreq, secs_to_jiffies(2 * timeout)); > else > dout("lreq %p failed to initiate notify %d\n", lreq, ret); > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-03-03 18:18 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-03-01 4:22 [PATCH v4 0/2] Converge on using secs_to_jiffies() part two Easwar Hariharan 2025-03-01 4:22 ` [PATCH v4 1/2] rbd: convert timeouts to secs_to_jiffies() Easwar Hariharan 2025-03-02 17:05 ` Alex Elder 2025-03-03 18:13 ` Easwar Hariharan 2025-03-03 18:18 ` Jens Axboe 2025-03-01 4:22 ` [PATCH v4 2/2] libceph: " Easwar Hariharan 2025-03-02 17:09 ` Alex Elder
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®