Replace below code with the wrapper FIELD_MODIFY(MASK, ®, val)
- reg &= ~MASK;
- reg |= FIELD_PREP(MASK, val);
The semantic patch that makes this change is available
in scripts/coccinelle/misc/field_modify.cocci.
More information about semantic patching is available at
http://coccinelle.lip6.fr/
Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
arch/arm64/include/asm/tlbflush.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
index aa9efee17277..cdcee05bdf6d 100644
--- a/arch/arm64/include/asm/tlbflush.h
+++ b/arch/arm64/include/asm/tlbflush.h
@@ -112,8 +112,7 @@ static inline unsigned long get_trans_granule(void)
level >= 0 && level <= 3) { \
u64 ttl = level & 3; \
ttl |= get_trans_granule() << 2; \
- arg &= ~TLBI_TTL_MASK; \
- arg |= FIELD_PREP(TLBI_TTL_MASK, ttl); \
+ FIELD_MODIFY(TLBI_TTL_MASK, &arg, ttl); \
} \
\
__tlbi(op, arg); \
--
2.34.1I see further refinement possibilities for such a change description. > Replace below code with the wrapper FIELD_MODIFY(MASK, ®, val) > - reg &= ~MASK; > - reg |= FIELD_PREP(MASK, val); * How do you think about to omit leading minus characters? * Subsequent blank line? > More information about semantic patching is available at > http://coccinelle.lip6.fr/ I suggest to omit this information here (and in similar patches). Regards, Markus
On 6/13/2025 4:15 AM, Markus Elfring wrote: > I see further refinement possibilities for such a change description. > > >> Replace below code with the wrapper FIELD_MODIFY(MASK, ®, val) >> - reg &= ~MASK; >> - reg |= FIELD_PREP(MASK, val); > > * How do you think about to omit leading minus characters? > > * Subsequent blank line? > > >> More information about semantic patching is available at >> http://coccinelle.lip6.fr/ > > I suggest to omit this information here (and in similar patches). > > Regards, > Markus Thank you for your suggestions. The current commit message was generated by the following patch mode command: ``` make coccicheck MODE=patch COCCI=scripts/coccinelle/misc/field_modify.cocci V=1 ``` However, as I understand, the discussion on the ARM patches (between Russel/Marc/Yury) has concluded that the ARM arch changes may not be adding value over the current code, so I will drop the ARM patches in the next version. I will review the generated message to improve the formatting as you suggested, the next time I use it.
>>> Replace below code with the wrapper FIELD_MODIFY(MASK, ®, val) >>> - reg &= ~MASK; >>> - reg |= FIELD_PREP(MASK, val); … > I will review the generated message to improve the formatting as you > suggested, the next time I use it. Would indentation be more helpful for the mentioned code excerpt (instead of trying to describe lines with bullet points)? Regards, Markus
On 6/16/2025 9:52 PM, Markus Elfring wrote: >>>> Replace below code with the wrapper FIELD_MODIFY(MASK, ®, val) >>>> - reg &= ~MASK; >>>> - reg |= FIELD_PREP(MASK, val); > … >> I will review the generated message to improve the formatting as you >> suggested, the next time I use it. > Would indentation be more helpful for the mentioned code excerpt > (instead of trying to describe lines with bullet points)? > > Regards, > Markus I agree that using indentation would make the code excerpt clearer. Thanks.
On Mon, Jun 16, 2025 at 06:37:41PM +0800, Luo Jie wrote:
>
>
> On 6/13/2025 4:15 AM, Markus Elfring wrote:
> > I see further refinement possibilities for such a change description.
> >
> >
> > > Replace below code with the wrapper FIELD_MODIFY(MASK, ®, val)
> > > - reg &= ~MASK;
> > > - reg |= FIELD_PREP(MASK, val);
> >
> > * How do you think about to omit leading minus characters?
> >
> > * Subsequent blank line?
> >
> >
> > > More information about semantic patching is available at
> > > http://coccinelle.lip6.fr/
> >
> > I suggest to omit this information here (and in similar patches).
> >
> > Regards,
> > Markus
>
> Thank you for your suggestions. The current commit message was generated
> by the following patch mode command:
> ```
> make coccicheck MODE=patch COCCI=scripts/coccinelle/misc/field_modify.cocci
> V=1
> ```
> However, as I understand, the discussion on the ARM patches (between
> Russel/Marc/Yury) has concluded that the ARM arch changes may not be
> adding value over the current code, so I will drop the ARM patches
> in the next version.
Well, hang on a second. From what I can tell, the objections haven't
been specific to arch/arm{,64}/. You haven't really explained why this
new helper is needed and what value it brings over the existing set of
functionality.
So maybe start there, rather than dropping the parts that attracted the
comments to start with?
Will
On 6/16/2025 6:41 PM, Will Deacon wrote:
> On Mon, Jun 16, 2025 at 06:37:41PM +0800, Luo Jie wrote:
>>
>>
>> On 6/13/2025 4:15 AM, Markus Elfring wrote:
>>> I see further refinement possibilities for such a change description.
>>>
>>>
>>>> Replace below code with the wrapper FIELD_MODIFY(MASK, ®, val)
>>>> - reg &= ~MASK;
>>>> - reg |= FIELD_PREP(MASK, val);
>>>
>>> * How do you think about to omit leading minus characters?
>>>
>>> * Subsequent blank line?
>>>
>>>
>>>> More information about semantic patching is available at
>>>> http://coccinelle.lip6.fr/
>>>
>>> I suggest to omit this information here (and in similar patches).
>>>
>>> Regards,
>>> Markus
>>
>> Thank you for your suggestions. The current commit message was generated
>> by the following patch mode command:
>> ```
>> make coccicheck MODE=patch COCCI=scripts/coccinelle/misc/field_modify.cocci
>> V=1
>> ```
>> However, as I understand, the discussion on the ARM patches (between
>> Russel/Marc/Yury) has concluded that the ARM arch changes may not be
>> adding value over the current code, so I will drop the ARM patches
>> in the next version.
>
> Well, hang on a second. From what I can tell, the objections haven't
> been specific to arch/arm{,64}/. You haven't really explained why this
> new helper is needed and what value it brings over the existing set of
> functionality.
>
> So maybe start there, rather than dropping the parts that attracted the
> comments to start with?
>
> Will
FIELD_MODIFY is similar to uxx_replace_bits() for in-memory bit field
modification, but with the added advantage of strict parameter type
checking at compile time.
Previous discussions (in patch series v3) among Russell, Marc, and Yury
focused on whether there is any added advantage of using FIELD_MODIFY()
(possibility of size overflow checking) for the specific cases being
modified by the patch series inside arm64. For the one case where enum
was being used, it was originally thought that there may be a
possibility of size overflow due to 32bit size used by the compiler for
the enum. However it was identified that the kernel's compiler already
uses 64bit types for enum if the number of bits used with the enum is
more than 32 (the code in this case used more than 32 bits).So, it seems
that there is no additional benefit of using FIELD_MODIFY() in the
specific cases in arm64/.
Please note that the current ARM64 code using FIELD_PREP() also supports
strict parameter checking at compile time. Both FIELD_PREP and
FIELD_MODIFY utilize `__BF_FIELD_CHECK()` to support this compile-time
parameter validation.
© 2016 - 2025 Red Hat, Inc.