Re: + pg_uncached-is-ia64-only.patch added to -mm tree

Re: + pg_uncached-is-ia64-only.patch added to -mm tree

[KAMEZAWA Hiroyuki]

Hi, Andrew

On Thu, 06 Apr 2006 21:20:26 -0700
akpm@osdl.org wrote:

> 
> The patch titled
> 
>      PG_uncached is ia64 only
> 
> has been added to the -mm tree.  Its filename is
> 
>      pg_uncached-is-ia64-only.patch
> 
> See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
> out what to do about this
> 

in include/linux/mmzone.h
==
#elif BITS_PER_LONG == 64
/*
 * with 64 bit flags field, there's plenty of room.
 */
#define FLAGS_RESERVED          32

#else
==

it looks this is used here.

#if SECTIONS_WIDTH+NODES_WIDTH+ZONES_WIDTH > FLAGS_RESERVED
#error SECTIONS_WIDTH+NODES_WIDTH+ZONES_WIDTH > FLAGS_RESERVED
#endif

I'm not sure but please compile check FLAGS_RESRVED with SPARSEMEM or

just 

#if (BITS_PER_LONG > 32)               /* 64-bit only flags. we can use full 
                                          low 32bits */
#define PG_uncached	31
#endif

Hm..Is this  ugly ? :(

-Kame


> 
> From: Andrew Morton <akpm@osdl.org>
> 
> As Nick points out, only ia64 uses PG_uncached.  So we can push it up into the
> higher 32-bits of page->flgs and make room for another flag on 32-bit
> machines.
> 
> Cc: "Luck, Tony" <tony.luck@intel.com>
> Cc: Jesse Barnes <jbarnes@sgi.com>
> Cc: Jes Sorensen <jes@trained-monkey.org>
> Cc: Nick Piggin <nickpiggin@yahoo.com.au>
> Signed-off-by: Andrew Morton <akpm@osdl.org>
> ---
> 
>  include/linux/page-flags.h |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff -puN include/linux/page-flags.h~pg_uncached-is-ia64-only include/linux/page-flags.h
> --- devel/include/linux/page-flags.h~pg_uncached-is-ia64-only	2006-04-06 21:17:16.000000000 -0700
> +++ devel-akpm/include/linux/page-flags.h	2006-04-06 21:18:31.000000000 -0700
> @@ -7,6 +7,8 @@
>  
>  #include <linux/percpu.h>
>  #include <linux/cache.h>
> +
> +#include <asm/types.h>
>  #include <asm/pgtable.h>
>  
>  /*
> @@ -86,7 +88,10 @@
>  #define PG_mappedtodisk		16	/* Has blocks allocated on-disk */
>  #define PG_reclaim		17	/* To be reclaimed asap */
>  #define PG_nosave_free		18	/* Free, should not be written */
> -#define PG_uncached		19	/* Page has been mapped as uncached */
> +
> +#if (BITS_PER_LONG > 32)
> +#define PG_uncached		32	/* Page has been mapped as uncached */
> +#endif
>  
>  /*
>   * Global page accounting.  One instance per CPU.  Only unsigned longs are
> _
> 
> Patches currently in -mm which might be from akpm@osdl.org are
> 
> select-warning-fixes.patch
> config_net=n-build-fix.patch
> git-acpi.patch
> acpi-update-asus_acpi-driver-registration-fix.patch
> acpi-memory-hotplug-cannot-manage-_crs-with-plural-resoureces.patch
> catch-notification-of-memory-add-event-of-acpi-via-container-driver-register-start-func-for-memory-device.patch
> catch-notification-of-memory-add-event-of-acpi-via-container-driveravoid-redundant-call-add_memory.patch
> sony_apci-resume.patch
> powernow-k8-crash-workaround.patch
> git-drm.patch
> bt866-build-fix.patch
> connector-exports.patch
> git-ia64.patch
> git-libata-all.patch
> pci-error-recovery-e1000-network-device-driver.patch
> pcmcia-remove-unneeded-forward-declarations.patch
> git-scsi-misc.patch
> megaraid-unused-variable.patch
> git-sas-jg.patch
> git-sas-jg-build-hack.patch
> git-watchdog.patch
> arm-add_memory-build-fix.patch
> acx1xx-wireless-driver.patch
> ext3-ext3-in-kernel-block-number-type-fixes-fix.patch
> sync_file_range-use-unsigned-for-flags.patch
> timer-initialisation-fix.patch
> timer-initialisation-fix-tidy.patch
> s3c24xx-gpio-led-support-tidy.patch
> make-tty_insert_flip_string_flags-a-non-gpl-export.patch
> prune_one_dentry-tweaks.patch
> sys_kexec_load-naming-fixups.patch
> hangcheck-remove-monotomic_clock-on-x86.patch
> knfsd-nfsd4-limit-number-of-delegations-handed-out-fix.patch
> pi-futex-futex-code-cleanups-fix.patch
> reiser4.patch
> kgdb-core-lite-add-reboot-command.patch
> kgdb-8250-fix.patch
> nr_blockdev_pages-in_interrupt-warning.patch
> device-suspend-debug.patch
> revert-tty-buffering-comment-out-debug-code.patch
> slab-leaks3-default-y.patch
> x86-kmap_atomic-debugging.patch
> pg_uncached-is-ia64-only.patch
> 
> -
> To unsubscribe from this list: send the line "unsubscribe mm-commits" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: + pg_uncached-is-ia64-only.patch added to -mm tree

[Andrew Morton]

KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>
> Hi, Andrew
> 
> On Thu, 06 Apr 2006 21:20:26 -0700
> akpm@osdl.org wrote:
> 
> > 
> > The patch titled
> > 
> >      PG_uncached is ia64 only
> > 
> > has been added to the -mm tree.  Its filename is
> > 
> >      pg_uncached-is-ia64-only.patch
> > 
> > See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
> > out what to do about this
> > 
> 
> in include/linux/mmzone.h
> ==
> #elif BITS_PER_LONG == 64
> /*
>  * with 64 bit flags field, there's plenty of room.
>  */
> #define FLAGS_RESERVED          32
> 
> #else

OK.

> 
> it looks this is used here.
> 
> #if SECTIONS_WIDTH+NODES_WIDTH+ZONES_WIDTH > FLAGS_RESERVED
> #error SECTIONS_WIDTH+NODES_WIDTH+ZONES_WIDTH > FLAGS_RESERVED
> #endif
> 
> I'm not sure but please compile check FLAGS_RESRVED with SPARSEMEM or
> 

Yes, that test won't trigger.

> 
> #if (BITS_PER_LONG > 32)               /* 64-bit only flags. we can use full 
>                                           low 32bits */
> #define PG_uncached	31
> #endif
> 
> Hm..Is this  ugly ? :(

It's easier to change FLAGS_RESERVED ;)

diff -puN include/linux/page-flags.h~pg_uncached-is-ia64-only include/linux/page-flags.h
--- devel/include/linux/page-flags.h~pg_uncached-is-ia64-only	2006-04-06 21:50:51.000000000 -0700
+++ devel-akpm/include/linux/page-flags.h	2006-04-06 21:50:51.000000000 -0700
@@ -7,6 +7,8 @@
 
 #include <linux/percpu.h>
 #include <linux/cache.h>
+#include <linux/types.h>
+
 #include <asm/pgtable.h>
 
 /*
@@ -86,7 +88,10 @@
 #define PG_mappedtodisk		16	/* Has blocks allocated on-disk */
 #define PG_reclaim		17	/* To be reclaimed asap */
 #define PG_nosave_free		18	/* Free, should not be written */
-#define PG_uncached		19	/* Page has been mapped as uncached */
+
+#if (BITS_PER_LONG > 32)
+#define PG_uncached		32	/* Page has been mapped as uncached */
+#endif
 
 /*
  * Global page accounting.  One instance per CPU.  Only unsigned longs are
diff -puN include/linux/mmzone.h~pg_uncached-is-ia64-only include/linux/mmzone.h
--- devel/include/linux/mmzone.h~pg_uncached-is-ia64-only	2006-04-06 21:50:56.000000000 -0700
+++ devel-akpm/include/linux/mmzone.h	2006-04-06 21:51:12.000000000 -0700
@@ -457,7 +457,7 @@ extern struct zone *next_zone(struct zon
 /*
  * with 64 bit flags field, there's plenty of room.
  */
-#define FLAGS_RESERVED		32
+#define FLAGS_RESERVED		24
 
 #else
 
_

Re: + pg_uncached-is-ia64-only.patch added to -mm tree

[David S. Miller]

From: Andrew Morton <akpm@osdl.org>
Date: Thu, 6 Apr 2006 21:52:42 -0700

> It's easier to change FLAGS_RESERVED ;)

It reminds me that I need to do something about the fact that
Sparc64 makes use of bits 24-->32 currently to record the cpu
that performs cpu stores into and thus dirties the D-cache for
a pagecache page.

Re: + pg_uncached-is-ia64-only.patch added to -mm tree

[KAMEZAWA Hiroyuki]

On Thu, 6 Apr 2006 21:52:42 -0700
Andrew Morton <akpm@osdl.org> wrote:

> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>
> -#define FLAGS_RESERVED		32
> +#define FLAGS_RESERVED		24
>
  
Oh..we get more 8bits flags on 64bit machines !!!
but could you reserve 30bits now ?

This is my understanding..
==
At the first look, SPARSEMEM with ia64

#define SECTION_SIZE_BITS       (30)
#define MAX_PHYSMEM_BITS        (50)

so SECTIONS_SHIFT = 50 - 30 = 20bits.

20bits of page->flags is used for SPARSEMEM's section id.

sgi have NODES_SHIFT=8, and always ZONES_SHIFT=2 .

At worst, 20 + 8 + 2 = 30bits should be used. but..

extra code is here..
==
#if SECTIONS_WIDTH+ZONES_WIDTH+NODES_SHIFT <= FLAGS_RESERVED
#define NODES_WIDTH             NODES_SHIFT
#else
#define NODES_WIDTH             0
#endif
==
So, node-id is not encoded into flags.
In this case, 
==
<snip>
#define FLAGS_HAS_NODE          (NODES_WIDTH > 0 || NODES_SHIFT == 0)

<snip>
static inline unsigned long page_to_nid(struct page *page)
{
        if (FLAGS_HAS_NODE)
                return (page->flags >> NODES_PGSHIFT) & NODES_MASK;
        else
                return page_zone(page)->zone_pgdat->node_id;
}
==
page_zone(page) looks up zone from zone_table[].
If FLAGS_HAS_NODE=0, zone_table indexing is this.
==

(page->flags >> (offset of zone bit)) & ((1 << (SECTIONS_SHIFT + ZONES_SHIFT)) - 1)

See above, SECTIONS_SHIFT + ZONES_SHIFT = 22bits.

so, many ia64 people has to use 32M(22bits * 8bytes) zone_table[] ???
please fix if I'm wrong ...this is complicated.

-Kame

Re: + pg_uncached-is-ia64-only.patch added to -mm tree

[Andy Whitcroft]

Andrew Morton wrote:
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> 
>>Hi, Andrew
>>
>>On Thu, 06 Apr 2006 21:20:26 -0700
>>akpm@osdl.org wrote:
>>
>>
>>>The patch titled
>>>
>>>     PG_uncached is ia64 only
>>>
>>>has been added to the -mm tree.  Its filename is
>>>
>>>     pg_uncached-is-ia64-only.patch
>>>
>>>See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
>>>out what to do about this
>>>
>>
>>in include/linux/mmzone.h
>>==
>>#elif BITS_PER_LONG == 64
>>/*
>> * with 64 bit flags field, there's plenty of room.
>> */
>>#define FLAGS_RESERVED          32
>>
>>#else
> 
> 
> OK.
> 
> 
>>it looks this is used here.
>>
>>#if SECTIONS_WIDTH+NODES_WIDTH+ZONES_WIDTH > FLAGS_RESERVED
>>#error SECTIONS_WIDTH+NODES_WIDTH+ZONES_WIDTH > FLAGS_RESERVED
>>#endif
>>
>>I'm not sure but please compile check FLAGS_RESRVED with SPARSEMEM or
>>
> 
> 
> Yes, that test won't trigger.
> 
> 
>>#if (BITS_PER_LONG > 32)               /* 64-bit only flags. we can use full 
>>                                          low 32bits */
>>#define PG_uncached	31
>>#endif
>>
>>Hm..Is this  ugly ? :(
> 
> 
> It's easier to change FLAGS_RESERVED ;)
> 
> diff -puN include/linux/page-flags.h~pg_uncached-is-ia64-only include/linux/page-flags.h
> --- devel/include/linux/page-flags.h~pg_uncached-is-ia64-only	2006-04-06 21:50:51.000000000 -0700
> +++ devel-akpm/include/linux/page-flags.h	2006-04-06 21:50:51.000000000 -0700
> @@ -7,6 +7,8 @@
>  
>  #include <linux/percpu.h>
>  #include <linux/cache.h>
> +#include <linux/types.h>
> +
>  #include <asm/pgtable.h>
>  
>  /*
> @@ -86,7 +88,10 @@
>  #define PG_mappedtodisk		16	/* Has blocks allocated on-disk */
>  #define PG_reclaim		17	/* To be reclaimed asap */
>  #define PG_nosave_free		18	/* Free, should not be written */
> -#define PG_uncached		19	/* Page has been mapped as uncached */
> +
> +#if (BITS_PER_LONG > 32)
> +#define PG_uncached		32	/* Page has been mapped as uncached */
> +#endif

As Hiroyuki-san points out we can need up to 30 bits to encode large 64
bit machines right now.  Reducing the space available for FIELDS but
reducing FLAGS_RESERVED for 64 bit machines will negativly impact them
when SPARSEMEM is enabled.  I think it makes much more sense here to use
the bits which have been released by the movement of the FIELDS upwards
in the 64 bit case.

32 bit  -------------------------------| FIELDS |       FLAGS         |
64 bit  |           FIELDS             | ??????         FLAGS         |
        63                            32                              0

Logically we should in the general case have FLAGS_RESERVED in 64 bit be
the value for 32 bit + 32; currently 9 + 32.  If we desire to have 64
bit only flags then it seems keeping FLAGS_RESERVED at 32 for 64 bit
would leave '32 bit FIELDS' segment free for those flags.

In short with the current values of FLAGS_RESERVED I would think
starting at 31 working downwards towards the 'common'/32 bit flags would
be the most logical.

Cheers.

-apw