lkml.org 
[lkml]   [2006]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
Date
From
SubjectRe: [BUG] 2.6.18-rc1-mm1: the simple way to reproduce it
On Wed, 12 Jul 2006 13:27:01 +0400
Paul Drynoff <pauldrynoff@gmail.com> wrote:

> I found that bug can be simply reproducible.
> if take
> gcc-3.3.6 or gcc-4.1.1
> qemu - 0.8.1
>
> $qemu-img create test.img 1M
> $cd linux-2.6.18-rc1-mm1
> $qemu -kernel arch/i386/boot/bzImage -hda test.img

That helps heaps, thanks.

Bisection shows that the crash (which seems to be a BUG) is due to
reduce-max_nr_zones-*.patch, and testing on the current -mm lineup
indicates that one of the two patches which Christoph sent
post-2.6.18-rc1-mm1 has fixed it.


From: Christoph Lameter <clameter@sgi.com>

We sometimes do comparisons like

xx & (__GFP_x| __GFP_y)) == GFP_y

This becomes a problem if GFP_y is redefined to be zero.

Both __GFP_HIGHMEM (caused by me) and __GFP_DMA32 (was this way before) can
become zero in order to cause a fall back to ZONE_NORMAL.

We also have a GFP_MASK that is applied before indexing into the nodelists
array. We can use that mask to avoid having both become zero and instead
use the GFP_MASK to clear the respective bits before retrieving the
zonelist.

- Remove #ifdefs that cause __GFP_HIGHMEM or __GFP_DMA32 to become zero.

- GFP_MASK works nicely to cause fall back to ZONE_NORMAL except if
CONFIG_ZONE_DMA32 and !CONFIG_HIGHMEM are set. In that case the
fallback to ZONE_NORMAL does not work. So add a special case
to mmzone.h to define a special mask that switches off the highmem
bit for this particular configuration.

Signed-off-by: Christoph Lameter <clameter@sgi.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

include/linux/gfp.h | 9 +--------
include/linux/mmzone.h | 21 ++++++++++++++++++---
2 files changed, 19 insertions(+), 11 deletions(-)

diff -puN include/linux/gfp.h~reduce-max_nr_zones-make-zone_highmem-optional-fix include/linux/gfp.h
--- a/include/linux/gfp.h~reduce-max_nr_zones-make-zone_highmem-optional-fix
+++ a/include/linux/gfp.h
@@ -12,17 +12,10 @@ struct vm_area_struct;
*/
/* Zone modifiers in GFP_ZONEMASK (see linux/mmzone.h - low three bits) */
#define __GFP_DMA ((__force gfp_t)0x01u)
-
-#ifdef CONFIG_HIGHMEM
#define __GFP_HIGHMEM ((__force gfp_t)0x02u)
-#else
-#define __GFP_HIGHMEM ((__force gfp_t)0x00) /* NORMAL is HIGHMEM */
-#endif

-#ifndef CONFIG_ZONE_DMA32
+#if !defined(CONFIG_ZONE_DMA32) && BITS_PER_LONG >= 64
#define __GFP_DMA32 ((__force gfp_t)0x01) /* ZONE_DMA is ZONE_DMA32 */
-#elif BITS_PER_LONG < 64
-#define __GFP_DMA32 ((__force gfp_t)0x00) /* ZONE_NORMAL is ZONE_DMA32 */
#else
#define __GFP_DMA32 ((__force gfp_t)0x04) /* Has own ZONE_DMA32 */
#endif
diff -puN include/linux/mmzone.h~reduce-max_nr_zones-make-zone_highmem-optional-fix include/linux/mmzone.h
--- a/include/linux/mmzone.h~reduce-max_nr_zones-make-zone_highmem-optional-fix
+++ a/include/linux/mmzone.h
@@ -162,18 +162,33 @@ enum zone_type {
*
* NOTE! Make sure this matches the zones in <linux/gfp.h>
*/
-#define GFP_ZONETYPES ((GFP_ZONEMASK + 1) / 2 + 1) /* Loner */

#ifdef CONFIG_ZONE_DMA32
+
+#ifdef CONFIG_HIGHMEM
+#define GFP_ZONETYPES ((GFP_ZONEMASK + 1) / 2 + 1) /* Loner */
#define GFP_ZONEMASK 0x07
-#define ZONES_SHIFT 2 /* ceil(log2(MAX_NR_ZONES)) */
+#define ZONES_SHIFT 2 /* ceil(log2(MAX_NR_ZONES)) */
+#else
+#define GFP_ZONETYPES ((0x07 + 1) / 2 + 1) /* Loner */
+/* Mask __GFP_HIGHMEM */
+#define GFP_ZONEMASK 0x05
+#define ZONES_SHIFT 2
+#endif
+
#else
#ifdef CONFIG_HIGHMEM
+
#define GFP_ZONEMASK 0x03
-#define ZONES_SHIFT 2
+#define ZONES_SHIFT 2
+#define GFP_ZONETYPES 3
+
#else
+
#define GFP_ZONEMASK 0x01
#define ZONES_SHIFT 1
+#define GFP_ZONETYPES 2
+
#endif
#endif

_


From: Christoph Lameter <clameter@sgi.com>

The __GFP_xx definitions in include/linux/gfp.h are used to refer to
individual bits in the GFP bitmasks. I think they should not be
conditional via ifdef. If a conditional definition is required then a
version without the underscore should be used.

My patch from yesterday cleared up the definitions that resulted in zero
values. This one removes the #ifdef around __GFP_DMA32.

The x86_64 arch code is the only user of __GFP_DMA32 and x86_64 sets
CONFIG_ZONE_DMA32. No driver is using DMA32. So we do not really need
fall back behavior for ZONE_DMA32 at this point.

If ZONE_DMA32 would be used by a device driver in the future on a platform
that has not set CONFIG_ZONE_DMA32 then we will fall back to ZONE_NORMAL
because the __GFP_DMA32 bit is not set in GFP_ZONEMASK. I think if
different behavior is desired then the device driver would have to take
care of falling back to a different allocation. The absence of DMA32 slab
support may already force the device driver to deal with this case anyways.
Another solution would be to make the definition of GFP_DMA32 conditional
on CONFIG_ZONE_DMA32 and 32/64 bit. That would leave the __GFP_xx
untouched.

Signed-off-by: Christoph Lameter <clameter@sgi.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
include/linux/gfp.h | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff -puN include/linux/gfp.h~reduce-max_nr_zones-make-zone_highmem-optional-fix-fix include/linux/gfp.h
--- a/include/linux/gfp.h~reduce-max_nr_zones-make-zone_highmem-optional-fix-fix
+++ a/include/linux/gfp.h
@@ -9,16 +9,19 @@ struct vm_area_struct;

/*
* GFP bitmasks..
+ *
+ * Zone modifiers (see linux/mmzone.h - low three bits)
+ *
+ * These may be masked by GFP_ZONEMASK to make allocations with this bit
+ * set fall back to ZONE_NORMAL.
+ *
+ * Do not put any conditional on these. If necessary modify the definitions
+ * without the underscores and use the consistently. The definitions here may
+ * be used in bit comparisons.
*/
-/* Zone modifiers in GFP_ZONEMASK (see linux/mmzone.h - low three bits) */
#define __GFP_DMA ((__force gfp_t)0x01u)
#define __GFP_HIGHMEM ((__force gfp_t)0x02u)
-
-#if !defined(CONFIG_ZONE_DMA32) && BITS_PER_LONG >= 64
-#define __GFP_DMA32 ((__force gfp_t)0x01) /* ZONE_DMA is ZONE_DMA32 */
-#else
-#define __GFP_DMA32 ((__force gfp_t)0x04) /* Has own ZONE_DMA32 */
-#endif
+#define __GFP_DMA32 ((__force gfp_t)0x04u)

/*
* Action modifiers - doesn't change the zoning
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-07-13 05:53    [W:0.107 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site