Message ID | 20241115-converge-secs-to-jiffies-v2-19-911fb7595e79@linux.microsoft.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Converge on using secs_to_jiffies() | expand |
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 >
On Sat 2024-11-16 11:10:52, Christophe Leroy wrote: > > > 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); Yes, schedule_delayed_work(&work, 0) looks like the right solution. Or even better, it seems that the delayed work might get replaced by a normal workqueue work. Anyway, I am working on a patchset which would remove this sample module. There is no need to put much effort into the clean up of this particular module. Do whatever is easiest for you. Best Regards, Petr
On 11/18/2024 3:06 AM, Petr Mladek wrote: > On Sat 2024-11-16 11:10:52, Christophe Leroy wrote: >> >> >> 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); > > Yes, schedule_delayed_work(&work, 0) looks like the right solution. > > Or even better, it seems that the delayed work might get replaced by > a normal workqueue work. > > Anyway, I am working on a patchset which would remove this sample > module. There is no need to put much effort into the clean up > of this particular module. Do whatever is easiest for you. > > Best Regards, > Petr If we're removing the module, I'll drop it from the series. Just to clarify, do you mean to remove all of samples/livepatch/* or some particular file(s)? Thanks, Easwar
> On Nov 19, 2024, at 02:18, Easwar Hariharan <eahariha@linux.microsoft.com> wrote: > > On 11/18/2024 3:06 AM, Petr Mladek wrote: >> On Sat 2024-11-16 11:10:52, Christophe Leroy wrote: >>> >>> >>> 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); >> >> Yes, schedule_delayed_work(&work, 0) looks like the right solution. >> >> Or even better, it seems that the delayed work might get replaced by >> a normal workqueue work. >> >> Anyway, I am working on a patchset which would remove this sample >> module. There is no need to put much effort into the clean up >> of this particular module. Do whatever is easiest for you. >> >> Best Regards, >> Petr > > If we're removing the module, I'll drop it from the series. Just to > clarify, do you mean to remove all of samples/livepatch/* or some > particular file(s)? > > Thanks, > Easwar > Hi Easwar! I think Petr is going to remove just this module away. Anyway, I don't think removing all of them is a good idea. Thanks. Wardenjohn.
On Mon 2024-11-18 10:18:49, Easwar Hariharan wrote: > On 11/18/2024 3:06 AM, Petr Mladek wrote: > > On Sat 2024-11-16 11:10:52, Christophe Leroy wrote: > >> > >> > >> 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); > > > > Yes, schedule_delayed_work(&work, 0) looks like the right solution. > > > > Or even better, it seems that the delayed work might get replaced by > > a normal workqueue work. > > > > Anyway, I am working on a patchset which would remove this sample > > module. There is no need to put much effort into the clean up > > of this particular module. Do whatever is easiest for you. > > > > Best Regards, > > Petr > > If we're removing the module, I'll drop it from the series. Just to > clarify, do you mean to remove all of samples/livepatch/* or some > particular file(s)? To be precise, I am going to replace: samples/livepatch/livepatch-callbacks-demo.c samples/livepatch/livepatch-callbacks-mod.c samples/livepatch/livepatch-callbacks-busymod.c with a completely different modules because I am reworking the callbacks API. All other sample modules are going to stay. Feel free to remove livepatch-callbacks-busymod.c from the patchset. But also feel free to keep it. The API rework goes slowly. I am not sure if it would be ready for 6.14. Best Regards, Petr
On Tue, 19 Nov 2024, Petr Mladek wrote: > On Mon 2024-11-18 10:18:49, Easwar Hariharan wrote: > > On 11/18/2024 3:06 AM, Petr Mladek wrote: > > > On Sat 2024-11-16 11:10:52, Christophe Leroy wrote: > > >> > > >> > > >> 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); > > > > > > Yes, schedule_delayed_work(&work, 0) looks like the right solution. > > > > > > Or even better, it seems that the delayed work might get replaced by > > > a normal workqueue work. > > > > > > Anyway, I am working on a patchset which would remove this sample > > > module. There is no need to put much effort into the clean up > > > of this particular module. Do whatever is easiest for you. > > > > > > Best Regards, > > > Petr > > > > If we're removing the module, I'll drop it from the series. Just to > > clarify, do you mean to remove all of samples/livepatch/* or some > > particular file(s)? > > To be precise, I am going to replace: > > samples/livepatch/livepatch-callbacks-demo.c > samples/livepatch/livepatch-callbacks-mod.c > samples/livepatch/livepatch-callbacks-busymod.c > > with a completely different modules because I am reworking the > callbacks API. > > All other sample modules are going to stay. > > Feel free to remove livepatch-callbacks-busymod.c from the patchset. > But also feel free to keep it. The API rework goes slowly. I am not > sure if it would be ready for 6.14. I would propose that Easwar goes on with his work and prepares an updated version of the patch based on Christophe's feedback. That is, disregarding Petr's rework for now. The patch set has a higher chance to be merged sooner. Petr can then easily rebase. If there is a conflict, we will handle it as usual. What do you think? Miroslav
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)); 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); /* 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)); } /* @@ -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)); } static int livepatch_shadow_mod_init(void) { schedule_delayed_work(&alloc_dwork, - msecs_to_jiffies(1000 * ALLOC_PERIOD)); + secs_to_jiffies(ALLOC_PERIOD)); schedule_delayed_work(&cleanup_dwork, - msecs_to_jiffies(1000 * CLEANUP_PERIOD)); + secs_to_jiffies(CLEANUP_PERIOD)); return 0; }
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(-)