Re: [PATCH v2 1/2] slub: Enable backtrace for create/delete points.

[Ben Greear <greearb@candelatech.com>]

On 07/01/2011 07:08 AM, Christoph Lameter wrote:
> On Tue, 28 Jun 2011, greearb@candelatech.com wrote:
>
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 35f351f..3477ce5 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -191,8 +191,12 @@ static LIST_HEAD(slab_caches);
>>   /*
>>    * Tracking user of a slab.
>>    */
>> +#define TRACK_ADDRS_COUNT 16
>>   struct track {
>> -	unsigned long addr;	/* Called from address */
>> +	unsigned long caddr;
>
> Keep the name the same since its role does not change. That makes the
> patch touch less code.
>
>> +#ifdef CONFIG_STACKTRACE
>> +	unsigned long addrs[TRACK_ADDRS_COUNT];	/* Called from address */
>> +#endif
>>   	int cpu;		/* Was running on cpu */
>>   	int pid;		/* Pid context */
>>   	unsigned long when;	/* When did the operation occur */
>> @@ -420,7 +424,25 @@ static void set_track(struct kmem_cache *s, void *object,
>>   	struct track *p = get_track(s, object, alloc);
>>
>>   	if (addr) {
>> -		p->addr = addr;
>> +#ifdef CONFIG_STACKTRACE
>> +		struct stack_trace trace;
>> +		int i;
>> +
>> +		trace.nr_entries = 0;
>> +		trace.max_entries = TRACK_ADDRS_COUNT;
>> +		trace.entries = p->addrs;
>> +		trace.skip = 3;
>> +		save_stack_trace(&trace);
>> +
>> +		/* See rant in lockdep.c */
>> +		if (trace.nr_entries != 0&&
>> +		    trace.entries[trace.nr_entries - 1] == ULONG_MAX)
>> +			trace.nr_entries--;
>> +
>> +		for (i = trace.nr_entries; i<  TRACK_ADDRS_COUNT; i++)
>> +			p->addrs[i] = 0;
>> +#endif
>
> Hmmm... Is this really working on all architectures? I remember from years
> past that we had issues with adding the same functionality to slab.

The lockdep code seems to use the same basic logic, and I didn't notice
any arch checks in it.  I think recently someone must have fixed the backtrace
logic to be more easily used...this API didn't exist a few kernels
ago.

>
> Otherwise the patch is quite straightforward (and it will be much cleaner
> if you do not change the name of "addr").

Ok, will change that.

Thanks,
Ben

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


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com