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 <eahariha@linux.microsoft.com>
---
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(-)
diff --git a/samples/livepatch/livepatch-callbacks-busymod.c b/samples/livepatch/livepatch-callbacks-busymod.c
index 378e2d40271a..0220f7715fcc 100644
--- a/samples/livepatch/livepatch-callbacks-busymod.c
+++ b/samples/livepatch/livepatch-callbacks-busymod.c
@@ -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;
}
diff --git a/samples/livepatch/livepatch-shadow-fix1.c b/samples/livepatch/livepatch-shadow-fix1.c
index 6701641bf12d..f3f153895d6c 100644
--- a/samples/livepatch/livepatch-shadow-fix1.c
+++ b/samples/livepatch/livepatch-shadow-fix1.c
@@ -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
diff --git a/samples/livepatch/livepatch-shadow-mod.c b/samples/livepatch/livepatch-shadow-mod.c
index 7e753b0d2fa6..5d83ad5a8118 100644
--- a/samples/livepatch/livepatch-shadow-mod.c
+++ b/samples/livepatch/livepatch-shadow-mod.c
@@ -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;
}
--
2.43.0On Tue 2024-12-17 23:09:59, 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:
>
> @@ 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.
>
> --- a/samples/livepatch/livepatch-callbacks-busymod.c
> +++ b/samples/livepatch/livepatch-callbacks-busymod.c
> @@ -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 :
> On Tue 2024-12-17 23:09:59, 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:
>>
>> @@ 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.
>>
>> --- a/samples/livepatch/livepatch-callbacks-busymod.c
>> +++ b/samples/livepatch/livepatch-callbacks-busymod.c
>> @@ -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.
>
> It might work in theory but I do not feel comfortable with it.
> Also I would expect a compiler warning.
__queue_delayed_work() does :
if (!delay) {
__queue_work(cpu, wq, &dwork->work);
return;
}
>
> 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
On 12/18/2024 12:48 AM, Christophe Leroy wrote:
>
>
> Le 18/12/2024 à 09:38, Petr Mladek a écrit :
>> On Tue 2024-12-17 23:09:59, 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:
>>>
>>> @@ 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.
>>>
>>> --- a/samples/livepatch/livepatch-callbacks-busymod.c
>>> +++ b/samples/livepatch/livepatch-callbacks-busymod.c
>>> @@ -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.
>
>>
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?
On Wed 2024-12-18 09:35:46, Easwar Hariharan wrote:
> On 12/18/2024 12:48 AM, Christophe Leroy wrote:
> >
> >
> > Le 18/12/2024 à 09:38, Petr Mladek a écrit :
> >> On Tue 2024-12-17 23:09:59, 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:
> >>>
> >>> @@ 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.
> >>>
> >>> --- a/samples/livepatch/livepatch-callbacks-busymod.c
> >>> +++ b/samples/livepatch/livepatch-callbacks-busymod.c
> >>> @@ -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.
> >
> >>
>
> 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?
I am fine with keeping v3 in next.
Best Regards,
Petr
Le 18/12/2024 à 00:09, Easwar Hariharan a écrit :
> 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 <eahariha@linux.microsoft.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> 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(-)
>
> diff --git a/samples/livepatch/livepatch-callbacks-busymod.c b/samples/livepatch/livepatch-callbacks-busymod.c
> index 378e2d40271a..0220f7715fcc 100644
> --- a/samples/livepatch/livepatch-callbacks-busymod.c
> +++ b/samples/livepatch/livepatch-callbacks-busymod.c
> @@ -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;
> }
>
> diff --git a/samples/livepatch/livepatch-shadow-fix1.c b/samples/livepatch/livepatch-shadow-fix1.c
> index 6701641bf12d..f3f153895d6c 100644
> --- a/samples/livepatch/livepatch-shadow-fix1.c
> +++ b/samples/livepatch/livepatch-shadow-fix1.c
> @@ -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
> diff --git a/samples/livepatch/livepatch-shadow-mod.c b/samples/livepatch/livepatch-shadow-mod.c
> index 7e753b0d2fa6..5d83ad5a8118 100644
> --- a/samples/livepatch/livepatch-shadow-mod.c
> +++ b/samples/livepatch/livepatch-shadow-mod.c
> @@ -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;
> }
© 2016 - 2025 Red Hat, Inc.