| Message ID | 20241115-converge-secs-to-jiffies-v2-5-911fb7595e79@linux.microsoft.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| 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) Is it a special script or is it done with the script in patch 2. That's nice to say how it is done, but you also have to say _what_ and _why_ you do it. This is even more important as you plan to get it merged independently in each tree instead of merging it as a single series. It could be something like: 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 > > Signed-off-by: Easwar Hariharan <eahariha@linux.microsoft.com> > --- > arch/powerpc/platforms/pseries/papr_scm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c > index 9e297f88adc5d97d4dc7b267b0bfebd58e5cf193..9e8086ec66e0f0e555ac27933854c06cfcf91a04 100644 > --- a/arch/powerpc/platforms/pseries/papr_scm.c > +++ b/arch/powerpc/platforms/pseries/papr_scm.c > @@ -543,7 +543,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p) > > /* Jiffies offset for which the health data is assumed to be same */ > cache_timeout = p->lasthealth_jiffies + > - msecs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL * 1000); > + secs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL); Wouldn't it now fit on a single line ? cache_timeout = p->lasthealth_jiffies + secs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL); Also I'm not sure it is worth the MIN_HEALTH_QUERY_INTERVAL macro as it is defined localy and used only once, but that's another story. > > /* Fetch new health info is its older than MIN_HEALTH_QUERY_INTERVAL */ > if (time_after(jiffies, cache_timeout)) > > -- > 2.34.1 >
On Sat, Nov 16, 2024 at 11:06:55AM +0100, Christophe Leroy wrote: > > diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c > > index 9e297f88adc5d97d4dc7b267b0bfebd58e5cf193..9e8086ec66e0f0e555ac27933854c06cfcf91a04 100644 > > --- a/arch/powerpc/platforms/pseries/papr_scm.c > > +++ b/arch/powerpc/platforms/pseries/papr_scm.c > > @@ -543,7 +543,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p) > > > > /* Jiffies offset for which the health data is assumed to be same */ > > cache_timeout = p->lasthealth_jiffies + > > - msecs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL * 1000); > > + secs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL); > > Wouldn't it now fit on a single line ? > Some maintainers still prefer to put a line break at 80 characters. It's kind of a nightmare for an automated script like this to figure out everyone's preferences. In this particular file, there are some lines which go over 80 characters so sure. Earlier in the patchset one of these introduced a line break that wasn't there before so I think maybe Coccinelle is applying the 80 character line break rule? There are sometimes where the 80 character rule really hurts readability, but here it doesn't make any difference. regards, dan carpenter
Sent from my iPhone > On 16 Nov 2024, at 05:40, Dan Carpenter <dan.carpenter@linaro.org> wrote: > > On Sat, Nov 16, 2024 at 11:06:55AM +0100, Christophe Leroy wrote: >>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c >>> index 9e297f88adc5d97d4dc7b267b0bfebd58e5cf193..9e8086ec66e0f0e555ac27933854c06cfcf91a04 100644 >>> --- a/arch/powerpc/platforms/pseries/papr_scm.c >>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c >>> @@ -543,7 +543,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p) >>> >>> /* Jiffies offset for which the health data is assumed to be same */ >>> cache_timeout = p->lasthealth_jiffies + >>> - msecs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL * 1000); >>> + secs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL); >> >> Wouldn't it now fit on a single line ? >> > > Some maintainers still prefer to put a line break at 80 characters. Coccinelle tries for 80 chars. It may have a command line option to specify something else. Julia > It's kind > of a nightmare for an automated script like this to figure out everyone's > preferences. In this particular > file, there are some lines which go over 80 > characters so sure. Earlier in the patchset one of these introduced a line > break that wasn't there before so I think maybe Coccinelle is applying the 80 > character line break rule? > > There are sometimes where the 80 character rule really hurts readability, but > here it doesn't make any difference. > > regards, > dan carpenter >
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c index 9e297f88adc5d97d4dc7b267b0bfebd58e5cf193..9e8086ec66e0f0e555ac27933854c06cfcf91a04 100644 --- a/arch/powerpc/platforms/pseries/papr_scm.c +++ b/arch/powerpc/platforms/pseries/papr_scm.c @@ -543,7 +543,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p) /* Jiffies offset for which the health data is assumed to be same */ cache_timeout = p->lasthealth_jiffies + - msecs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL * 1000); + secs_to_jiffies(MIN_HEALTH_QUERY_INTERVAL); /* Fetch new health info is its older than MIN_HEALTH_QUERY_INTERVAL */ if (time_after(jiffies, cache_timeout))
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> --- arch/powerpc/platforms/pseries/papr_scm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)