Re: [PATCH] memcg: don't initialize kmem-cache destroying work forroot caches

[Glauber Costa]

On 05/29/2013 06:48 AM, Glauber Costa wrote:
> On 05/29/2013 04:23 AM, Andrew Morton wrote:
>> On Tue, 14 May 2013 16:38:38 +0400 Andrey Vagin <avagin@xxxxxxxxxx> wrote:
>>
>>> struct memcg_cache_params has a union. Different parts of this union are
>>> used for root and non-root caches. A part with destroying work is used only
>>> for non-root caches.
>>
>> That union is a bit dangerous.  Perhaps it would be better to do
>> something like
>>
>> --- a/include/linux/slab.h~a
>> +++ a/include/linux/slab.h
>> @@ -337,15 +337,17 @@ static __always_inline int kmalloc_size(
>>  struct memcg_cache_params {
>>  	bool is_root_cache;
>>  	union {
>> -		struct kmem_cache *memcg_caches[0];
>> -		struct {
>> +		struct memcg_root_cache {
>> +			struct kmem_cache *caches[0];
>> +		} memcg_root_cache;
>> +		struct memcg_child_cache {
>>  			struct mem_cgroup *memcg;
>>  			struct list_head list;
>>  			struct kmem_cache *root_cache;
>>  			bool dead;
>>  			atomic_t nr_pages;
>>  			struct work_struct destroy;
>> -		};
>> +		} memcg_child_cache;
>>  	};
>>  };
>>
>> And then adopt the convention of selecting either memcg_root_cache or
>> memcg_child_cache at the highest level then passing the more strongly
>> typed pointer to callees.
>>
> 
> Since it is already creating problems, yes, I agree.
> 
> I will try to cook up something soon.
> 

There are other cleanups being requested as well. (Tejun claims that we
would be better off with locks than with barriers for the destruction
path). To avoid conflicting with the current shrinkers work - that is
very massive, and since none of those are pressing, I will try to tackle
both next week (on top of that, if possible)
--
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/