From patchwork Wed Apr 13 02:09:29 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boqun Feng X-Patchwork-Id: 8817921 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id ADF659F39A for ; Wed, 13 Apr 2016 02:10:43 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A94F3201F2 for ; Wed, 13 Apr 2016 02:10:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 885D3201EF for ; Wed, 13 Apr 2016 02:10:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933467AbcDMCKU (ORCPT ); Tue, 12 Apr 2016 22:10:20 -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 Received: by mail-ig0-f179.google.com with SMTP id gy3so104679008igb.0; Tue, 12 Apr 2016 19:10:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=mCpR8Qferhh4nw8lFjMZrYFvVx7yl/1PULOkUYuRXRo=; b=u+qw9GaqoUpm5G3eFJBhi6s6dPiT/pkNdCB0GJJv6FoS3K3FejIFrYqG/xZXZcxQJ4 2oUlgtnQI4CLnBGm2r+sSwxydml51fvb2sAlnsh7Fd+gjscXS3uVWwjaRrsuCjTfCmIJ c37R1013L3xZSquAEuDSzcQRflLWW531UGh/xzCtenBSaI6zmFFyj0vv1X/W0puFvrNe S0gFxEPSnZ+v+UsoVElg1itIVZ/Mx7dIS2ScETVLdIl1AftCxcftPPOS3w56qRSv3kSv ESS2CwvlXeW4rr/knv0RhjgnKljek/TCwPTCMCtvPK6rLEfC2aOmeRR0Z1hmy4rEOfON QE2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=mCpR8Qferhh4nw8lFjMZrYFvVx7yl/1PULOkUYuRXRo=; b=lROc4VmmniC1Tfy9jP4CQYyYHZl5mV4HYG1vmWnlcP73CL3J8jR9MpR6cG0FIEiVYt xYAKKsipAvY+ingXjka3CTJg9pMqDScC5ePMbur5yXRWTLyHMeykPsclgwRshnSgTe7i Z3Glv8KhngUU4l/UZ6J42b7j4R5P07vEVhMKlc4TwIHs2cH33Ma5u8Pmy4Znjrg9u6wW EFm3tUVxatHQAlRyhbDAzYEw+Y4nkNiuk3nVYkJ/zjau0upb4s1vodEymlvCHZJUXz6K N4Gl/TGztqBTKrxiCkh2H5OlnAyl5J7Rf2NEHaq9yAmTx8PR9XHqAiw3RTzlX3jLV2DI tKlw== X-Gm-Message-State: AOPr4FWxL8qFaUZb56qX1r4j2fi3eUbXNgKLUh4FEEhOSIt/jrzLHIVpyS0FAb6pVjZ3Xg== X-Received: by 10.50.217.200 with SMTP id pa8mr26562535igc.78.1460513417254; Tue, 12 Apr 2016 19:10:17 -0700 (PDT) Received: from localhost ([45.32.128.109]) by smtp.gmail.com with ESMTPSA id in6sm17565610igb.0.2016.04.12.19.10.15 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 Apr 2016 19:10:16 -0700 (PDT) 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-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-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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;