mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: "Jan H. Schönherr" <schnhrr@cs.tu-berlin.de>
Cc: Ingo Molnar <mingo@elte.hu>, Paul Turner <pjt@google.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Dipankar Sarma <dipankar@in.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFCv2 3/8] sched: Handle on_list ancestor in list_add_leaf_cfs_rq()
Date: Tue, 02 Aug 2011 15:50:37 +0200	[thread overview]
Message-ID: <1312293037.1147.181.camel@twins> (raw)
In-Reply-To: <1311793825-31933-4-git-send-email-schnhrr@cs.tu-berlin.de>

On Wed, 2011-07-27 at 21:10 +0200, Jan H. Schönherr wrote:
> +       /*
> +        * Carefully collect leaf-cfs entries.
> +        *
> +        * There are basically two cases:
> +        * 1. cfs_rq has not yet been on the leaf-list.
> +        * 2. cfs_rq has been deleted previously from the leaf_list.
> +        *
> +        * In case 2, we might still have concurrent readers.
> +        *
> +        * Therefore, the requirements of list_add_tail_nobackref() are
> +        * fulfilled:
> +        *
> +        * a) If there are concurrent readers, ->next must lead back to the
> +        *    list.
> +        *
> +        *    We can only have readers after case 2. After case 2, only case 2
> +        *    can follow. The next pointers of case 2 nodes always lead back to
> +        *    the list.
> +        *
> +        * b) If there are concurrent readers, ->next must not lead to any
> +        *    already collected node.
> +        *
> +        *    As we collect nodes always bottom-up, all already collected nodes
> +        *    must be below this node in the task group hierarchy.  The
> +        *    ordering constraint of the leaf list guarantees that next
> +        *    pointers only lead to nodes further up in the hierarchy (or to
> +        *    unrelated nodes). Neither deleting nodes nor the manipulations
> +        *    done here change that. Thus, we cannot reach an already collected
> +        *    node.
> +        *
> +        * c) If there are concurrent readers, they must already know this
> +        *    node.
> +        *
> +        *    If we have to add case 1 nodes, they are collected in the
> +        *    beginning and cannot be reached by readers until they are
> +        *    spliced. Furthermore, after they are spliced, we will not
> +        *    encounter more case 1 nodes higher up in the task group
> +        *    hierarchy. For this reason any reader on an earlier collected
> +        *    case 2 node must know all nodes that we collect later.
> +        */
> +       list_add_tail_nobackref(&cfs_rq->leaf_cfs_rq_list, leaf_cfs_rqs); 

I think there's an argument for not adding _nobackref and simply
open-coding the operation here. Could there possibly be another user
that wants this? 

Furthermore, since its tricky like hell every site would want a comment
like the above explaining exactly what and why, and when you put in that
much effort, you might as well write the list-op itself too.

Bit of a barrier to shooting your foot off or so..

  reply	other threads:[~2011-08-02 13:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-27 19:10 [PATCH RFCv2 0/8] sched: Enforce order of leaf CFS runqueues (and list cleanup) Jan H. Schönherr
2011-07-27 19:10 ` [PATCH RFCv2 1/8] list, treewide: Rename __list_del() to __list_link() Jan H. Schönherr
2011-07-27 19:10 ` [PATCH RFCv2 2/8] rcu: More rcu-variants for list manipulation Jan H. Schönherr
2011-07-29  8:41   ` Jan Schönherr
2011-08-02 13:39     ` Peter Zijlstra
2011-07-27 19:10 ` [PATCH RFCv2 3/8] sched: Handle on_list ancestor in list_add_leaf_cfs_rq() Jan H. Schönherr
2011-08-02 13:50   ` Peter Zijlstra [this message]
2011-08-03 20:44     ` Jan Schönherr
2011-07-27 19:10 ` [PATCH RFCv2 4/8] list, treewide: Rename __list_del_entry() to __list_del() Jan H. Schönherr
2011-07-27 19:10 ` [PATCH RFCv2 5/8] treewide: Use __list_del() instead of __list_link() Jan H. Schönherr
2011-07-27 19:10 ` [PATCH RFCv2 6/8] list: Make use " Jan H. Schönherr
2011-07-27 19:10 ` [PATCH RFCv2 7/8] rcu: Make use of __list_link() and __list_link_rcu() Jan H. Schönherr
2011-07-27 19:10 ` [PATCH RFCv2 8/8] rcu: Rewrite and rename list_splice_init_rcu() Jan H. Schönherr
2011-08-03 21:05 ` [PATCH RFCv2 0/8] sched: Enforce order of leaf CFS runqueues (and list cleanup) Jan Schönherr
2011-08-03 21:35   ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1312293037.1147.181.camel@twins \
    --to=peterz@infradead.org \
    --cc=dipankar@in.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=pjt@google.com \
    --cc=schnhrr@cs.tu-berlin.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome