Pekka Enberg wrote:
> Hi,
>
> On 4/17/07, Pavel Emelianov <[EMAIL PROTECTED]> wrote:
>> The out_of_memory() function and SysRq-M handler call
>> show_mem() to show the current memory usage state.
>>
>> This is also helpful to see which slabs are the largest
>> in the system.
>
> Makes sense.
Thanks! :)
> On 4/17/07, Pavel Emelianov <[EMAIL PROTECTED]> wrote:
>> diff --git a/mm/slab.c b/mm/slab.c
>> index 21b3c61..9a5829a 100644
>> --- a/mm/slab.c
>> +++ b/mm/slab.c
>> @@ -749,6 +749,7 @@ static inline void init_lock_keys(void)
>> * 2. Protect sanity of cpu_online_map against cpu hotplug events
>> */
>> static DEFINE_MUTEX(cache_chain_mutex);
>> +static DEFINE_SPINLOCK(cache_chain_lock);
>
> So, now we have two locks protecting cache_chain? Please explain why
> you can't use the mutex.
Because OOM can actually happen with this mutex locked. For example
kmem_cache_create() locks it and calls kmalloc(), or write to
/proc/slabinfo also locks it and calls do_tune_cpu_caches(). This is
very rare case and the deadlock is VERY unlikely to happen, but it
will be very disappointing if it happens.
Moreover, I put the call to show_slabs() into sysrq handler, so it may
be called from atomic context.
Making mutex_trylock() is possible, but we risk of loosing this info
in case OOM happens while the mutex is locked for cache shrinking (see
cache_reap() for example)...
So we have a choice - either we have an additional lock on a slow and
rare paths and show this info for sure, or we do not have a lock, but
have a risk of loosing this info.
>> +static unsigned long get_cache_size(struct kmem_cache *cachep)
>> +{
>> + unsigned long slabs;
>> + struct kmem_list3 *l3;
>> + struct list_head *lh;
>> + int node;
>> +
>> + slabs = 0;
>> +
>> + for_each_online_node (node) {
>> + l3 = cachep->nodelists[node];
>> + if (l3 == NULL)
>> + continue;
>> +
>> + spin_lock(&l3->list_lock);
>> + list_for_each (lh, &l3->slabs_full)
>> + slabs++;
>> + list_for_each (lh, &l3->slabs_partial)
>> + slabs++;
>> + list_for_each (lh, &l3->slabs_free)
>> + slabs++;
>> + spin_unlock(&l3->list_lock);
>> + }
>> +
>> + return slabs * ((PAGE_SIZE << cachep->gfporder) +
>> + (OFF_SLAB(cachep) ? cachep->slabp_cache->buffer_size :
>> 0));
>> +}
>
> Considering you're doing this at out_of_memory() time, wouldn't it
> make more sense to add a ->nr_pages to struct kmem_cache and do the
> tracking in kmem_getpages/kmem_freepages?
Sounds good.
> I would also drop the OFF_SLAB bits because it really doesn't matter
> that much for your purposes. Besides, you're already per-node and
> per-CPU caches here which attribute to much more memory on NUMA setups
> for example.
This gives us a more precise information :) The precision is less than 1%
so if nobody likes/needs it, this may be dropped.
Pavel.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
