Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1260040 > unrolled thread

Re: [PATCH] ipc/msg: Implement lockless pipelined wakeups

Started byDavidlohr Bueso <dave@stgolabs.net>
First post2015-10-31 20:00 +0100
Last post2015-11-03 15:20 +0100
Articles 4 — 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.


Contents

  Re: [PATCH] ipc/msg: Implement lockless pipelined wakeups Davidlohr Bueso <dave@stgolabs.net> - 2015-10-31 20:00 +0100
    Re: [PATCH] ipc/msg: Implement lockless pipelined wakeups Davidlohr Bueso <dave@stgolabs.net> - 2015-10-31 20:10 +0100
      Re: [PATCH] ipc/msg: Implement lockless pipelined wakeups Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2015-11-03 15:20 +0100
    Re: [PATCH] ipc/msg: Implement lockless pipelined wakeups Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2015-11-03 15:20 +0100

#1260040 — Re: [PATCH] ipc/msg: Implement lockless pipelined wakeups

FromDavidlohr Bueso <dave@stgolabs.net>
Date2015-10-31 20:00 +0100
SubjectRe: [PATCH] ipc/msg: Implement lockless pipelined wakeups
Message-ID<qpJxE-6qG-11@gated-at.bofh.it>
On Sat, 31 Oct 2015, George Spelvin wrote:

>> Don't we need to keep that NULL init? I might be missing something.
>
>I wondered the same thing, but on reading it, the cleanup is that he's
>gotten rid of the need for the entire thing.  Previously, there was a
>mechanism for detecting "wakeup not quite finished" that used a NULL
>value, but it's no longer needed.
>
>The resultant busy-waiting on the part of the woken-up task was the
>entire problem this patch aims to fix.  So it gets rid of a whole lot
>of code and barriers.  And, as you noticed, the comments explaining them.
>
>As the old code explained, the issue is that a task may exit as
>soon as r_msg is set, so the wakeup procedure has to be:
>- Ensure r_msg is set to NULL (special-case flag)
>- Do the wake up
>- Set r_msg to the final value
>
>The woken-up task has to spin as long as r_msg is NULL.  Ick.

I agree this is a nice cleanup. It is similar to what we did
with posix mqueues, but this one gets rid of even more code afaict.

>However, a wake_q keeps a reference to a task, so exiting is
>not a danger.  As long as wake_q_add precedes setting r_msg,
>all is well.

Yes, and this confirms that we still rely on the implicit barrier
from the cmpxchg as tglx mentioned. As such, we also need to keep
the pairing when reading 'r_msg' in do_msgrcv(), instead of dropping
the comments.

Thanks,
Davidlohr
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1260042

FromDavidlohr Bueso <dave@stgolabs.net>
Date2015-10-31 20:10 +0100
Message-ID<qpJHj-6JK-1@gated-at.bofh.it>
In reply to#1260040
On Sat, 31 Oct 2015, Bueso wrote:

>Yes, and this confirms that we still rely on the implicit barrier
>from the cmpxchg as tglx mentioned. As such, we also need to keep
>the pairing when reading 'r_msg' in do_msgrcv(), instead of dropping
>the comments.

Hmm having r_msg as volatile seems even less needed now, we should
drop it. I imagine it was there initially for the busy-wait on the variable
becoming non-nil, but we had cpu_relax _anyway_, so ...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261525

FromSebastian Andrzej Siewior <bigeasy@linutronix.de>
Date2015-11-03 15:20 +0100
Message-ID<qqKBj-3MG-3@gated-at.bofh.it>
In reply to#1260042
On 10/31/2015 08:06 PM, Davidlohr Bueso wrote:
> On Sat, 31 Oct 2015, Bueso wrote:
> 
>> Yes, and this confirms that we still rely on the implicit barrier
>> from the cmpxchg as tglx mentioned. As such, we also need to keep
>> the pairing when reading 'r_msg' in do_msgrcv(), instead of dropping
>> the comments.
> 
> Hmm having r_msg as volatile seems even less needed now, we should
> drop it. I imagine it was there initially for the busy-wait on the variable
> becoming non-nil, but we had cpu_relax _anyway_, so ...

Didn't noticed that. So that volatile part and the casts are gone, too.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261524

FromSebastian Andrzej Siewior <bigeasy@linutronix.de>
Date2015-11-03 15:20 +0100
Message-ID<qqKBj-3MG-1@gated-at.bofh.it>
In reply to#1260040
On 10/31/2015 07:54 PM, Davidlohr Bueso wrote:
>> However, a wake_q keeps a reference to a task, so exiting is
>> not a danger.  As long as wake_q_add precedes setting r_msg,
>> all is well.
> 
> Yes, and this confirms that we still rely on the implicit barrier
> from the cmpxchg as tglx mentioned. As such, we also need to keep
> the pairing when reading 'r_msg' in do_msgrcv(), instead of dropping
> the comments.

Okay, I took the memory-barrier comment from your mqueue patch and
adjusted.

> 
> Thanks,
> Davidlohr

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web