Messages in this thread Patch in this message | | | Date | Wed, 13 Jul 2011 04:28:44 -0700 (PDT) | From | Hugh Dickins <> | 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@linux.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. */ }; }; };
| |