[dpdk-dev] [PATCH] checkpatch: enable volatile warning

[dpdk-dev] [PATCH] checkpatch: enable volatile warning

[Dharmik Thakkar]

Enable volatile considered harmful warning since use of volatile
is suspect.

Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 devtools/checkpatches.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 78a408ef9823..92f1984454b0 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -29,7 +29,7 @@ options="$options --max-line-length=$length"
 options="$options --show-types"
 options="$options --ignore=LINUX_VERSION_CODE,ENOSYS,\
 FILE_PATH_CHANGES,MAINTAINERS_STYLE,SPDX_LICENSE_TAG,\
-VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
+PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
 PREFER_KERNEL_TYPES,PREFER_FALLTHROUGH,BIT_MACRO,CONST_STRUCT,\
 SPLIT_STRING,LONG_LINE_STRING,C99_COMMENT_TOLERANCE,\
 LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
-- 
2.17.1

Re: [dpdk-dev] [PATCH] checkpatch: enable volatile warning

[Thomas Monjalon]

10/03/2021 12:04, Dharmik Thakkar:
> Enable volatile considered harmful warning since use of volatile
> is suspect.
> 
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>

git grep -w volatile | wc -l
	1796

How much is it suspect?

Re: [dpdk-dev] [PATCH] checkpatch: enable volatile warning

[Stephen Hemminger]

On Wed, 10 Mar 2021 12:10:01 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 10/03/2021 12:04, Dharmik Thakkar:
> > Enable volatile considered harmful warning since use of volatile
> > is suspect.
> > 
> > Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>  
> 
> git grep -w volatile | wc -l
> 	1796
> 
> How much is it suspect?
> 
> 

Many seem to be unsafe.
   testpmd: uses flags values in unsafe manner
      it also uses volatile when accessing hardware registers

   test-alarm is expecting that alarm() is a signal
       (it is not)

   test-atomic is ok
   test-barrier is doing barriers and not using __atomic

   drivers use volatile to mark hardware registers.

It is still true volatile is not enough are not weak memory model.

Re: [dpdk-dev] [PATCH] checkpatch: enable volatile warning

[Dharmik Thakkar]

Hi,

Apologies for the delayed response!

> On Mar 10, 2021, at 9:10 AM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> On Wed, 10 Mar 2021 12:10:01 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
>> 10/03/2021 12:04, Dharmik Thakkar:
>>> Enable volatile considered harmful warning since use of volatile
>>> is suspect.
>>> 
>>> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>  
>> 
>> git grep -w volatile | wc -l
>> 	1796
>> 
>> How much is it suspect?
>> 
>> 
> 
> Many seem to be unsafe.
>   testpmd: uses flags values in unsafe manner
>      it also uses volatile when accessing hardware registers
> 
>   test-alarm is expecting that alarm() is a signal
>       (it is not)
> 
>   test-atomic is ok
>   test-barrier is doing barriers and not using __atomic
> 
>   drivers use volatile to mark hardware registers.
> 
> It is still true volatile is not enough are not weak memory model.

Agree, and this warning should help avoid such unsafe / incorrect use of volatile for future patches.