next-20130204 - bisected slab problem to "slab: Common constants for kmalloc boundaries"

James Hogan

author

2013-02-04 16:22 UTC #0 lore raw

Hi,

I've hit boot problems in next-20130204 on Meta:

META213-Thread0 DSP [LogF] kobject (4fc03980): tried to init an initialized object, something is seriously wrong. META213-Thread0 DSP [LogF]
META213-Thread0 DSP [LogF] Call trace:
META213-Thread0 DSP [LogF] [<4000888c>] _show_stack+0x68/0x7c
META213-Thread0 DSP [LogF] [<400088b4>] _dump_stack+0x14/0x28
META213-Thread0 DSP [LogF] [<40103794>] _kobject_init+0x58/0x9c
META213-Thread0 DSP [LogF] [<40103810>] _kobject_create+0x38/0x64
META213-Thread0 DSP [LogF] [<40103eac>] _kobject_create_and_add+0x14/0x8c
META213-Thread0 DSP [LogF] [<40190ac4>] _mnt_init+0xd8/0x220
META213-Thread0 DSP [LogF] [<40190508>] _vfs_caches_init+0xb0/0x160
META213-Thread0 DSP [LogF] [<401851f4>] _start_kernel+0x274/0x340
META213-Thread0 DSP [LogF] [<40188424>] _metag_start_kernel+0x58/0x6c
META213-Thread0 DSP [LogF] [<40000044>] __start+0x44/0x48
META213-Thread0 DSP [LogF]
META213-Thread0 DSP [LogF] devtmpfs: initialized
META213-Thread0 DSP [LogF] L2 Cache: Not present
META213-Thread0 DSP [LogF] BUG: failure at fs/sysfs/dir.c:736/sysfs_read_ns_type()! META213-Thread0 DSP [LogF] Kernel panic - not syncing: BUG!
META213-Thread0 DSP [Thread Exit] Thread has exited - return code = 4294967295

I've bisected it to the following commit:

commit 95a05b428cc675694321c8f762591984f3fd2b1e

Author: Christoph Lameter <[email protected]>
Date:   Thu Jan 10 19:14:19 2013 +0000

slab: Common constants for kmalloc boundaries

Standardize the constants that describe the smallest and largest object kept in the kmalloc arrays for SLAB and SLUB.

Differentiate between the maximum size for which a slab cache is used (KMALLOC_MAX_CACHE_SIZE) and the maximum allocatable size (KMALLOC_MAX_SIZE, KMALLOC_MAX_ORDER).

Signed-off-by Christoph Lameter <[email protected]>

Signed-off-by Pekka Enberg <[email protected]>

Any ideas what could be going on here?

This commit merged with the metag for-next branch fails, but the previous commit merged with the metag for-next branch doesn't fail.

make savedefconfig output:

CONFIG_EXPERIMENTAL=y
# CONFIG_LOCALVERSION_AUTO is not set
# CONFIG_SWAP is not set
CONFIG_SYSVIPC=y
CONFIG_LOG_BUF_SHIFT=13 CONFIG_SYSFS_DEPRECATED=y CONFIG_SYSFS_DEPRECATED_V2=y CONFIG_BLK_DEV_INITRD=y CONFIG_INITRAMFS_SOURCE="arch/metag/boot/ramdisk.cpio" CONFIG_INITRAMFS_COMPRESSION_GZIP=y CONFIG_KALLSYMS_ALL=y
# CONFIG_ELF_CORE is not set
CONFIG_SLAB=y
# CONFIG_BLK_DEV_BSG is not set CONFIG_PARTITION_ADVANCED=y
# CONFIG_MSDOS_PARTITION is not set
# CONFIG_IOSCHED_DEADLINE is not set
# CONFIG_IOSCHED_CFQ is not set CONFIG_METAG_L2C=y
CONFIG_FLATMEM_MANUAL=y CONFIG_METAG_HALT_ON_PANIC=y CONFIG_METAG_ATOMICITY_IRQSOFF=y CONFIG_METAG_DA=y
CONFIG_HZ_100=y
CONFIG_DEVTMPFS=y
# CONFIG_STANDALONE is not set
# CONFIG_PREVENT_FIRMWARE_BUILD is not set
# CONFIG_FW_LOADER is not set CONFIG_BLK_DEV_RAM=y CONFIG_BLK_DEV_RAM_COUNT=1 CONFIG_BLK_DEV_RAM_SIZE=16384
# CONFIG_INPUT is not set
# CONFIG_SERIO is not set
# CONFIG_VT is not set
# CONFIG_LEGACY_PTYS is not set CONFIG_DA_TTY=y
CONFIG_DA_CONSOLE=y
# CONFIG_DEVKMEM is not set
# CONFIG_HW_RANDOM is not set
# CONFIG_HWMON is not set
# CONFIG_USB_SUPPORT is not set
# CONFIG_DNOTIFY is not set
CONFIG_TMPFS=y
# CONFIG_MISC_FILESYSTEMS is not set
# CONFIG_SCHED_DEBUG is not set CONFIG_DEBUG_INFO=y
CONFIG_FRAME_POINTER=y

Thanks
James

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2013-02-04 19:22 UTC #1 lore raw

Quoted 1 line · James Hogan

I've hit boot problems in next-20130204 on Meta:

Meta is an arch that is not in the tree yet? How would I build for meta?

What are the values of

MAX_ORDER
PAGE_SHIFT
ARCH_DMA_MINALIGN
CONFIG_ZONE_DMA

?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

James Hogan

author

2013-02-05 10:08 UTC #2 lore raw

Quoted 5 lines · Christoph Lameter

Quoted 1 line · James Hogan
I've hit boot problems in next-20130204 on Meta:
Meta is an arch that is not in the tree yet? How would I build for meta?

Yes (well, it's in -next now, so merging the for-next branch of
git://github.com/jahogan/metag-linux.git would add Meta support, which at the point of your commit produces no conflicts).

It sounds like Stephen Warren has hit the same problem (in the configuration I'm using ARCH_DMA_MINALIGN was also 64, but in another configuration that worked, ARCH_DMA_MINALIGN was 8 (see arch/metag/include/asm/cache.h).

For the record though, to cross compile, you'd need to build a meta2_defconfig of the buildroot at
git://github.com/img-meta/metag-buildroot.git.

Cheers
James

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

James Hogan

author

2013-02-05 12:53 UTC #3 lore raw

In reply to Christoph Lameter

Hi Christoph,

Quoted 3 lines · Christoph Lameter

What are the values of
MAX_ORDER

10

Quoted 1 line

PAGE_SHIFT

12

Quoted 1 line

ARCH_DMA_MINALIGN

64 (it works if changed to 8)

Quoted 1 line

CONFIG_ZONE_DMA

not defined

Cheers
James

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Stephen Warren

2013-02-04 19:39 UTC #4 lore raw

In reply to James Hogan

Quoted 14 lines · James Hogan

Hi,
I've hit boot problems in next-20130204 on Meta:
META213-Thread0 DSP [LogF] kobject (4fc03980): tried to init an initialized object, something is seriously wrong. META213-Thread0 DSP [LogF]
META213-Thread0 DSP [LogF] Call trace:
META213-Thread0 DSP [LogF] [<4000888c>] _show_stack+0x68/0x7c
META213-Thread0 DSP [LogF] [<400088b4>] _dump_stack+0x14/0x28
META213-Thread0 DSP [LogF] [<40103794>] _kobject_init+0x58/0x9c
META213-Thread0 DSP [LogF] [<40103810>] _kobject_create+0x38/0x64
META213-Thread0 DSP [LogF] [<40103eac>] _kobject_create_and_add+0x14/0x8c
META213-Thread0 DSP [LogF] [<40190ac4>] _mnt_init+0xd8/0x220
META213-Thread0 DSP [LogF] [<40190508>] _vfs_caches_init+0xb0/0x160

...

Quoted 11 lines

I've bisected it to the following commit:
commit 95a05b428cc675694321c8f762591984f3fd2b1e
Author: Christoph Lameter <[email protected]>
Date:   Thu Jan 10 19:14:19 2013 +0000
slab: Common constants for kmalloc boundaries
Standardize the constants that describe the smallest and largest object kept in the kmalloc arrays for SLAB and SLUB.

Quoted 6 lines

Differentiate between the maximum size for which a slab cache is used (KMALLOC_MAX_CACHE_SIZE) and the maximum allocatable size (KMALLOC_MAX_SIZE, KMALLOC_MAX_ORDER).
Signed-off-by Christoph Lameter <[email protected]>
Signed-off-by Pekka Enberg <[email protected]>

I see the same problem on ARM. I believe it's because of the changes to the calculation of MALLOC_SHIFT_LOW.

The old code was:

#if defined(ARCH_DMA_MINALIGN) && ARCH_DMA_MINALIGN > 8 #define KMALLOC_MIN_SIZE ARCH_DMA_MINALIGN #else
#ifdef CONFIG_SLAB
#define KMALLOC_MIN_SIZE 32
#else
#define KMALLOC_MIN_SIZE 8
#endif
#endif

#define KMALLOC_SHIFT_LOW ilog2(KMALLOC_MIN_SIZE)

Here, KMALLOC_SHIFT_LOW and KMALLOC_MIN_SIZE are always consistent/related.

The new code is:

#define KMALLOC_SHIFT_LOW 5
...
#if defined(ARCH_DMA_MINALIGN) && ARCH_DMA_MINALIGN > 8 #define KMALLOC_MIN_SIZE ARCH_DMA_MINALIGN #else
#define KMALLOC_MIN_SIZE (1 << KMALLOC_SHIFT_LOW) #endif

Here, if defined(ARCH_DMA_MINALIGN), then KMALLOC_MIN_SIZE isn't relative-to/derived-from KMALLOC_SHIFT_LOW, so the two may become inconsistent.

On my ARM system at least, CONFIG_ARM_L1_CACHE_SHIFT_6 is set since I have an ARMv7 CPU (see arch/arm/mm/Kconfig), which causes CONFIG_ARM_L1_CACHE_SHIFT=6, then:

Quoted 3 lines

arch/arm/include/asm/cache.h:7:#define L1_CACHE_SHIFT CONFIG_ARM_L1_CACHE_SHIFT
arch/arm/include/asm/cache.h:8:#define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
arch/arm/include/asm/cache.h:17:#define ARCH_DMA_MINALIGN L1_CACHE_BYTES

... hence that case triggers.

I also see that in most parts of the patch, SLUB_PAGE_SHIFT was replaced with (KMALLOC_SHIFT_HIGH + 1), or equivalently tests were changed from < to <=:

Review

Quoted 2 lines

(quoted diff) +1 -1

-		size <= SLUB_MAX_SIZE && !(flags & SLUB_DMA)) {
+		size <= KMALLOC_MAX_CACHE_SIZE && !(flags & SLUB_DMA)) {

However, the following doesn't seem to have that adjustment:

mm/slub.c init_kmem_cache_node(struct kmem_cache_node *n) · line 2778

Quoted 10 lines

mm/slub.c +1 -1

index ba2ca53..d0f72ee 100644
@@ -2775,7 +2775,7 @@ init_kmem_cache_node(struct kmem_cache_node *n)
 static inline int alloc_kmem_cache_cpus(struct kmem_cache *s)
 {
 	BUILD_BUG_ON(PERCPU_DYNAMIC_EARLY_SIZE <
-			SLUB_PAGE_SHIFT * sizeof(struct kmem_cache_cpu));
+			KMALLOC_SHIFT_HIGH * sizeof(struct kmem_cache_cpu));

Should that also be (KMALLOC_SHIFT_HIGH + 1)?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2013-02-05 02:29 UTC #5 lore raw

Quoted 3 lines · Stephen Warren

Here, if defined(ARCH_DMA_MINALIGN), then KMALLOC_MIN_SIZE isn't relative-to/derived-from KMALLOC_SHIFT_LOW, so the two may become inconsistent.

Right. And kmalloc_index() will therefore return KMALLOC_SHIFT_LOW which will dereference a NULL pointer since only the later cache pointers are populated. KMALLOC_SHIFT_LOW needs to be set correctly.

Quoted 4 lines

Quoted 4 lines
mm/slub.c +0 -0
index ba2ca53..d0f72ee 100644

Quoted 8 lines

mm/slub.c init_kmem_cache_node(struct kmem_cache_node *n) · line 2778

Quoted 6 lines

mm/slub.c +1 -1

@@ -2775,7 +2775,7 @@ init_kmem_cache_node(struct kmem_cache_node *n)
 static inline int alloc_kmem_cache_cpus(struct kmem_cache *s)
 {
 	BUILD_BUG_ON(PERCPU_DYNAMIC_EARLY_SIZE <
-			SLUB_PAGE_SHIFT * sizeof(struct kmem_cache_cpu));
+			KMALLOC_SHIFT_HIGH * sizeof(struct kmem_cache_cpu));

Should that also be (KMALLOC_SHIFT_HIGH + 1)?

That is already a pretty fuzzy test. The nr of kmem_cache_cpu allocated is lower than KMALLOC_SHIFT_HIGH since several index positions will not be occupied.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2013-02-05 16:36 UTC #7 lore raw

OK I was able to reproduce it by setting ARCH_DMA_MINALIGN in slab.h. This patch fixes it here:

Subject: slab: Handle ARCH_DMA_MINALIGN correctly

A fixed KMALLOC_SHIFT_LOW does not work for arches with higher alignment requirements.

Determine KMALLOC_SHIFT_LOW from ARCH_DMA_MINALIGN instead.

Signed-off-by Christoph Lameter <[email protected]>

include/linux/slab.h +18 ~~-14~~

Index: linux/include/linux/slab.h
===================================================================
@@ -133,6 +133,19 @@ void kfree(const void *);
 void kzfree(const void *);
 size_t ksize(const void *);
 
+/*
+ * Some archs want to perform DMA into kmalloc caches and need a guaranteed
+ * alignment larger than the alignment of a 64-bit integer.
+ * Setting ARCH_KMALLOC_MINALIGN in arch headers allows that.
+ */
+#if defined(ARCH_DMA_MINALIGN) && ARCH_DMA_MINALIGN > 8
+#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
+#define KMALLOC_MIN_SIZE ARCH_DMA_MINALIGN
+#define KMALLOC_SHIFT_LOW ilog2(ARCH_DMA_MINALIGN)
+#else
+#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
+#endif
+
 #ifdef CONFIG_SLOB
 /*
  * Common fields provided in kmem_cache by all slab allocators

@@ -179,7 +192,9 @@ struct kmem_cache {
 #define KMALLOC_SHIFT_HIGH	((MAX_ORDER + PAGE_SHIFT - 1) <= 25 ? \
 				(MAX_ORDER + PAGE_SHIFT - 1) : 25)
 #define KMALLOC_SHIFT_MAX	KMALLOC_SHIFT_HIGH
+#ifndef KMALLOC_SHIFT_LOW
 #define KMALLOC_SHIFT_LOW	5
+#endif
 #else
 /*
  * SLUB allocates up to order 2 pages directly and otherwise

@@ -187,8 +202,10 @@ struct kmem_cache {
  */
 #define KMALLOC_SHIFT_HIGH	(PAGE_SHIFT + 1)
 #define KMALLOC_SHIFT_MAX	(MAX_ORDER + PAGE_SHIFT)
+#ifndef KMALLOC_SHIFT_LOW
 #define KMALLOC_SHIFT_LOW	3
 #endif
+#endif
 
 /* Maximum allocatable size */
 #define KMALLOC_MAX_SIZE	(1UL << KMALLOC_SHIFT_MAX)

@@ -200,9 +217,7 @@ struct kmem_cache {
 /*
  * Kmalloc subsystem.
  */
-#if defined(ARCH_DMA_MINALIGN) && ARCH_DMA_MINALIGN > 8
-#define KMALLOC_MIN_SIZE ARCH_DMA_MINALIGN
-#else
+#ifndef KMALLOC_MIN_SIZE
 #define KMALLOC_MIN_SIZE (1 << KMALLOC_SHIFT_LOW)
 #endif

@@ -285,17 +300,6 @@ static __always_inline int kmalloc_size(
 #endif /* !CONFIG_SLOB */
 
 /*
- * Some archs want to perform DMA into kmalloc caches and need a guaranteed
- * alignment larger than the alignment of a 64-bit integer.
- * Setting ARCH_KMALLOC_MINALIGN in arch headers allows that.
- */
-#ifdef ARCH_DMA_MINALIGN
-#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
-#else
-#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
-#endif
-
-/*
  * Setting ARCH_SLAB_MINALIGN in arch headers allows a different alignment.
  * Intended for arches that get misalignment faults even for 64 bit integer
  * aligned buffers.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

James Hogan

author

2013-02-05 17:08 UTC #8 lore raw

Quoted 4 lines · Christoph Lameter

OK I was able to reproduce it by setting ARCH_DMA_MINALIGN in slab.h. This patch fixes it here:

Quoted 8 lines

Subject: slab: Handle ARCH_DMA_MINALIGN correctly
A fixed KMALLOC_SHIFT_LOW does not work for arches with higher alignment requirements.
Determine KMALLOC_SHIFT_LOW from ARCH_DMA_MINALIGN instead.
Signed-off-by Christoph Lameter <[email protected]>

Thanks, your patch fixes it for me.

Cheers
James

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2013-02-05 18:34 UTC #9 lore raw

Quoted 7 lines · James Hogan

Quoted 6 lines · Christoph Lameter
OK I was able to reproduce it by setting ARCH_DMA_MINALIGN in slab.h. This patch fixes it here:
Subject: slab: Handle ARCH_DMA_MINALIGN correctly

Quoted 8 lines

Quoted 6 lines
A fixed KMALLOC_SHIFT_LOW does not work for arches with higher alignment requirements.
Determine KMALLOC_SHIFT_LOW from ARCH_DMA_MINALIGN instead.
Signed-off-by Christoph Lameter <[email protected]>
Thanks, your patch fixes it for me.

Ok I guess that implies a Tested-by:

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

James Hogan

author

2013-02-06 09:44 UTC #10 lore raw

Quoted 12 lines · Christoph Lameter

Quoted 10 lines · James Hogan
Quoted 9 lines · Christoph Lameter
OK I was able to reproduce it by setting ARCH_DMA_MINALIGN in slab.h. This patch fixes it here:
Subject: slab: Handle ARCH_DMA_MINALIGN correctly
A fixed KMALLOC_SHIFT_LOW does not work for arches with higher alignment requirements.

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
Determine KMALLOC_SHIFT_LOW from ARCH_DMA_MINALIGN instead.
Signed-off-by Christoph Lameter <[email protected]>
Thanks, your patch fixes it for me.
Ok I guess that implies a Tested-by:

Yep sorry, feel free to add my Tested-by: if you roll this as a separate patch.

Cheers
James

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Pekka Enberg

Applied

2013-02-06 18:33 UTC #11 lore raw

Quoted 17 lines · James Hogan

Quoted 16 lines · Christoph Lameter
Quoted 14 lines · James Hogan
Quoted 12 lines · Christoph Lameter
OK I was able to reproduce it by setting ARCH_DMA_MINALIGN in slab.h. This patch fixes it here:
Subject: slab: Handle ARCH_DMA_MINALIGN correctly
A fixed KMALLOC_SHIFT_LOW does not work for arches with higher alignment requirements.
Determine KMALLOC_SHIFT_LOW from ARCH_DMA_MINALIGN instead.
Signed-off-by Christoph Lameter <[email protected]>

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Thanks, your patch fixes it for me.
Ok I guess that implies a Tested-by:
Yep sorry, feel free to add my Tested-by: if you roll this as a separate patch.

Applied, thanks guys!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Stephen Warren

Tested-by

2013-02-05 18:28 UTC #12 lore raw

In reply to Christoph Lameter

Quoted 10 lines · Christoph Lameter

OK I was able to reproduce it by setting ARCH_DMA_MINALIGN in slab.h. This patch fixes it here:
Subject: slab: Handle ARCH_DMA_MINALIGN correctly
A fixed KMALLOC_SHIFT_LOW does not work for arches with higher alignment requirements.
Determine KMALLOC_SHIFT_LOW from ARCH_DMA_MINALIGN instead.

Tested-by Stephen Warren <[email protected]>

include/linux/slab.h void kfree(const void *); · line 143

Quoted 8 lines

include/linux/slab.h +8 -0

+/*
+ * Some archs want to perform DMA into kmalloc caches and need a guaranteed
+ * alignment larger than the alignment of a 64-bit integer.
+ * Setting ARCH_KMALLOC_MINALIGN in arch headers allows that.
+ */
+#if defined(ARCH_DMA_MINALIGN) && ARCH_DMA_MINALIGN > 8
+#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
+#define KMALLOC_MIN_SIZE ARCH_DMA_MINALIGN

I might be tempted to drop that #define of KMALLOC_MIN_SIZE ...

include/linux/slab.h void kfree(const void *);

Quoted 8 lines

include/linux/slab.h +5 -0

+#define KMALLOC_SHIFT_LOW ilog2(ARCH_DMA_MINALIGN)
+#else
+#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
+#endif
 
+#ifndef KMALLOC_MIN_SIZE
 #define KMALLOC_MIN_SIZE (1 << KMALLOC_SHIFT_LOW)
 #endif

... and simply drop the ifdef around that #define instead.

That way, KMALLOC_MIN_SIZE is always defined in one place, and derived from KMALLOC_SHIFT_LOW; the logic will just set KMALLOC_SHIFT_LOW based on the various conditions. This seems a little safer to me; fewer conditions and less code to update if anything changes.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Christoph Lameter

2013-02-05 18:52 UTC #13 lore raw

Quoted 10 lines · Stephen Warren

include/linux/slab.h void kfree(const void *); · line 143

Quoted 8 lines

include/linux/slab.h +8 -0

+/*
+ * Some archs want to perform DMA into kmalloc caches and need a guaranteed
+ * alignment larger than the alignment of a 64-bit integer.
+ * Setting ARCH_KMALLOC_MINALIGN in arch headers allows that.
+ */
+#if defined(ARCH_DMA_MINALIGN) && ARCH_DMA_MINALIGN > 8
+#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
+#define KMALLOC_MIN_SIZE ARCH_DMA_MINALIGN

I might be tempted to drop that #define of KMALLOC_MIN_SIZE ...

Initially I thought so too.

Quoted 11 lines

include/linux/slab.h void kfree(const void *);
Quoted 9 lines
include/linux/slab.h +5 -0
+#define KMALLOC_SHIFT_LOW ilog2(ARCH_DMA_MINALIGN)
+#else
+#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
+#endif
 
+#ifndef KMALLOC_MIN_SIZE
 #define KMALLOC_MIN_SIZE (1 << KMALLOC_SHIFT_LOW)
 #endif
... and simply drop the ifdef around that #define instead.

That is going to be one hell of a macro expansion.

Quoted 4 lines

That way, KMALLOC_MIN_SIZE is always defined in one place, and derived from KMALLOC_SHIFT_LOW; the logic will just set KMALLOC_SHIFT_LOW based on the various conditions. This seems a little safer to me; fewer conditions and less code to update if anything changes.

Yeah but we do an ilog2 and then reverse this back to the original number.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>