search for: smp_barriers

...? } > ?# check for memory barriers without a comment. > - if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { > + > + my $barriers = qr{ > + mb| > + rmb| > + wmb| > + read_barrier_depends > + }x; > + my $smp_barriers = qr{ > + store_release| > + load_acquire| > + store_mb| > + ($barriers) > + }x; If I use a variable called $smp_barriers, I'd expect it to actually be the smp_barriers, not to have to prefix it with smp_ before using it. my $smp_barriers = qr{ smp_store_release|...

...? } > ?# check for memory barriers without a comment. > - if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { > + > + my $barriers = qr{ > + mb| > + rmb| > + wmb| > + read_barrier_depends > + }x; > + my $smp_barriers = qr{ > + store_release| > + load_acquire| > + store_mb| > + ($barriers) > + }x; If I use a variable called $smp_barriers, I'd expect it to actually be the smp_barriers, not to have to prefix it with smp_ before using it. my $smp_barriers = qr{ smp_store_release|...

As part of memory barrier cleanup, this patchset extends checkpatch to make it easier to stop incorrect memory barrier usage. This applies on top of my series arch: barrier cleanup + barriers for virt and will be included in the next version of the series. Michael S. Tsirkin (3): checkpatch.pl: add missing memory barriers checkpatch: check for __smp outside barrier.h checkpatch: add virt

As part of memory barrier cleanup, this patchset extends checkpatch to make it easier to stop incorrect memory barrier usage. This applies on top of my series arch: barrier cleanup + barriers for virt and will be included in the next version of the series. Michael S. Tsirkin (3): checkpatch.pl: add missing memory barriers checkpatch: check for __smp outside barrier.h checkpatch: add virt

...} > ? } > ?# check for memory barriers without a comment. > - if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { > + > + my @barriers = ('mb', 'rmb', 'wmb', 'read_barrier_depends'); > + my @smp_barriers = ('smp_store_release', 'smp_load_acquire', 'smp_store_mb'); > + > + @smp_barriers = (@smp_barriers, map {"smp_" . $_} @barriers); I think using map, which so far checkpatch doesn't use, makes smp_barriers harder to understand and it'd be better to...

...} > ? } > ?# check for memory barriers without a comment. > - if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { > + > + my @barriers = ('mb', 'rmb', 'wmb', 'read_barrier_depends'); > + my @smp_barriers = ('smp_store_release', 'smp_load_acquire', 'smp_store_mb'); > + > + @smp_barriers = (@smp_barriers, map {"smp_" . $_} @barriers); I think using map, which so far checkpatch doesn't use, makes smp_barriers harder to understand and it'd be better to...

As part of memory barrier cleanup, this patchset extends checkpatch to make it easier to stop incorrect memory barrier usage. This applies on top of my series arch: barrier cleanup + barriers for virt and will be included in the next version of the series. Changes from v2: catch optional\s* before () in barriers rewrite using qr{} instead of map Michael S. Tsirkin (3): checkpatch.pl: add

As part of memory barrier cleanup, this patchset extends checkpatch to make it easier to stop incorrect memory barrier usage. This applies on top of my series arch: barrier cleanup + barriers for virt and will be included in the next version of the series. Changes from v2: catch optional\s* before () in barriers rewrite using qr{} instead of map Michael S. Tsirkin (3): checkpatch.pl: add

...barriers without a comment. > > > - if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { > > > + > > > + my @barriers = ('mb', 'rmb', 'wmb', 'read_barrier_depends'); > > > + my @smp_barriers = ('smp_store_release', 'smp_load_acquire', 'smp_store_mb'); > > > + > > > + @smp_barriers = (@smp_barriers, map {"smp_" . $_} @barriers); > > > > I think using map, which so far checkpatch doesn't use, > > makes smp_barrie...

...barriers without a comment. > > > - if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { > > > + > > > + my @barriers = ('mb', 'rmb', 'wmb', 'read_barrier_depends'); > > > + my @smp_barriers = ('smp_store_release', 'smp_load_acquire', 'smp_store_mb'); > > > + > > > + @smp_barriers = (@smp_barriers, map {"smp_" . $_} @barriers); > > > > I think using map, which so far checkpatch doesn't use, > > makes smp_barrie...

...n <mst at redhat.com> --- scripts/checkpatch.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e3f9ad9..5fb6ef7 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5121,7 +5121,8 @@ sub process { my @smp_barriers = ('smp_store_release', 'smp_load_acquire', 'smp_store_mb'); @smp_barriers = (@smp_barriers, map {"smp_" . $_} @barriers); - my $all_barriers = join('|', (@barriers, @smp_barriers)); + my @virt_barriers = map {my $l = $_; $l =~ s/smp_/virt_/; $l} @sm...

...?# check for memory barriers without a comment. > > - if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { > > + > > + my @barriers = ('mb', 'rmb', 'wmb', 'read_barrier_depends'); > > + my @smp_barriers = ('smp_store_release', 'smp_load_acquire', 'smp_store_mb'); > > + > > + @smp_barriers = (@smp_barriers, map {"smp_" . $_} @barriers); > > I think using map, which so far checkpatch doesn't use, > makes smp_barriers harder to understand a...

...,7 +5116,14 @@ sub process { } } # check for memory barriers without a comment. - if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { + + my @barriers = ('mb', 'rmb', 'wmb', 'read_barrier_depends'); + my @smp_barriers = ('smp_store_release', 'smp_load_acquire', 'smp_store_mb'); + + @smp_barriers = (@smp_barriers, map {"smp_" . $_} @barriers); + my $all_barriers = join('|', (@barriers, @smp_barriers)); + + if ($line =~ /\b($all_barriers)\(/) { if (!ctx_has_comment(...

...01-10 at 13:56 +0200, Michael S. Tsirkin wrote: > > SMP-only barriers were missing in checkpatch.pl > > > > Refactor code slightly to make adding more variants easier. > [] > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > If I use a variable called $smp_barriers, I'd expect > it to actually be the smp_barriers, not to have to > prefix it with smp_ before using it. > > my $smp_barriers = qr{ > smp_store_release| > smp_load_acquire| > smp_store_mb| > smp_read_barrier_depends That's missing (?:barriers) too. bt...

...gt; > Also, ideally this would be longest match first or use \b > after the matches so that $all_barriers could work > successfully without a following \s*\( > > my $all_barriers = qr{ > (?:smp|virt)_(?:barrier_stems)| > $barriers) > }x; > > or maybe add separate $smp_barriers and $virt_barriers > > <shrug> it doesn't matter much in any case OK just to clarify - are you OK with merging the patch as is? Refactorings can come as patches on top if required. -- MST

...gt; > Also, ideally this would be longest match first or use \b > after the matches so that $all_barriers could work > successfully without a following \s*\( > > my $all_barriers = qr{ > (?:smp|virt)_(?:barrier_stems)| > $barriers) > }x; > > or maybe add separate $smp_barriers and $virt_barriers > > <shrug> it doesn't matter much in any case OK just to clarify - are you OK with merging the patch as is? Refactorings can come as patches on top if required. -- MST

...atch.pl @@ -5116,7 +5116,25 @@ sub process { } } # check for memory barriers without a comment. - if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { + + my $barriers = qr{ + mb| + rmb| + wmb| + read_barrier_depends + }x; + my $smp_barriers = qr{ + store_release| + load_acquire| + store_mb| + ($barriers) + }x; + my $all_barriers = qr{ + $barriers| + smp_($smp_barriers) + }x; + + if ($line =~ /\b($all_barriers)\s*\(/) { if (!ctx_has_comment($first_line, $linenr)) { WARN("MEMORY_BARRIER", &qu...

Hi Michael, On Mon, Jan 11, 2016 at 6:31 AM, Michael S. Tsirkin <mst at redhat.com> wrote: > Add virt_ barriers to list of barriers to check for > presence of a comment. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > scripts/checkpatch.pl | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/scripts/checkpatch.pl

Hi Michael, On Mon, Jan 11, 2016 at 6:31 AM, Michael S. Tsirkin <mst at redhat.com> wrote: > Add virt_ barriers to list of barriers to check for > presence of a comment. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > --- > scripts/checkpatch.pl | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/scripts/checkpatch.pl

...hanges from v3: rename smp_barrier_stems to barrier_stems as suggested by Julian Calaby. add (?: ... ) around a variable in regexp, in case we change the value later so that it matters. changes from v2: address comments by Joe Perches: use (?: ... ) to avoid unnecessary capture groups rename smp_barriers to smp_barrier_stems for clarity add barriers before/after atomic Changes from v1: catch optional\s* before () in barriers rewrite using qr{} instead of map Michael S. Tsirkin (3): checkpatch.pl: add missing memory barriers checkpatch: check for __smp outside barrier.h checkpatch: add vir...