Re: [PATCH 6/6] mm/slab: allocation fastpath without disabling irq

[Joonsoo Kim]

On Mon, Jan 05, 2015 at 09:28:14AM -0600, Christoph Lameter wrote:
> On Mon, 5 Jan 2015, Joonsoo Kim wrote:
> 
> > index 449fc6b..54656f0 100644
> > --- a/mm/slab.c
> > +++ b/mm/slab.c
> > @@ -168,6 +168,41 @@ typedef unsigned short freelist_idx_t;
> >
> >  #define SLAB_OBJ_MAX_NUM ((1 << sizeof(freelist_idx_t) * BITS_PER_BYTE) - 1)
> >
> > +#ifdef CONFIG_PREEMPT
> > +/*
> > + * Calculate the next globally unique transaction for disambiguiation
> > + * during cmpxchg. The transactions start with the cpu number and are then
> > + * incremented by CONFIG_NR_CPUS.
> > + */
> > +#define TID_STEP  roundup_pow_of_two(CONFIG_NR_CPUS)
> > +#else
> > +/*
> > + * No preemption supported therefore also no need to check for
> > + * different cpus.
> > + */
> > +#define TID_STEP 1
> > +#endif
> > +
> > +static inline unsigned long next_tid(unsigned long tid)
> > +{
> > +	return tid + TID_STEP;
> > +}
> > +
> > +static inline unsigned int tid_to_cpu(unsigned long tid)
> > +{
> > +	return tid % TID_STEP;
> > +}
> > +
> > +static inline unsigned long tid_to_event(unsigned long tid)
> > +{
> > +	return tid / TID_STEP;
> > +}
> > +
> > +static inline unsigned int init_tid(int cpu)
> > +{
> > +	return cpu;
> > +}
> > +
> 
> Ok the above stuff needs to go into the common code. Maybe in mm/slab.h?
> And its a significant feature contributed by me so I'd like to have an
> attribution here.

Okay. I will try!

> 
> >  /*
> >   * true if a page was allocated from pfmemalloc reserves for network-based
> >   * swap
> > @@ -187,7 +222,8 @@ static bool pfmemalloc_active __read_mostly;
> >   *
> >   */
> >  struct array_cache {
> > -	unsigned int avail;
> > +	unsigned long avail;
> > +	unsigned long tid;
> >  	unsigned int limit;
> >  	unsigned int batchcount;
> >  	unsigned int touched;
> > @@ -657,7 +693,8 @@ static void start_cpu_timer(int cpu)
> >  	}
> >  }
> 
> This increases the per cpu struct size and should lead to a small
> performance penalty.

Yes, but, it's marginal than improvement of this patchset.
> 
> > -	 */
> > -	if (likely(objp)) {
> > -		STATS_INC_ALLOCHIT(cachep);
> > -		goto out;
> > +	objp = ac->entry[avail - 1];
> > +	if (unlikely(!this_cpu_cmpxchg_double(
> > +		cachep->cpu_cache->avail, cachep->cpu_cache->tid,
> > +		avail, tid,
> > +		avail - 1, next_tid(tid))))
> > +		goto redo;
> 
> 
> Hmm... Ok that looks good.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/