| 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
| ||
|
Message-ID: <alpine.LSU.2.00.1107130357560.4184@sister.anvils>
Date: Wed, 13 Jul 2011 04:28:44 -0700 (PDT)
From: Hugh Dickins <hughd@...gle.com>
To: Christoph Lameter <cl@...ux.com>
cc: Pekka Enberg <penberg@...nel.org>,
Eric Dumazet <eric.dumazet@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH next/mmotm] slub: partly fix freeze in __slab_free
On Tue, 12 Jul 2011, Christoph Lameter wrote:
> I wish there would be some easier way to overlay fields.
Yes, struct page has become ... a mess.
(I'd use stronger language, but I'm being a little careful at the
moment, not to say something that Andrew might then throw back at
me in our radix-tree discussion ;)
I'd be happy for the 32bit and 64bit cases to be separated entirely,
if that would help seeing their layouts clearly (but I don't think
that's actually the problem).
It is the (standard e.g. x86_64) 64bit layout to which you are adding
8 bytes, isn't it? Not in the patch below, but in your cmpxchg_double
slub series. Which aligns it nicely for everyone, but does use
significantly more mem_map memory - I wonder how many people
have really taken that on board.
> The counters
> field exists to allow an easier reference to the bitfield structure in
> order to pass it to cmpxchg_double_slab.
>
> In 64 bit the counters field needs to overlay the bitfields and _count
> since we can only pass 64 bits integers to cmpxchg_double_slab.
>
> On 32 bit counters field only covers the bitfields.
>
> The "new" page struct in __slab_free is only partially populated and the
> overlays are used for conversion between structs and words in order to be
> able to pass the struct contents by value.
>
> Signed-off-by: Christoph Lameter <cl@...ux.com>
As I reported in Fengguang's more_io thread, I couldn't get this patch
to apply, unsurprisingly to mmotm, more surprisingly to Pekka's for-next
(which adds in the _count build fix). Did you try to tidy up the
indentations a little in between?
At the bottom I've put the patch against mmotm which I started testing
instead on ppc64 a few hours ago. It again seized up in __slab_free()
after two and a half hours. So either I screwed up my transliteration
of your patch, or your patch is still wrong. Please check mine out.
Is yours actually tested on PowerPC or ARM, say? I wonder if you're
making endianness or bitfield ordering assumptions; but I've made no
attempt to work it out. Anything I can send you? Disassembly of
__slab_free()?
I didn't actually try the patch on x86 at all.
>
> ---
> include/linux/mm_types.h | 22 ++++++++++++++--------
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> Index: linux-2.6/include/linux/mm_types.h
> ===================================================================
> --- linux-2.6.orig/include/linux/mm_types.h 2011-07-12 10:14:08.057706223 -0500
> +++ linux-2.6/include/linux/mm_types.h 2011-07-12 10:14:14.117706184 -0500
> @@ -56,20 +56,26 @@ struct page {
> };
>
> union {
> - atomic_t _mapcount; /* Count of ptes mapped in mms,
> + /* Used for cmpxchg_double in slub */
> + unsigned long counters;
> +
> + struct {
> +
> + union {
> + atomic_t _mapcount; /* Count of ptes mapped in mms,
> * to show when page is mapped
> * & limit reverse map searches.
> */
>
> - /* Used for cmpxchg_double in slub */
> - unsigned long counters;
> - struct {
> - unsigned inuse:16;
> - unsigned objects:15;
> - unsigned frozen:1;
> + struct {
> + unsigned inuse:16;
> + unsigned objects:15;
> + unsigned frozen:1;
> + };
> + };
> + atomic_t _count; /* Usage count, see below. */
> };
> };
> - atomic_t _count; /* Usage count, see below. */
> };
>
> /* Third double word block */
This is what I tested with: is it what you intended against mmotm?
--- mmotm/include/linux/mm_types.h 2011-07-09 11:25:18.000000000 -0700
+++ linux/include/linux/mm_types.h 2011-07-12 22:04:01.000000000 -0700
@@ -49,30 +49,31 @@ struct page {
* see PAGE_MAPPING_ANON below.
*/
/* Second double word */
- union {
- struct {
+ struct {
+ union {
pgoff_t index; /* Our offset within mapping. */
- atomic_t _mapcount; /* Count of ptes mapped in mms,
+ void *freelist; /* slub first free object */
+ };
+
+ union {
+ /* Used for cmpxchg_double in slub */
+ unsigned long counters;
+
+ struct {
+
+ union {
+ atomic_t _mapcount; /* Count of ptes mapped in mms,
* to show when page is mapped
* & limit reverse map searches.
*/
- atomic_t _count; /* Usage count, see below. */
- };
- struct { /* SLUB cmpxchg_double area */
- void *freelist;
- union {
- unsigned long counters;
- struct {
- unsigned inuse:16;
- unsigned objects:15;
- unsigned frozen:1;
- /*
- * Kernel may make use of this field even when slub
- * uses the rest of the double word!
- */
- atomic_t _count;
+ struct {
+ unsigned inuse:16;
+ unsigned objects:15;
+ unsigned frozen:1;
+ };
};
+ atomic_t _count; /* Usage count, see below. */
};
};
};
--
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/
Powered by blists - more mailing lists