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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.10.1511120144020.18610@chino.kir.corp.google.com>
Date:	Thu, 12 Nov 2015 01:59:35 -0800 (PST)
From:	David Rientjes <rientjes@...gle.com>
To:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
cc:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Lameter <cl@...ux.com>,
	Pekka Enberg <penberg@...nel.org>,
	Joonsoo Kim <iamjoonsoo.kim@....com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] tools/vm/slabinfo: update struct slabinfo members'
 types

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/