Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965220AbcDMCKV (ORCPT ); Tue, 12 Apr 2016 22:10:21 -0400 Received: from mail-ig0-f179.google.com ([209.85.213.179]:35686 "EHLO mail-ig0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932848AbcDMCKS (ORCPT ); Tue, 12 Apr 2016 22:10:18 -0400 Date: Wed, 13 Apr 2016 10:09:29 +0800 From: Boqun Feng To: Waiman Long Cc: Alexander Viro , Jan Kara , Jeff Layton , "J. Bruce Fields" , Tejun Heo , Christoph Lameter , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Ingo Molnar , Peter Zijlstra , Andi Kleen , Dave Chinner , Scott J Norton , Douglas Hatch Subject: Re: [PATCH v7 1/4] lib/percpu-list: Per-cpu list with associated per-cpu locks Message-ID: <20160413020929.GA23058@fixme-laptop.cn.ibm.com> References: <1460501686-37096-1-git-send-email-Waiman.Long@hpe.com> <1460501686-37096-2-git-send-email-Waiman.Long@hpe.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1460501686-37096-2-git-send-email-Waiman.Long@hpe.com> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5206 Lines: 152 Hi Waiman, On Tue, Apr 12, 2016 at 06:54:43PM -0400, Waiman Long wrote: [...] > + > +/* > + * Initialize the per-cpu list head > + */ > +int init_pcpu_list_head(struct pcpu_list_head **ppcpu_head) > +{ > + struct pcpu_list_head *pcpu_head = alloc_percpu(struct pcpu_list_head); > + int cpu; > + > + if (!pcpu_head) > + return -ENOMEM; > + > + for_each_possible_cpu(cpu) { > + struct pcpu_list_head *head = per_cpu_ptr(pcpu_head, cpu); > + > + INIT_LIST_HEAD(&head->list); > + head->lock = __SPIN_LOCK_UNLOCKED(&head->lock); > + lockdep_set_class(&head->lock, &percpu_list_key); > + } > + > + *ppcpu_head = pcpu_head; > + return 0; > +} The first time I looked at this patch, I had a hard time to figure out which "struct pcpu_list_head" pointer is pointing to percpu data(the pointer could be the parameter for per/this_cpu_ptr()), and which pointer is pointing to actual structure. For example, 'pcpu_head' and 'head' above are different types of pointers. So besides improving my code reading skills, I think the following patch helps ;-) Also it can resolve several splats of sparse when running 'make C=1 lib/'. Thoughts? Regards, Boqun -------------------------------------------->8 From: Boqun Feng Date: Wed, 13 Apr 2016 09:49:13 +0800 Subject: [PATCH] lib/percpu-list: Add __percpu modifier for parameters Add __percpu modifier properly to help: 1. Differ pointers to actual structures with those to percpu structures, which could improve readability. 2. Prevent sparse from complaining about "different address spaces" Signed-off-by: Boqun Feng --- include/linux/percpu-list.h | 16 +++++++++------- lib/percpu-list.c | 8 +++++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/include/linux/percpu-list.h b/include/linux/percpu-list.h index ce8238a78198..4c8496004dc2 100644 --- a/include/linux/percpu-list.h +++ b/include/linux/percpu-list.h @@ -107,7 +107,8 @@ static inline void init_pcpu_list_node(struct pcpu_list_node *node) node->lockptr = NULL; } -static inline void free_pcpu_list_head(struct pcpu_list_head **ppcpu_head) +static inline void +free_pcpu_list_head(struct pcpu_list_head __percpu **ppcpu_head) { free_percpu(*ppcpu_head); *ppcpu_head = NULL; @@ -116,7 +117,7 @@ static inline void free_pcpu_list_head(struct pcpu_list_head **ppcpu_head) /* * Check if all the per-cpu lists are empty */ -static inline bool pcpu_list_empty(struct pcpu_list_head *pcpu_head) +static inline bool pcpu_list_empty(struct pcpu_list_head __percpu *pcpu_head) { int cpu; @@ -133,7 +134,8 @@ static inline bool pcpu_list_empty(struct pcpu_list_head *pcpu_head) * Return: true if the entry is found, false if all the lists exhausted */ static __always_inline bool -__pcpu_list_next_cpu(struct pcpu_list_head *head, struct pcpu_list_state *state) +__pcpu_list_next_cpu(struct pcpu_list_head __percpu *head, + struct pcpu_list_state *state) { if (state->lock) spin_unlock(state->lock); @@ -170,7 +172,7 @@ next_cpu: * * Return: true if the next entry is found, false if all the entries iterated */ -static inline bool pcpu_list_iterate(struct pcpu_list_head *head, +static inline bool pcpu_list_iterate(struct pcpu_list_head __percpu *head, struct pcpu_list_state *state) { /* @@ -198,7 +200,7 @@ static inline bool pcpu_list_iterate(struct pcpu_list_head *head, * * Return: true if the next entry is found, false if all the entries iterated */ -static inline bool pcpu_list_iterate_safe(struct pcpu_list_head *head, +static inline bool pcpu_list_iterate_safe(struct pcpu_list_head __percpu *head, struct pcpu_list_state *state) { /* @@ -224,8 +226,8 @@ static inline bool pcpu_list_iterate_safe(struct pcpu_list_head *head, } extern void pcpu_list_add(struct pcpu_list_node *node, - struct pcpu_list_head *head); + struct pcpu_list_head __percpu *head); extern void pcpu_list_del(struct pcpu_list_node *node); -extern int init_pcpu_list_head(struct pcpu_list_head **ppcpu_head); +extern int init_pcpu_list_head(struct pcpu_list_head __percpu **ppcpu_head); #endif /* __LINUX_PERCPU_LIST_H */ diff --git a/lib/percpu-list.c b/lib/percpu-list.c index 8a9600169966..ef2bcb8e5a1b 100644 --- a/lib/percpu-list.c +++ b/lib/percpu-list.c @@ -27,9 +27,10 @@ static struct lock_class_key percpu_list_key; /* * Initialize the per-cpu list head */ -int init_pcpu_list_head(struct pcpu_list_head **ppcpu_head) +int init_pcpu_list_head(struct pcpu_list_head __percpu **ppcpu_head) { - struct pcpu_list_head *pcpu_head = alloc_percpu(struct pcpu_list_head); + struct pcpu_list_head __percpu *pcpu_head = + alloc_percpu(struct pcpu_list_head); int cpu; if (!pcpu_head) @@ -52,7 +53,8 @@ int init_pcpu_list_head(struct pcpu_list_head **ppcpu_head) * function is called. However, deletion may be done by a different CPU. * So we still need to use a lock to protect the content of the list. */ -void pcpu_list_add(struct pcpu_list_node *node, struct pcpu_list_head *head) +void pcpu_list_add(struct pcpu_list_node *node, + struct pcpu_list_head __percpu *head) { struct pcpu_list_head *myhead; -- 2.8.0