On Thu, Mar 07, 2024 at 02:53:43PM -0500, Mathieu Desnoyers wrote:
> On 2024-03-07 14:47, Paul E. McKenney wrote:
> > On Thu, Mar 07, 2024 at 08:53:05AM -0500, Mathieu Desnoyers wrote:
> > > On 2024-03-06 22:37, Paul E. McKenney wrote:
> > > > On Wed, Mar 06, 2024 at 10:06:21PM -0500, Mathieu
On Thu, Mar 07, 2024 at 02:09:00PM -0800, Linus Torvalds wrote:
> On Thu, 7 Mar 2024 at 13:40, Julia Lawall wrote:
> >
> > I tried the following:
> >
> > @@
> > expression x;
> > @@
> >
> > *WRITE_ONCE(x,<+...READ_ONCE(x)...+>)
> >
> > This gave a number of results, shown below. Let me know if
On Thu, 7 Mar 2024 at 13:40, Julia Lawall wrote:
>
> I tried the following:
>
> @@
> expression x;
> @@
>
> *WRITE_ONCE(x,<+...READ_ONCE(x)...+>)
>
> This gave a number of results, shown below. Let me know if some of them
> are undesirable.
Well, all the ones you list do look like garbage.
On Thu, 7 Mar 2024, Paul E. McKenney wrote:
> On Thu, Mar 07, 2024 at 12:00:44PM -0800, Linus Torvalds wrote:
> > On Thu, 7 Mar 2024 at 11:47, Paul E. McKenney wrote:
> > >
> > > > - The per-thread counter (Thread-Local Storage) incremented by a single
> > > > thread, read by various
On Thu, Mar 07, 2024 at 12:00:44PM -0800, Linus Torvalds wrote:
> On Thu, 7 Mar 2024 at 11:47, Paul E. McKenney wrote:
> >
> > > - The per-thread counter (Thread-Local Storage) incremented by a single
> > > thread, read by various threads concurrently, is a good target
> > > for
On Thu, 7 Mar 2024 at 11:47, Paul E. McKenney wrote:
>
> > - The per-thread counter (Thread-Local Storage) incremented by a single
> > thread, read by various threads concurrently, is a good target
> > for WRITE_ONCE()/READ_ONCE() pairing. This is actually what we do in
> > various liburcu
On 2024-03-07 14:47, Paul E. McKenney wrote:
On Thu, Mar 07, 2024 at 08:53:05AM -0500, Mathieu Desnoyers wrote:
On 2024-03-06 22:37, Paul E. McKenney wrote:
On Wed, Mar 06, 2024 at 10:06:21PM -0500, Mathieu Desnoyers wrote:
[...]
As far as the WRITE_ONCE(x, READ_ONCE(x) + 1) pattern
is
On Thu, Mar 07, 2024 at 08:53:05AM -0500, Mathieu Desnoyers wrote:
> On 2024-03-06 22:37, Paul E. McKenney wrote:
> > On Wed, Mar 06, 2024 at 10:06:21PM -0500, Mathieu Desnoyers wrote:
> [...]
> >
> > > As far as the WRITE_ONCE(x, READ_ONCE(x) + 1) pattern
> > > is concerned, the only valid
On Thu, Mar 07, 2024 at 12:44:39AM -0500, Joel Fernandes wrote:
>
>
> On 3/6/2024 10:37 PM, Paul E. McKenney wrote:
> > On Wed, Mar 06, 2024 at 10:06:21PM -0500, Mathieu Desnoyers wrote:
> >> On 2024-03-06 21:43, Linus Torvalds wrote:
> >> [...]
> >>>
> >>> Honestly, this all makes me think that
On Thu, 7 Mar 2024 08:20:59 -0500
Steven Rostedt wrote:
> When a write happens, it looks to see if the smallest watermark is hit,
> if so, calls irqwork to wakeup all the waiters.
>
> The waiters will wake up, check to see if a signal is pending or if the
> ring buffer has hit the watermark the
On 2024-03-06 22:37, Paul E. McKenney wrote:
On Wed, Mar 06, 2024 at 10:06:21PM -0500, Mathieu Desnoyers wrote:
[...]
As far as the WRITE_ONCE(x, READ_ONCE(x) + 1) pattern
is concerned, the only valid use-case I can think of is
split counters or RCU implementations where there is a
single
On Wed, 6 Mar 2024 12:06:00 -0800
Linus Torvalds wrote:
> On Wed, 6 Mar 2024 at 11:45, Steven Rostedt wrote:
> >
> > Here's the back story. I received the following patch:
> >
> >
> > https://lore.kernel.org/all/tencent_ba1473492bc618b473864561ea3ab1418...@qq.com/
> >
> > I didn't like it.
On 3/6/2024 10:37 PM, Paul E. McKenney wrote:
> On Wed, Mar 06, 2024 at 10:06:21PM -0500, Mathieu Desnoyers wrote:
>> On 2024-03-06 21:43, Linus Torvalds wrote:
>> [...]
>>>
>>> Honestly, this all makes me think that we'd be *much* better off
>>> showing the real "handoff" with
On Wed, Mar 06, 2024 at 10:06:21PM -0500, Mathieu Desnoyers wrote:
> On 2024-03-06 21:43, Linus Torvalds wrote:
> [...]
> >
> > Honestly, this all makes me think that we'd be *much* better off
> > showing the real "handoff" with smp_store_release() and
> > smp_load_acquire().
>
> We've done
On Wed, Mar 06, 2024 at 06:49:38PM -0800, Linus Torvalds wrote:
> On Wed, 6 Mar 2024 at 18:43, Linus Torvalds
> wrote:
> >
> > I dunno.
>
> Oh, and just looking at that patch, I still think the code is confused.
>
> On the reading side, we have:
>
> pipe_count =
On 2024-03-06 21:43, Linus Torvalds wrote:
[...]
Honestly, this all makes me think that we'd be *much* better off
showing the real "handoff" with smp_store_release() and
smp_load_acquire().
We've done something similar in liburcu (userspace code) to allow
Thread Sanitizer to understand the
On Wed, Mar 06, 2024 at 06:43:42PM -0800, Linus Torvalds wrote:
> On Wed, 6 Mar 2024 at 18:29, Paul E. McKenney wrote:
> >
> > TL;DR: Those ->rtort_pipe_count increments cannot run concurrently
> > with each other or any other update of that field, so that update-side
> > READ_ONCE() call is
On Wed, 6 Mar 2024 at 18:43, Linus Torvalds
wrote:
>
> I dunno.
Oh, and just looking at that patch, I still think the code is confused.
On the reading side, we have:
pipe_count = smp_load_acquire(>rtort_pipe_count);
if (pipe_count > RCU_TORTURE_PIPE_LEN) {
/* Should not happen,
On Wed, 6 Mar 2024 at 18:29, Paul E. McKenney wrote:
>
> TL;DR: Those ->rtort_pipe_count increments cannot run concurrently
> with each other or any other update of that field, so that update-side
> READ_ONCE() call is unnecessary and the update-side plain C-language
> read is OK. The
On Wed, Mar 06, 2024 at 11:46:10AM -0800, Linus Torvalds wrote:
> On Wed, 6 Mar 2024 at 11:27, Steven Rostedt wrote:
> >
> > Note this has nothing to do with tracing. This thread is in RCU. I just
> > happen to receive the same patch "fix" for my code.
>
> Ok, googling for rtort_pipe_count, I
On Wed, 6 Mar 2024 at 11:46, Linus Torvalds
wrote:
>
> That 'rtort_pipe_count' should be an atomic_t, and the "add one and
> return the old value" should be an "atomic_inc_return()-1" (the "-1"
> is because "inc_return" returns the *new* value).
Bah. I am lost in a twisty maze of operations, all
On Wed, 6 Mar 2024 at 11:45, Steven Rostedt wrote:
>
> Here's the back story. I received the following patch:
>
>
> https://lore.kernel.org/all/tencent_ba1473492bc618b473864561ea3ab1418...@qq.com/
>
> I didn't like it. My reply was:
>
> > - rbwork->wait_index++;
> > +
On Wed, 6 Mar 2024 at 11:27, Steven Rostedt wrote:
>
> Note this has nothing to do with tracing. This thread is in RCU. I just
> happen to receive the same patch "fix" for my code.
Ok, googling for rtort_pipe_count, I can only state that that code is
complete garbage.
And no amount of
On Wed, 6 Mar 2024 11:27:04 -0800
Linus Torvalds wrote:
> On Wed, 6 Mar 2024 at 11:01, Linus Torvalds
> wrote:
> >
> > In some individual tracing C file where it has a comment above it how
> > it's braindamaged and unsafe and talking about why it's ok in that
> > particular context? Go wild.
On Wed, 6 Mar 2024 11:01:55 -0800
Linus Torvalds wrote:
> On Wed, 6 Mar 2024 at 10:53, Steven Rostedt wrote:
> >
> > Now, are you OK with an addition of ADD_ONCE() and/or INC_ONCE()? So that we
> > don't have to look at:
> >
> > WRITE_ONCE(a, READ_ONCE(a) + 1);
> >
> > ?
>
> In a
On Wed, 6 Mar 2024 at 11:01, Linus Torvalds
wrote:
>
> In some individual tracing C file where it has a comment above it how
> it's braindamaged and unsafe and talking about why it's ok in that
> particular context? Go wild.
Actually, I take that back.
Even in a random C file, the naming makes
On Wed, 6 Mar 2024 at 10:53, Steven Rostedt wrote:
>
> Now, are you OK with an addition of ADD_ONCE() and/or INC_ONCE()? So that we
> don't have to look at:
>
> WRITE_ONCE(a, READ_ONCE(a) + 1);
>
> ?
In a generic header file under include/linux/?
Absolutely not. The above is a
On Wed, 6 Mar 2024 10:43:25 -0800
Linus Torvalds wrote:
Thanks for the history lesson ;-)
> So yes, READ_ONCE/WRITE_ONCE do control "tearing", but realistically,
> it was always only about the "complex values" kind of tearing that the
> old ACCESS_ONCE() model silently and incorrectly allowed.
On Wed, 6 Mar 2024 at 09:59, Steven Rostedt wrote:
>
> IIRC, the original purpose of READ_ONCE() and WRITE_ONCE() was to make sure
> that the compiler only reads or writes the variable "once". Hence the name.
> That way after a load, you don't need to worry that the content of the
> variable you
On Wed, 6 Mar 2024 10:09:48 -0800
"Paul E. McKenney" wrote:
> > Perhaps we need a way to annotate them, like we have with __rcu. "__shared"?
> >
> > Then all accesses to that variable must be wrapped with a READ_ONCE() or
> > WRITE_ONCE()? I mean, if this can cause legitimate bugs, we should
On Wed, Mar 06, 2024 at 01:01:03PM -0500, Steven Rostedt wrote:
> On Wed, 6 Mar 2024 09:36:16 -0800
> "Paul E. McKenney" wrote:
>
> > > If we take the policy of handling a compiler that can tear reads and
> > > writes
> > > of any size word, then we should have proper macros to handle it.
> >
On Wed, 6 Mar 2024 09:36:16 -0800
"Paul E. McKenney" wrote:
> > If we take the policy of handling a compiler that can tear reads and writes
> > of any size word, then we should have proper macros to handle it.
>
> Those are in fact READ_ONCE() and WRITE_ONCE() when given machine-word
>
On Wed, Mar 06, 2024 at 10:37:19AM -0500, Steven Rostedt wrote:
> On Tue, 5 Mar 2024 14:24:20 +0800
> linke li wrote:
>
> > > Anyway, a slightly related/different question:
> > >
> > > Should that:
> > > WRITE_ONCE(rp->rtort_pipe_count, rp->rtort_pipe_count + 1);
> > >
> > > Be:
> > >
On Tue, 5 Mar 2024 14:24:20 +0800
linke li wrote:
> > Anyway, a slightly related/different question:
> >
> > Should that:
> > WRITE_ONCE(rp->rtort_pipe_count, rp->rtort_pipe_count + 1);
> >
> > Be:
> > WRITE_ONCE(rp->rtort_pipe_count, READ_ONCE(rp->rtort_pipe_count) + 1);
> >
> > ?
>
>
> Anyway, a slightly related/different question:
>
> Should that:
> WRITE_ONCE(rp->rtort_pipe_count, rp->rtort_pipe_count + 1);
>
> Be:
> WRITE_ONCE(rp->rtort_pipe_count, READ_ONCE(rp->rtort_pipe_count) + 1);
>
> ?
Hi, Joel. Sorry, I am not very sure about this. I do a little research on
it.
Thank you both. This looks good to me. I will send a new patch.
> 2024年3月5日 04:47，Paul E. McKenney 写道：
>
> On Mon, Mar 04, 2024 at 03:13:10PM -0500, Joel Fernandes wrote:
>>
>>
>> On 3/4/2024 2:44 PM, Paul E. McKenney wrote:
>>> On Mon, Mar 04, 2024 at 02:10:09PM -0500, Joel Fernandes wrote:
On Mon, Mar 04, 2024 at 03:13:10PM -0500, Joel Fernandes wrote:
>
>
> On 3/4/2024 2:44 PM, Paul E. McKenney wrote:
> > On Mon, Mar 04, 2024 at 02:10:09PM -0500, Joel Fernandes wrote:
> >>
> >>
> >> On 3/4/2024 12:14 PM, Paul E. McKenney wrote:
> >>> On Mon, Mar 04, 2024 at 11:19:21AM -0500, Joel
On 3/4/2024 2:44 PM, Paul E. McKenney wrote:
> On Mon, Mar 04, 2024 at 02:10:09PM -0500, Joel Fernandes wrote:
>>
>>
>> On 3/4/2024 12:14 PM, Paul E. McKenney wrote:
>>> On Mon, Mar 04, 2024 at 11:19:21AM -0500, Joel Fernandes wrote:
On 3/4/2024 5:54 AM, linke li wrote:
>
On Mon, Mar 04, 2024 at 02:10:09PM -0500, Joel Fernandes wrote:
>
>
> On 3/4/2024 12:14 PM, Paul E. McKenney wrote:
> > On Mon, Mar 04, 2024 at 11:19:21AM -0500, Joel Fernandes wrote:
> >>
> >>
> >> On 3/4/2024 5:54 AM, linke li wrote:
> >>> Some changes are done to fix a data race in commit
On 3/4/2024 12:14 PM, Paul E. McKenney wrote:
> On Mon, Mar 04, 2024 at 11:19:21AM -0500, Joel Fernandes wrote:
>>
>>
>> On 3/4/2024 5:54 AM, linke li wrote:
>>> Some changes are done to fix a data race in commit 202489101f2e
>>> ("rcutorture: Fix rcu_torture_one_read()/rcu_torture_writer()
On Mon, Mar 04, 2024 at 11:19:21AM -0500, Joel Fernandes wrote:
>
>
> On 3/4/2024 5:54 AM, linke li wrote:
> > Some changes are done to fix a data race in commit 202489101f2e
> > ("rcutorture: Fix rcu_torture_one_read()/rcu_torture_writer() data race")
> >
> > {
> > int i;
> >
> > - i
On 3/4/2024 5:54 AM, linke li wrote:
> Some changes are done to fix a data race in commit 202489101f2e ("rcutorture:
> Fix rcu_torture_one_read()/rcu_torture_writer() data race")
>
> {
> int i;
>
> - i = rp->rtort_pipe_count;
> + i = READ_ONCE(rp->rtort_pipe_count);
> if
Some changes are done to fix a data race in commit 202489101f2e ("rcutorture:
Fix rcu_torture_one_read()/rcu_torture_writer() data race")
{
int i;
- i = rp->rtort_pipe_count;
+ i = READ_ONCE(rp->rtort_pipe_count);
if (i > RCU_TORTURE_PIPE_LEN)
i =
43 matches
Mail list logo
