From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755139AbbDINnu (ORCPT ); Thu, 9 Apr 2015 09:43:50 -0400 Received: from smtprelay0024.hostedemail.com ([216.40.44.24]:50741 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752833AbbDINns (ORCPT ); Thu, 9 Apr 2015 09:43:48 -0400 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::::,RULES_HIT:2:41:355:379:541:599:800:960:973:982:988:989:1260:1277:1311:1313:1314:1345:1359:1437:1515:1516:1518:1535:1593:1594:1605:1606:1730:1747:1777:1792:2393:2553:2559:2562:2693:2840:2898:3138:3139:3140:3141:3142:3151:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:4117:4321:4605:5007:6119:6261:7576:7875:9010:10004:10848:10967:11026:11232:11473:11658:11914:12043:12291:12296:12517:12519:12555:12683:12740:13161:13163:13229:13255:14096:14097:21067:21080,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0 X-HE-Tag: wrist40_39e658456c28 X-Filterd-Recvd-Size: 6251 Date: Thu, 9 Apr 2015 09:43:43 -0400 From: Steven Rostedt To: Xunlei Pang Cc: linux-kernel@vger.kernel.org, Peter Zijlstra , Juri Lelli , Dan Streetman , Xunlei Pang Subject: Re: [PATCH v5 1/3] lib/plist: Provide plist_add_head() for nodes with the same prio Message-ID: <20150409094343.3d4b116a@gandalf.local.home> In-Reply-To: <1428550038-13619-1-git-send-email-xlpang@126.com> References: <1428550038-13619-1-git-send-email-xlpang@126.com> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 9 Apr 2015 11:27:16 +0800 Xunlei Pang wrote: > From: Xunlei Pang > > If there're multiple nodes with the same prio as @node, currently > plist_add() will add @node behind all of them. Now we need to add > @node before all of these nodes for SMP RT scheduler. > > This patch adds a common __plist_add() for adding @node before or > after existing nodes with the same prio, then adds plist_add_head() > and plist_add_tail() inline wrapper functions for convenient uses. > > Finally, define plist_add() as plist_add_tail() which has the same > behaviour as before. > > Reviewed-by: Dan Streetman > Signed-off-by: Xunlei Pang > --- > include/linux/plist.h | 30 +++++++++++++++++++++++++++++- > lib/plist.c | 22 +++++++++++++++++++--- > 2 files changed, 48 insertions(+), 4 deletions(-) > > diff --git a/include/linux/plist.h b/include/linux/plist.h > index 9788360..10c834c 100644 > --- a/include/linux/plist.h > +++ b/include/linux/plist.h > @@ -138,7 +138,35 @@ static inline void plist_node_init(struct plist_node *node, int prio) > INIT_LIST_HEAD(&node->node_list); > } > > -extern void plist_add(struct plist_node *node, struct plist_head *head); > +extern void __plist_add(struct plist_node *node, > + struct plist_head *head, bool is_head); > + > +/** > + * plist_add_head - add @node to @head, before all existing same-prio nodes > + * > + * @node: &struct plist_node pointer > + * @head: &struct plist_head pointer I just noticed that the plist kerneldoc is really pathetic. Not your fault, but lets not continue the trend. The above descriptions of the @node and @head are really lame. It adds nothing to the prototype. It's equivalent to: /* increment i */ i++; Something like this is better: * @node: The plist_node to be added to @head * @head: The plist_head that @node is being added to The oneline description you have above is good. Ideally, we should have a bit more detailed description here too. But for now we can leave it. The rest of the @node and @head's should be fixed too. > + */ > +static inline > +void plist_add_head(struct plist_node *node, struct plist_head *head) > +{ > + __plist_add(node, head, true); > +} > + > +/** > + * plist_add_tail - add @node to @head, after all existing same-prio nodes > + * > + * @node: &struct plist_node pointer > + * @head: &struct plist_head pointer > + */ > +static inline > +void plist_add_tail(struct plist_node *node, struct plist_head *head) > +{ > + __plist_add(node, head, false); > +} > + > +#define plist_add plist_add_tail > + > extern void plist_del(struct plist_node *node, struct plist_head *head); > > extern void plist_requeue(struct plist_node *node, struct plist_head *head); > diff --git a/lib/plist.c b/lib/plist.c > index 3a30c53..9f396f1 100644 > --- a/lib/plist.c > +++ b/lib/plist.c > @@ -66,12 +66,16 @@ static void plist_check_head(struct plist_head *head) > #endif > > /** > - * plist_add - add @node to @head > + * __plist_add - add @node to @head > * > * @node: &struct plist_node pointer > * @head: &struct plist_head pointer > + * @is_head: bool As I said above. This horrible kerneldoc was there before and is not your fault. But if you are going to touch it, lets fix it up first. * @is_head: True if adding to head of prio list, false otherwise > + * > + * If there're any nodes with the same prio, add @node > + * behind or before all of them according to @is_head. The above is very lacking if I want to use this, or know how to use it. It does not tell me what @is_head must be to add before or behind them. * For nodes of the same prio, @node will be added at the head of * previously added nodes if @is_head is true, or it will be added * at the tail of previously added nodes if @is_head is false. That will be much more descriptive for people looking at this code for the first time. The rest of the code looks fine. -- Steve > */ > -void plist_add(struct plist_node *node, struct plist_head *head) > +void __plist_add(struct plist_node *node, struct plist_head *head, bool is_head) > { > struct plist_node *first, *iter, *prev = NULL; > struct list_head *node_next = &head->node_list; > @@ -96,8 +100,20 @@ void plist_add(struct plist_node *node, struct plist_head *head) > struct plist_node, prio_list); > } while (iter != first); > > - if (!prev || prev->prio != node->prio) > + if (!prev || prev->prio != node->prio) { > list_add_tail(&node->prio_list, &iter->prio_list); > + } else if (is_head) { > + /* > + * prev has the same priority as the node that is being > + * added. It is also the first node for this priority, > + * but the new node needs to be added ahead of it. > + * To accomplish this, replace prev in the prio_list > + * with node. Then set node_next to prev->node_list so > + * that the new node gets added before prev and not iter. > + */ > + list_replace_init(&prev->prio_list, &node->prio_list); > + node_next = &prev->node_list; > + } > ins_node: > list_add_tail(&node->node_list, node_next); >