Re: [PATCH] slab: fix oops when reading /proc/slab_allocators

[Joonsoo Kim]

On Tue, Jun 17, 2014 at 11:29:33AM +0400, Vladimir Davydov wrote:
> Hi,
> 
> On Tue, Jun 17, 2014 at 10:09:52AM +0900, Joonsoo Kim wrote:
> [...]
> > To fix the problem, I introduces object status buffer on each slab.
> > With this, we can track object status precisely, so slab leak detector
> > would not access active object and no kernel oops would occur.
> > Memory overhead caused by this fix is only imposed to
> > CONFIG_DEBUG_SLAB_LEAK which is mainly used for debugging, so memory
> > overhead isn't big problem.
> [...]
> >  
> > +static size_t calculate_freelist_size(int nr_objs, size_t align)
> > +{
> > +	size_t freelist_size;
> > +
> > +	freelist_size = nr_objs * sizeof(freelist_idx_t);
> > +	if (IS_ENABLED(CONFIG_DEBUG_SLAB_LEAK))
> > +		freelist_size += nr_objs * sizeof(char);
> > +
> > +	if (align)
> > +		freelist_size = ALIGN(freelist_size, align);
> > +
> > +	return freelist_size;
> > +}
> > +
> >  static int calculate_nr_objs(size_t slab_size, size_t buffer_size,
> >  				size_t idx_size, size_t align)
> >  {
> >  	int nr_objs;
> > +	size_t remained_size;
> >  	size_t freelist_size;
> > +	int extra_space = 0;
> >  
> > +	if (IS_ENABLED(CONFIG_DEBUG_SLAB_LEAK))
> > +		extra_space = sizeof(char);
> >  	/*
> >  	 * Ignore padding for the initial guess. The padding
> >  	 * is at most @align-1 bytes, and @buffer_size is at
> > @@ -590,14 +641,15 @@ static int calculate_nr_objs(size_t slab_size, size_t buffer_size,
> >  	 * into the memory allocation when taking the padding
> >  	 * into account.
> >  	 */
> > -	nr_objs = slab_size / (buffer_size + idx_size);
> > +	nr_objs = slab_size / (buffer_size + idx_size + extra_space);
> 
> There is one more function that wants to know how much space per object
> is spent for management. It's calculate_slab_order():
> 
> 	if (flags & CFLGS_OFF_SLAB) {
> 		/*
> 		 * Max number of objs-per-slab for caches which
> 		 * use off-slab slabs. Needed to avoid a possible
> 		 * looping condition in cache_grow().
> 		 */
> 		offslab_limit = size;
> 		offslab_limit /= sizeof(freelist_idx_t);
> 
> 		if (num > offslab_limit)
> 			break;
> 	}
> 
> May be, we should update it too?

Hello, Vladimir.

Yes, you are right! I sent v2 with this update. :)

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/