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

Groups > linux.kernel > #1240628 > unrolled thread

Re: [PATCH] Documentation: Remove misleading examples of the barriers in wake_*()

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] Documentation: Remove misleading examples of the  barriers in wake_*() Peter Zijlstra <peterz@infradead.org> - 2015-10-06 18:10 +0200
    Re: [PATCH] Documentation: Remove misleading examples of the  barriers in wake_*() Boqun Feng <boqun.feng@gmail.com> - 2015-10-11 17:30 +0200
      Re: [PATCH] Documentation: Remove misleading examples of the  barriers in wake_*() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-10-12 02:50 +0200
        Re: [PATCH] Documentation: Remove misleading examples of the  barriers in wake_*() Boqun Feng <boqun.feng@gmail.com> - 2015-10-12 11:10 +0200
          Re: [PATCH] Documentation: Remove misleading examples of the  barriers in wake_*() Peter Zijlstra <peterz@infradead.org> - 2015-10-12 14:00 +0200
            Re: [PATCH] Documentation: Remove misleading examples of the  barriers in wake_*() Boqun Feng <boqun.feng@gmail.com> - 2015-10-12 15:10 +0200
              Re: [PATCH] Documentation: Remove misleading examples of the  barriers in wake_*() Peter Zijlstra <peterz@infradead.org> - 2015-10-12 18:30 +0200

#1240628 — Re: [PATCH] Documentation: Remove misleading examples of the barriers in wake_*()

On Thu, Sep 24, 2015 at 09:21:22PM +0800, Boqun Feng wrote:
> > Included in it are some of the details on this subject, because a wakeup
> > has two prior states that are of importance, the tasks own prior state
> > and the wakeup state, both should be considered in the 'program order'
> > flow.
> > 
> 
> Great and very helpful ;-)
> 
> > So maybe we can reduce the description in memory-barriers to this
> > 'split' program order guarantee, where a woken task must observe both
> > its own prior state and its wakee state.
>                               ^^^^^
> I think you mean "waker" here, right?

Yes.

> And the waker is not necessarily the same task who set the @cond to
> true, right? 

It should be.

> If so, I feel like it's really hard to *use* this 'split'
> program order guarantee in other places than sleep/wakeup itself. Could
> you give an example? Thank you.

It was not meant to be used in any other scenario; the 'split' PO really
is part of the whole sleep/wakeup. It does not apply to anything else.
--
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]

#1244149

[Multipart message — attachments visible in raw view] — view raw

On Tue, Oct 06, 2015 at 06:06:50PM +0200, Peter Zijlstra wrote:
> On Thu, Sep 24, 2015 at 09:21:22PM +0800, Boqun Feng wrote:
> > > Included in it are some of the details on this subject, because a wakeup
> > > has two prior states that are of importance, the tasks own prior state
> > > and the wakeup state, both should be considered in the 'program order'
> > > flow.
> > > 
> > 
> > Great and very helpful ;-)
> > 
> > > So maybe we can reduce the description in memory-barriers to this
> > > 'split' program order guarantee, where a woken task must observe both
> > > its own prior state and its wakee state.
> >                               ^^^^^
> > I think you mean "waker" here, right?
> 
> Yes.
> 
> > And the waker is not necessarily the same task who set the @cond to
> > true, right? 
> 
> It should be.
> 
> > If so, I feel like it's really hard to *use* this 'split'
> > program order guarantee in other places than sleep/wakeup itself. Could
> > you give an example? Thank you.
> 
> It was not meant to be used in any other scenario; the 'split' PO really
> is part of the whole sleep/wakeup. It does not apply to anything else.

Got it. So at this point, I think it's better to remove the entire
"Sleep and wake-up functions" section in memory-barriers.txt. Because
this order guarantee is not for other users except sleep/wakeup. Any
concern, Paul?

Regards,
Boqun

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

#1244318

On Sun, Oct 11, 2015 at 11:26:40PM +0800, Boqun Feng wrote:
> On Tue, Oct 06, 2015 at 06:06:50PM +0200, Peter Zijlstra wrote:
> > On Thu, Sep 24, 2015 at 09:21:22PM +0800, Boqun Feng wrote:
> > > > Included in it are some of the details on this subject, because a wakeup
> > > > has two prior states that are of importance, the tasks own prior state
> > > > and the wakeup state, both should be considered in the 'program order'
> > > > flow.
> > > > 
> > > 
> > > Great and very helpful ;-)
> > > 
> > > > So maybe we can reduce the description in memory-barriers to this
> > > > 'split' program order guarantee, where a woken task must observe both
> > > > its own prior state and its wakee state.
> > >                               ^^^^^
> > > I think you mean "waker" here, right?
> > 
> > Yes.
> > 
> > > And the waker is not necessarily the same task who set the @cond to
> > > true, right? 
> > 
> > It should be.
> > 
> > > If so, I feel like it's really hard to *use* this 'split'
> > > program order guarantee in other places than sleep/wakeup itself. Could
> > > you give an example? Thank you.
> > 
> > It was not meant to be used in any other scenario; the 'split' PO really
> > is part of the whole sleep/wakeup. It does not apply to anything else.
> 
> Got it. So at this point, I think it's better to remove the entire
> "Sleep and wake-up functions" section in memory-barriers.txt. Because
> this order guarantee is not for other users except sleep/wakeup. Any
> concern, Paul?

The concern I have with just removing it is that it is all too easy for
people to assume that they provide ordering.  So we should at least have
a section stating clearly that ordering is not guaranteed without help
from locks, release-acquire, explicit memory barriers, etc.

							Thanx, Paul

--
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]

#1244497

[Multipart message — attachments visible in raw view] — view raw

On Sun, Oct 11, 2015 at 05:40:44PM -0700, Paul E. McKenney wrote:
> On Sun, Oct 11, 2015 at 11:26:40PM +0800, Boqun Feng wrote:
> > On Tue, Oct 06, 2015 at 06:06:50PM +0200, Peter Zijlstra wrote:
> > > On Thu, Sep 24, 2015 at 09:21:22PM +0800, Boqun Feng wrote:
> > > > > Included in it are some of the details on this subject, because a wakeup
> > > > > has two prior states that are of importance, the tasks own prior state
> > > > > and the wakeup state, both should be considered in the 'program order'
> > > > > flow.
> > > > > 
> > > > 
> > > > Great and very helpful ;-)
> > > > 
> > > > > So maybe we can reduce the description in memory-barriers to this
> > > > > 'split' program order guarantee, where a woken task must observe both
> > > > > its own prior state and its wakee state.
> > > >                               ^^^^^
> > > > I think you mean "waker" here, right?
> > > 
> > > Yes.
> > > 
> > > > And the waker is not necessarily the same task who set the @cond to
> > > > true, right? 
> > > 
> > > It should be.
> > > 
> > > > If so, I feel like it's really hard to *use* this 'split'
> > > > program order guarantee in other places than sleep/wakeup itself. Could
> > > > you give an example? Thank you.
> > > 
> > > It was not meant to be used in any other scenario; the 'split' PO really
> > > is part of the whole sleep/wakeup. It does not apply to anything else.
> > 
> > Got it. So at this point, I think it's better to remove the entire
> > "Sleep and wake-up functions" section in memory-barriers.txt. Because
> > this order guarantee is not for other users except sleep/wakeup. Any
> > concern, Paul?
> 
> The concern I have with just removing it is that it is all too easy for
> people to assume that they provide ordering.  So we should at least have

Understood.

But, IMO, the position of this section is already misleading:

(*) Implicit kernel memory barriers.
     - Locking functions.
     - Interrupt disabling functions.
   ->- Sleep and wake-up functions.<-
     - Miscellaneous functions.

I read it as that sleep and wake-up functions provide some kernel memory
barriers which we can use *externally*(outside sleep/wakeup themselves).

So how about something like:

(*) Barriers only for internal use
     - Sleep and wake-up functions.

Regards,
Boqun

> a section stating clearly that ordering is not guaranteed without help
> from locks, release-acquire, explicit memory barriers, etc.
> 
> 							Thanx, Paul
> 

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

#1244624

On Mon, Oct 12, 2015 at 05:06:36PM +0800, Boqun Feng wrote:
> Understood.
> 
> But, IMO, the position of this section is already misleading:
> 
> (*) Implicit kernel memory barriers.
>      - Locking functions.
>      - Interrupt disabling functions.
>    ->- Sleep and wake-up functions.<-
>      - Miscellaneous functions.
> 
> I read it as that sleep and wake-up functions provide some kernel memory
> barriers which we can use *externally*(outside sleep/wakeup themselves).

I think it is useful to state that the primitives handle the ordering
between the waker and wakee wrt the 'blocking' state.

But I've not put much thought into wording. I wanted to finish process
order 'comment' patch first.
--
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]

#1244673

[Multipart message — attachments visible in raw view] — view raw

On Mon, Oct 12, 2015 at 01:54:38PM +0200, Peter Zijlstra wrote:
> On Mon, Oct 12, 2015 at 05:06:36PM +0800, Boqun Feng wrote:
> > Understood.
> > 
> > But, IMO, the position of this section is already misleading:
> > 
> > (*) Implicit kernel memory barriers.
> >      - Locking functions.
> >      - Interrupt disabling functions.
> >    ->- Sleep and wake-up functions.<-
> >      - Miscellaneous functions.
> > 
> > I read it as that sleep and wake-up functions provide some kernel memory
> > barriers which we can use *externally*(outside sleep/wakeup themselves).
> 
> I think it is useful to state that the primitives handle the ordering
> between the waker and wakee wrt the 'blocking' state.
> 

I agree that's useful, however, the 'blocking' state is something
internal for sleep and wakeup, right? Not sure whether the users of
wake_up() and wait_event() will care much about this or they need to
understand that detailedly to use wake_up() and wait_event() correctly.

I treat this part of memory-barriers.txt as an API document to describe
the implicit barriers in some primitives, which can be used *externally*
by someone, but anyway, that's just my own opinion ;-)

> But I've not put much thought into wording. I wanted to finish process
> order 'comment' patch first.

Of course. Actually your 'comment' patch is the reason why I think this
section may be removed.

Regards,
Boqun

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

#1244891

On Mon, Oct 12, 2015 at 09:09:24PM +0800, Boqun Feng wrote:
> On Mon, Oct 12, 2015 at 01:54:38PM +0200, Peter Zijlstra wrote:
> > On Mon, Oct 12, 2015 at 05:06:36PM +0800, Boqun Feng wrote:
> > > Understood.
> > > 
> > > But, IMO, the position of this section is already misleading:
> > > 
> > > (*) Implicit kernel memory barriers.
> > >      - Locking functions.
> > >      - Interrupt disabling functions.
> > >    ->- Sleep and wake-up functions.<-
> > >      - Miscellaneous functions.
> > > 
> > > I read it as that sleep and wake-up functions provide some kernel memory
> > > barriers which we can use *externally*(outside sleep/wakeup themselves).
> > 
> > I think it is useful to state that the primitives handle the ordering
> > between the waker and wakee wrt the 'blocking' state.
> > 
> 
> I agree that's useful, however, the 'blocking' state is something
> internal for sleep and wakeup, right? 

Not entirely; its also the @cond thing in wait queues. IE:

	for (;;)
		set_current_state(TASK_INTERRUPTIBLE);
		if (@cond)
			break;
		schedule();
	}
	__set_current_state(TASK_RUNNING);

vs.

	@cond = true;
	wake_up_process(p);


So we guarantee that 'p' will see the @cond stores IF it does the
wakeup. (If it does not, ie. 'p' wasn't sleeping, any guarantee is out
the window).

> Not sure whether the users of
> wake_up() and wait_event() will care much about this or they need to
> understand that detailedly to use wake_up() and wait_event() correctly.

I think its mostly natural; but it explains why you don't have to do:

	wait_event(wq, @cond);

vs.

	@cond = true;
	smp_wmb();
	wake_up(wq);

(or worse...)


> > But I've not put much thought into wording. I wanted to finish process
> > order 'comment' patch first.
> 
> Of course. Actually your 'comment' patch is the reason why I think this
> section may be removed.

Yes, that is another option, referring to the comment, once that's
sorted.
--
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