[PATCH v2 0/3] Converge on using secs_to_jiffies() part two

[PATCH v2 0/3] Converge on using secs_to_jiffies() part two

[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-20250203

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 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 (3):
      coccinelle: misc: secs_to_jiffies: Patch expressions too
      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 +--
 scripts/coccinelle/misc/secs_to_jiffies.cocci | 10 ++++++++++
 5 files changed, 26 insertions(+), 25 deletions(-)
---
base-commit: 00f3246adeeacbda0bd0b303604e46eb59c32e6e
change-id: 20241217-converge-secs-to-jiffies-part-two-f44017aa6f67

Best regards,
-- 
Easwar Hariharan <eahariha@linux.microsoft.com>

[PATCH v2 1/3] coccinelle: misc: secs_to_jiffies: Patch expressions too

[Easwar Hariharan]

Teach the script to suggest conversions for timeout patterns where the
arguments to msecs_to_jiffies() are expressions as well.

Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
---
 scripts/coccinelle/misc/secs_to_jiffies.cocci | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/scripts/coccinelle/misc/secs_to_jiffies.cocci b/scripts/coccinelle/misc/secs_to_jiffies.cocci
index 8bbb2884ea5db939c63fd4513cf5ca8c977aa8cb..416f348174ca659b544441f5f68f04a41d1ad4a3 100644
--- a/scripts/coccinelle/misc/secs_to_jiffies.cocci
+++ b/scripts/coccinelle/misc/secs_to_jiffies.cocci
@@ -20,3 +20,13 @@ virtual patch
 
 - msecs_to_jiffies(C * MSEC_PER_SEC)
 + secs_to_jiffies(C)
+
+@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)

-- 
2.43.0

[PATCH v2 2/3] rbd: convert timeouts to secs_to_jiffies()

[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)

While here, remove the no-longer necessary check for range since there's
no multiplication involved.

Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
---
 drivers/block/rbd.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index faafd7ff43d6ef53110ab3663cc7ac322214cc8c..41207133e21e9203192adf3b92390818e8fa5a58 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,7 @@ 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)
-			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

[PATCH v2 3/3] libceph: convert timeouts to secs_to_jiffies()

[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)

While here, remove the no-longer necessary checks for range since there's
no multiplication involved.

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, 13 insertions(+), 20 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..c2a2c3bcc4e91a628c99bd1cef1211d54389efa2 100644
--- a/net/ceph/ceph_common.c
+++ b/net/ceph/ceph_common.c
@@ -527,29 +527,23 @@ 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)
 			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)
 			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)
-			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)
-			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

Re: [cocci] [PATCH v2 1/3] coccinelle: misc: secs_to_jiffies: Patch expressions too

[Markus Elfring]

> Teach the script to suggest conversions for timeout patterns where the
> arguments to msecs_to_jiffies() are expressions as well.
How do you imagine that the shown SmPL code fits ever to patch reviews?

Examples:
* https://lore.kernel.org/cocci/80cae791-663c-4589-b67e-d4d1049fcd98@web.de/
  https://sympa.inria.fr/sympa/arc/cocci/2025-02/msg00002.html
  https://lkml.org/lkml/2025/2/3/151

* https://lore.kernel.org/cocci/e06cb7f5-7aa3-464c-a8a1-2c7b9b6a29eb@web.de/
  https://sympa.inria.fr/sympa/arc/cocci/2025-01/msg00122.html
  https://lkml.org/lkml/2025/1/30/307


Regards,
Markus

Re: [cocci] [PATCH v2 1/3] coccinelle: misc: secs_to_jiffies: Patch expressions too

[Easwar Hariharan]

On 2/3/2025 11:56 PM, Markus Elfring wrote:
>> Teach the script to suggest conversions for timeout patterns where the
>> arguments to msecs_to_jiffies() are expressions as well.
> How do you imagine that the shown SmPL code fits ever to patch reviews?

By the simple fact of accomplishing the same result despite differences in
code style, as explicitly called out in the changelog.

- Easwar (he/him)

Re: [PATCH v2 2/3] rbd: convert timeouts to secs_to_jiffies()

[Ilya Dryomov]

On Tue, Feb 4, 2025 at 12:50 AM Easwar Hariharan
<eahariha@linux.microsoft.com> 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)
>
> While here, remove the no-longer necessary check for range since there's
> no multiplication involved.
>
> Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com>
> ---
>  drivers/block/rbd.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index faafd7ff43d6ef53110ab3663cc7ac322214cc8c..41207133e21e9203192adf3b92390818e8fa5a58 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,7 @@ 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)
> -                       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
>

Acked-by: Ilya Dryomov <idryomov@gmail.com>

Thanks,

                Ilya

Re: [PATCH v2 3/3] libceph: convert timeouts to secs_to_jiffies()

[Ilya Dryomov]

On Tue, Feb 4, 2025 at 12:50 AM Easwar Hariharan
<eahariha@linux.microsoft.com> 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)
>
> While here, remove the no-longer necessary checks for range since there's
> no multiplication involved.
>
> 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, 13 insertions(+), 20 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..c2a2c3bcc4e91a628c99bd1cef1211d54389efa2 100644
> --- a/net/ceph/ceph_common.c
> +++ b/net/ceph/ceph_common.c
> @@ -527,29 +527,23 @@ 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)
>                         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)
>                         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)
> -                       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)
> -                       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
>

Acked-by: Ilya Dryomov <idryomov@gmail.com>

Thanks,

                Ilya

Re: [cocci] [v2 1/3] coccinelle: misc: secs_to_jiffies: Patch expressions too

[Markus Elfring]

>> How do you imagine that the shown SmPL code fits ever to patch reviews?
>
> By the simple fact of accomplishing the same result despite differences in
> code style, as explicitly called out in the changelog.
Do any other contributors care more for the mentioned software development concerns?

How do you think about helping to improve affected implementation details?

Regards,
Markus