Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1305544 > unrolled thread
| Started by | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| First post | 2016-01-10 13:00 +0100 |
| Last post | 2016-01-10 20:20 +0100 |
| Articles | 5 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v2 1/3] checkpatch.pl: add missing memory barriers "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-10 13:00 +0100
Re: [PATCH v2 1/3] checkpatch.pl: add missing memory barriers Joe Perches <joe@perches.com> - 2016-01-10 16:10 +0100
Re: [PATCH v2 1/3] checkpatch.pl: add missing memory barriers Joe Perches <joe@perches.com> - 2016-01-10 16:20 +0100
Re: [PATCH v2 1/3] checkpatch.pl: add missing memory barriers "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-10 20:30 +0100
Re: [PATCH v2 1/3] checkpatch.pl: add missing memory barriers "Michael S. Tsirkin" <mst@redhat.com> - 2016-01-10 20:20 +0100
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2016-01-10 13:00 +0100 |
| Subject | [PATCH v2 1/3] checkpatch.pl: add missing memory barriers |
| Message-ID | <qPmP8-3H8-9@gated-at.bofh.it> |
SMP-only barriers were missing in checkpatch.pl
Refactor code slightly to make adding more variants easier.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
scripts/checkpatch.pl | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2b3c228..97b8b62 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.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",
"memory barrier without comment\n" . $herecurr);
--
MST
[toc] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2016-01-10 16:10 +0100 |
| Message-ID | <qPpN0-5Qp-15@gated-at.bofh.it> |
| In reply to | #1305544 |
On Sun, 2016-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
[]
> @@ -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;
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
}x;
or
my $smp_barriers = qr{
smp_(?:store_release|load_acquire|store_mb|read_barrier_depends)
}x;
> + my $all_barriers = qr{
> + $barriers|
> + smp_($smp_barriers)
> + }x;
And this shouldn't have a capture group.
my $all_barriers = qr{
$barriers|
$smp_barriers
}x;
> +
> + if ($line =~ /\b($all_barriers)\s*\(/) {
This doesn't need the capture group either (?:all_barriers)
> if (!ctx_has_comment($first_line, $linenr))
> {
> WARN("MEMORY_BARRIER",
> "memory barrier without
> comment\n" . $herecurr);
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2016-01-10 16:20 +0100 |
| Message-ID | <qPpWG-5U9-7@gated-at.bofh.it> |
| In reply to | #1305615 |
On Sun, 2016-01-10 at 07:07 -0800, Joe Perches wrote:
> On Sun, 2016-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.
btw: shouldn't this also have
smp_mb__(?:before|after)_atomic
?
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2016-01-10 20:30 +0100 |
| Message-ID | <qPtQC-8oH-19@gated-at.bofh.it> |
| In reply to | #1305617 |
On Sun, Jan 10, 2016 at 07:17:31AM -0800, Joe Perches wrote:
> On Sun, 2016-01-10 at 07:07 -0800, Joe Perches wrote:
> > On Sun, 2016-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.
My version has it but need to add ?: to avoid
a capture group.
> btw: shouldn't this also have
> smp_mb__(?:before|after)_atomic
> ?
Good catch, included in the next version.
--
MST
[toc] | [prev] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2016-01-10 20:20 +0100 |
| Message-ID | <qPtGW-8kK-11@gated-at.bofh.it> |
| In reply to | #1305615 |
On Sun, Jan 10, 2016 at 07:07:05AM -0800, Joe Perches wrote:
> On Sun, 2016-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
> []
> > @@ -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;
>
> 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
> }x;
>
> or
>
> my $smp_barriers = qr{
> smp_(?:store_release|load_acquire|store_mb|read_barrier_depends)
> }x;
>
Yes but virt barriers (added in patch 3) are same things but prefixed
with virt_. So we need the stems without smp_ prefix. If smp_barriers is
too confusing we'll just need to give them some other name.
How about:
my $smp_barrier_stems
?
> > + my $all_barriers = qr{
> > + $barriers|
> > + smp_($smp_barriers)
> > + }x;
>
> And this shouldn't have a capture group.
>
> my $all_barriers = qr{
> $barriers|
> $smp_barriers
> }x;
> > +
> > + if ($line =~ /\b($all_barriers)\s*\(/) {
>
> This doesn't need the capture group either (?:all_barriers)
>
> > if (!ctx_has_comment($first_line, $linenr))
> > {
> > WARN("MEMORY_BARRIER",
> > "memory barrier without
> > comment\n" . $herecurr);
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web