lkml.org 
[lkml]   [2011]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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. */
};
};
};

\
 
 \ /
  Last update: 2011-07-13 13:31    [W:0.121 / U:0.360 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site