public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Kent Overstreet <kmo@daterainc.com>
To: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org, axboe@kernel.dk, hch@infradead.org,
	hannes@cmpxchg.org
Subject: Re: [PATCH v2 5/9] percpu_ref: add PCPU_REF_DEAD
Date: Tue, 23 Sep 2014 14:14:48 -0700	[thread overview]
Message-ID: <20140923211448.GG15142@kmo-pixel> (raw)
In-Reply-To: <20140923134851.GE14905@mtj.dyndns.org>

On Tue, Sep 23, 2014 at 09:48:51AM -0400, Tejun Heo wrote:
> From 6f5bdc32c66317416c13eedb68ead2b36fb02603 Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj@kernel.org>
> Date: Tue, 23 Sep 2014 09:45:56 -0400
> 
> percpu_ref will be restructured so that percpu/atomic mode switching
> and reference killing are dedoupled.  In preparation, add
> PCPU_REF_DEAD and PCPU_REF_ATOMIC_DEAD which is OR of ATOMIC and DEAD.
> For now, ATOMIC and DEAD are changed together and all PCPU_REF_ATOMIC
> uses are converted to PCPU_REF_ATOMIC_DEAD without causing any
> behavior changes.
> 
> percpu_ref_init() now specifies an explicit alignment when allocating
> the percpu counters so that the pointer has enough unused low bits to
> accomodate the flags.  Note that one flag was fine as min alignment
> for percpu memory is 2 bytes but two flags are already too many for
> the natural alignment of unsigned longs on archs like cris and m68k.
> 
> v2: The original patch had BUILD_BUG_ON() which triggers if unsigned
>     long's alignment isn't enough to accomodate the flags, which
>     triggered on cris and m64k.  percpu_ref_init() updated to specify
>     the required alignment explicitly.  Reported by Fengguang.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Kent Overstreet <kmo@daterainc.com>
> Cc: kbuild test robot <fengguang.wu@intel.com>

Reviewed-by: Kent Overstreet <kmo@daterainc.com>

> ---
>  include/linux/percpu-refcount.h |  6 +++++-
>  lib/percpu-refcount.c           | 19 +++++++++++--------
>  2 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/percpu-refcount.h b/include/linux/percpu-refcount.h
> index 910e5f7..bd9483d 100644
> --- a/include/linux/percpu-refcount.h
> +++ b/include/linux/percpu-refcount.h
> @@ -57,6 +57,10 @@ typedef void (percpu_ref_func_t)(struct percpu_ref *);
>  /* flags set in the lower bits of percpu_ref->percpu_count_ptr */
>  enum {
>  	__PERCPU_REF_ATOMIC	= 1LU << 0,	/* operating in atomic mode */
> +	__PERCPU_REF_DEAD	= 1LU << 1,	/* (being) killed */
> +	__PERCPU_REF_ATOMIC_DEAD = __PERCPU_REF_ATOMIC | __PERCPU_REF_DEAD,
> +
> +	__PERCPU_REF_FLAG_BITS	= 2,
>  };
>  
>  struct percpu_ref {
> @@ -107,7 +111,7 @@ static inline bool __ref_is_percpu(struct percpu_ref *ref,
>  	/* paired with smp_store_release() in percpu_ref_reinit() */
>  	smp_read_barrier_depends();
>  
> -	if (unlikely(percpu_ptr & __PERCPU_REF_ATOMIC))
> +	if (unlikely(percpu_ptr & __PERCPU_REF_ATOMIC_DEAD))
>  		return false;
>  
>  	*percpu_countp = (unsigned long __percpu *)percpu_ptr;
> diff --git a/lib/percpu-refcount.c b/lib/percpu-refcount.c
> index 7aef590..e2ff19f 100644
> --- a/lib/percpu-refcount.c
> +++ b/lib/percpu-refcount.c
> @@ -34,7 +34,7 @@
>  static unsigned long __percpu *percpu_count_ptr(struct percpu_ref *ref)
>  {
>  	return (unsigned long __percpu *)
> -		(ref->percpu_count_ptr & ~__PERCPU_REF_ATOMIC);
> +		(ref->percpu_count_ptr & ~__PERCPU_REF_ATOMIC_DEAD);
>  }
>  
>  /**
> @@ -52,10 +52,13 @@ static unsigned long __percpu *percpu_count_ptr(struct percpu_ref *ref)
>  int percpu_ref_init(struct percpu_ref *ref, percpu_ref_func_t *release,
>  		    gfp_t gfp)
>  {
> +	size_t align = max_t(size_t, 1 << __PERCPU_REF_FLAG_BITS,
> +			     __alignof__(unsigned long));
> +
>  	atomic_long_set(&ref->count, 1 + PERCPU_COUNT_BIAS);
>  
> -	ref->percpu_count_ptr =
> -		(unsigned long)alloc_percpu_gfp(unsigned long, gfp);
> +	ref->percpu_count_ptr = (unsigned long)
> +		__alloc_percpu_gfp(sizeof(unsigned long), align, gfp);
>  	if (!ref->percpu_count_ptr)
>  		return -ENOMEM;
>  
> @@ -80,7 +83,7 @@ void percpu_ref_exit(struct percpu_ref *ref)
>  
>  	if (percpu_count) {
>  		free_percpu(percpu_count);
> -		ref->percpu_count_ptr = __PERCPU_REF_ATOMIC;
> +		ref->percpu_count_ptr = __PERCPU_REF_ATOMIC_DEAD;
>  	}
>  }
>  EXPORT_SYMBOL_GPL(percpu_ref_exit);
> @@ -145,10 +148,10 @@ static void percpu_ref_kill_rcu(struct rcu_head *rcu)
>  void percpu_ref_kill_and_confirm(struct percpu_ref *ref,
>  				 percpu_ref_func_t *confirm_kill)
>  {
> -	WARN_ONCE(ref->percpu_count_ptr & __PERCPU_REF_ATOMIC,
> +	WARN_ONCE(ref->percpu_count_ptr & __PERCPU_REF_ATOMIC_DEAD,
>  		  "%s called more than once on %pf!", __func__, ref->release);
>  
> -	ref->percpu_count_ptr |= __PERCPU_REF_ATOMIC;
> +	ref->percpu_count_ptr |= __PERCPU_REF_ATOMIC_DEAD;
>  	ref->confirm_switch = confirm_kill;
>  
>  	call_rcu_sched(&ref->rcu, percpu_ref_kill_rcu);
> @@ -180,12 +183,12 @@ void percpu_ref_reinit(struct percpu_ref *ref)
>  	 * Restore per-cpu operation.  smp_store_release() is paired with
>  	 * smp_read_barrier_depends() in __ref_is_percpu() and guarantees
>  	 * that the zeroing is visible to all percpu accesses which can see
> -	 * the following __PERCPU_REF_ATOMIC clearing.
> +	 * the following __PERCPU_REF_ATOMIC_DEAD clearing.
>  	 */
>  	for_each_possible_cpu(cpu)
>  		*per_cpu_ptr(percpu_count, cpu) = 0;
>  
>  	smp_store_release(&ref->percpu_count_ptr,
> -			  ref->percpu_count_ptr & ~__PERCPU_REF_ATOMIC);
> +			  ref->percpu_count_ptr & ~__PERCPU_REF_ATOMIC_DEAD);
>  }
>  EXPORT_SYMBOL_GPL(percpu_ref_reinit);
> -- 
> 1.9.3
> 

  reply	other threads:[~2014-09-23 21:13 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-23  5:55 [PATCHSET percpu/for-3.18] percpu_ref: implement switch_to_atomic/percpu() Tejun Heo
2014-09-23  5:55 ` [PATCH 1/9] percpu_ref: relocate percpu_ref_reinit() Tejun Heo
2014-09-23 21:01   ` Kent Overstreet
2014-09-23 21:07   ` Kent Overstreet
2014-09-23  5:55 ` [PATCH 2/9] percpu_ref: minor code and comment updates Tejun Heo
2014-09-23 21:09   ` Kent Overstreet
2014-09-23  5:55 ` [PATCH 3/9] percpu_ref: replace pcpu_ prefix with percpu_ Tejun Heo
2014-09-23 21:10   ` Kent Overstreet
2014-09-23  5:55 ` [PATCH 4/9] percpu_ref: rename things to prepare for decoupling percpu/atomic mode switch Tejun Heo
2014-09-23 21:11   ` Kent Overstreet
2014-09-23  5:55 ` [PATCH 5/9] percpu_ref: add PCPU_REF_DEAD Tejun Heo
2014-09-23 13:48   ` [PATCH v2 " Tejun Heo
2014-09-23 21:14     ` Kent Overstreet [this message]
2014-09-23  5:55 ` [PATCH 6/9] percpu_ref: decouple switching to atomic mode and killing Tejun Heo
2014-09-23 21:13   ` Kent Overstreet
2014-09-23  5:55 ` [PATCH 7/9] percpu_ref: decouple switching to percpu mode and reinit Tejun Heo
2014-09-23 13:49   ` [PATCH v2 " Tejun Heo
2014-09-23 21:15     ` Kent Overstreet
2014-09-23  5:55 ` [PATCH 8/9] percpu_ref: add PERCPU_REF_INIT_* flags Tejun Heo
2014-09-23 21:16   ` Kent Overstreet
2014-09-23  5:55 ` [PATCH 9/9] percpu_ref: make INIT_ATOMIC and switch_to_atomic() sticky Tejun Heo
2014-09-23 21:17   ` Kent Overstreet
2014-09-24 17:32 ` [PATCHSET percpu/for-3.18] percpu_ref: implement switch_to_atomic/percpu() Tejun Heo

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=20140923211448.GG15142@kmo-pixel \
    --to=kmo@daterainc.com \
    --cc=axboe@kernel.dk \
    --cc=hannes@cmpxchg.org \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    /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