Re: [PATCH 3/3] tools/vm/slabinfo: update struct slabinfo members' types

From: David Rientjes
Date: Thu Nov 12 2015 - 04:59:46 EST


On Thu, 12 Nov 2015, Sergey Senozhatsky wrote:

> > This has nothing to do with object_size in the kernel.
>
> what we have in slabinfo as slab_size(), ->object_size, etc.
> comming from slub's sysfs attrs:
>
> chdir("/sys/kernel/slab")
> while readdir
> ...
> slab->object_size = get_obj("object_size");
> slab->slab_size = get_obj("slab_size");
> ...
>
> and attr show handlers are:
>
> ...
> static ssize_t slab_size_show(struct kmem_cache *s, char *buf)
> {
> return sprintf(buf, "%d\n", s->size);
> }
> SLAB_ATTR_RO(slab_size);
>
> static ssize_t object_size_show(struct kmem_cache *s, char *buf)
> {
> return sprintf(buf, "%d\n", s->object_size);
> }
> SLAB_ATTR_RO(object_size);
> ...
>
> so those are sprintf("%d") of `struct kmem_cache'-s `int'
> values.
>
>
> > total_used and total_objects are unsigned long long.
>
> yes, that's correct.
> but `total_used / total_objects' cannot be larger that the size
> of the largest object, which is represented in the kernel and
> returned to user space as `int'. it must fit into `unsigned int'.
>

Again, I am referring only to slabinfo as its own logical unit, it
shouldn't be based on the implementation of any slab allocator in
particular. avg_objsize has nothing to do with your patch, which is
advertised as fixing the mismatch in sign type of variables under
comparison.

There seems to be an on-going issue in this patchset that you're not
confronting: you are mixing extraneous changes into patches that are
supposed to do one thing. This already got you in trouble in the first
patch where you just threw -O2 into the Makefile randomly, and without any
mention in the commit description, and then you don't understand how to
fix the warnings that it now presents in page-types.

The warnings being shown are a result of the particular _optimization_
that your gcc version has done and your subsequent patch is only
addressing the ones that appear when you, yourself, compile. Between
different gcc versions, the optimization done by -O2 may be different and
it will warn of more or less variables that may be clobbered as a result
OF ITS OPTIMIZATION. You miss entirely that _any_ variable modified after
the setjmp() can be clobbered, most notably "off" which is the iterator
of the very loop the setjmp() appears in! Playing whack-a-mole in the
warnings you get without understanding them is the issue here.

Please, very respectfully, do not include extraneous changes into
patches, especially without mentioning them in the commit description,
when the change isn't needed or understood.
--
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/