Converge on using secs_to_jiffies()

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:

@@ constant C; @@

- msecs_to_jiffies(C * 1000)
+ secs_to_jiffies(C)

@@ constant C; @@

- msecs_to_jiffies(C * MSEC_PER_SEC)
+ secs_to_jiffies(C)

While here, replace the schedule_delayed_work() call with a 0 timeout with an immediate schedule_work() call.

Signed-off-by Easwar Hariharan <[email protected]>

---
  samples/livepatch/livepatch-callbacks-busymod.c |  3 +--
  samples/livepatch/livepatch-shadow-fix1.c       |  3 +--
  samples/livepatch/livepatch-shadow-mod.c        | 15 +++++----------
  3 files changed, 7 insertions(+), 14 deletions(-)

samples/livepatch/livepatch-callbacks-busymod.c +1 -2

index 378e2d40271a..0220f7715fcc 100644
@@ -44,8 +44,7 @@ static void busymod_work_func(struct work_struct *work)
  static int livepatch_callbacks_mod_init(void)
  {
  	pr_info("%s\n", __func__);
-	schedule_delayed_work(&work,
-		msecs_to_jiffies(1000 * 0));
+	schedule_work(&work);
  	return 0;
  }
 

samples/livepatch/livepatch-shadow-fix1.c +1 -2

index 6701641bf12d..f3f153895d6c 100644
@@ -72,8 +72,7 @@ static struct dummy *livepatch_fix1_dummy_alloc(void)
  	if (!d)
  		return NULL;
 
-	d->jiffies_expire = jiffies +
-		msecs_to_jiffies(1000 * EXPIRE_PERIOD);
+	d->jiffies_expire = jiffies + secs_to_jiffies(EXPIRE_PERIOD);
 
  	/*
  	 * Patch: save the extra memory location into a SV_LEAK shadow

samples/livepatch/livepatch-shadow-mod.c +5 -10

index 7e753b0d2fa6..5d83ad5a8118 100644
@@ -101,8 +101,7 @@ static __used noinline struct dummy *dummy_alloc(void)
  	if (!d)
  		return NULL;
 
-	d->jiffies_expire = jiffies +
-		msecs_to_jiffies(1000 * EXPIRE_PERIOD);
+	d->jiffies_expire = jiffies + secs_to_jiffies(EXPIRE_PERIOD);
 
  	/* Oops, forgot to save leak! */
  	leak = kzalloc(sizeof(*leak), GFP_KERNEL);

@@ -152,8 +151,7 @@ static void alloc_work_func(struct work_struct *work)
  	list_add(&d->list, &dummy_list);
  	mutex_unlock(&dummy_list_mutex);
 
-	schedule_delayed_work(&alloc_dwork,
-		msecs_to_jiffies(1000 * ALLOC_PERIOD));
+	schedule_delayed_work(&alloc_dwork, secs_to_jiffies(ALLOC_PERIOD));
  }
 
  /*

@@ -184,16 +182,13 @@ 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));
+	schedule_delayed_work(&cleanup_dwork, secs_to_jiffies(CLEANUP_PERIOD));
  }
 
  static int livepatch_shadow_mod_init(void)
  {
-	schedule_delayed_work(&alloc_dwork,
-		msecs_to_jiffies(1000 * ALLOC_PERIOD));
-	schedule_delayed_work(&cleanup_dwork,
-		msecs_to_jiffies(1000 * CLEANUP_PERIOD));
+	schedule_delayed_work(&alloc_dwork, secs_to_jiffies(ALLOC_PERIOD));
+	schedule_delayed_work(&cleanup_dwork, secs_to_jiffies(CLEANUP_PERIOD));
 
  	return 0;
  }

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:

@@ constant C; @@

- msecs_to_jiffies(C * 1000)
+ secs_to_jiffies(C)

@@ constant C; @@

- msecs_to_jiffies(C * MSEC_PER_SEC)
+ secs_to_jiffies(C)

While here, replace the schedule_delayed_work() call with a 0 timeout with an immediate schedule_work() call.

Quoted 20 lines

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:

@@ constant C; @@

- msecs_to_jiffies(C * 1000)
+ secs_to_jiffies(C)

@@ constant C; @@

- msecs_to_jiffies(C * MSEC_PER_SEC)
+ secs_to_jiffies(C)

While here, replace the schedule_delayed_work() call with a 0 timeout with an immediate schedule_work() call.

samples/livepatch/livepatch-callbacks-busymod.c static void busymod_work_func(struct work_struct *work) · line 47

Quoted 9 lines

samples/livepatch/livepatch-callbacks-busymod.c +1 -2

@@ -44,8 +44,7 @@ static void busymod_work_func(struct work_struct *work)
  static int livepatch_callbacks_mod_init(void)
  {
  	pr_info("%s\n", __func__);
-	schedule_delayed_work(&work,
-		msecs_to_jiffies(1000 * 0));
+	schedule_work(&work);

Is it safe to use schedule_work() for struct delayed_work?

It might work in theory but I do not feel comfortable with it. Also I would expect a compiler warning.

If you really want to use schedule_work() then please also define the structure with DECLARE_WORK()
and use cancel_work_sync() in livepatch_callbacks_mod_exit().

Best Regards,
Petr

Le 18/12/2024 à 09:38, Petr Mladek a écrit :

Quoted 23 lines · Easwar Hariharan

Quoted 22 lines

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:

@@ constant C; @@

- msecs_to_jiffies(C * 1000)
+ secs_to_jiffies(C)

@@ constant C; @@

- msecs_to_jiffies(C * MSEC_PER_SEC)
+ secs_to_jiffies(C)

While here, replace the schedule_delayed_work() call with a 0 timeout with an immediate schedule_work() call.

Quoted 12 lines

samples/livepatch/livepatch-callbacks-busymod.c static void busymod_work_func(struct work_struct · line 48

Quoted 10 lines

samples/livepatch/livepatch-callbacks-busymod.c +1 -2

@@ -44,8 +44,7 @@ static void busymod_work_func(struct work_struct
 *work)
   static int livepatch_callbacks_mod_init(void)
   {
       pr_info("%s\n", __func__);
-    schedule_delayed_work(&work,
-        msecs_to_jiffies(1000 * 0));
+    schedule_work(&work);

Is it safe to use schedule_work() for struct delayed_work?

Should be, but you are right it should then be a standard work not a delayed work.

So probably the easiest is to keep

    schedule_delayed_work(&work, 0)

And eventually changing it to a not delayed work could be a follow-up patch.

Quoted 1 line

Quoted 42 lines · Christophe Leroy

Le 18/12/2024 à 09:38, Petr Mladek a écrit :

Quoted 23 lines · Easwar Hariharan

Quoted 22 lines

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:

@@ constant C; @@

- msecs_to_jiffies(C * 1000)
+ secs_to_jiffies(C)

@@ constant C; @@

- msecs_to_jiffies(C * MSEC_PER_SEC)
+ secs_to_jiffies(C)

While here, replace the schedule_delayed_work() call with a 0 timeout with an immediate schedule_work() call.

Quoted 12 lines

samples/livepatch/livepatch-callbacks-busymod.c static void busymod_work_func(struct work_struct · line 48

Quoted 10 lines

samples/livepatch/livepatch-callbacks-busymod.c +1 -2

@@ -44,8 +44,7 @@ static void busymod_work_func(struct work_struct
 *work)
   static int livepatch_callbacks_mod_init(void)
   {
       pr_info("%s\n", __func__);
-    schedule_delayed_work(&work,
-        msecs_to_jiffies(1000 * 0));
+    schedule_work(&work);

Is it safe to use schedule_work() for struct delayed_work?

Should be, but you are right it should then be a standard work not a delayed work.

Quoted 8 lines

So probably the easiest is to keep

    schedule_delayed_work(&work, 0)

And eventually changing it to a not delayed work could be a follow-up patch.

Quoted 1 line

Thanks for the catch, Petr! This suggestion would effectively revert this patch to the v3 version, albeit with some extra explanation in the commit message. I'd propose just keeping the v3 in the next branch where it is.

Andrew, Petr, Christophe, what do you think?