AGP and PAT (induced?) problem (on AMD family 6) - Kernel

This is a discussion on AGP and PAT (induced?) problem (on AMD family 6) - Kernel ; On 20-08-08 21:41, Venki Pallipadi wrote: > OK. I have reproduced this list size issue locally and this order 1 > allocation and set_memory_uc on that allocation is actually coming > from agp_allocate_memory() -> agp_generic_alloc_page() -> > map_page_into_agp() agp_allocate_memory breaks ...

+ Reply to Thread
Page 2 of 3 FirstFirst 1 2 3 LastLast
Results 21 to 40 of 46

Thread: AGP and PAT (induced?) problem (on AMD family 6)

  1. Re: AGP and PAT (induced?) problem (on AMD family 6)

    On 20-08-08 21:41, Venki Pallipadi wrote:

    > OK. I have reproduced this list size issue locally and this order 1
    > allocation and set_memory_uc on that allocation is actually coming
    > from agp_allocate_memory() -> agp_generic_alloc_page() ->
    > map_page_into_agp() agp_allocate_memory breaks higher order page
    > requests into order 1 allocs.
    >
    > On my system I see multiple agp_allocate_memory requests for nrpages
    > 8841, 1020, 16, 2160, 2160, 8192. Together they end up resulting in
    > more than 22K entries in PAT pages.


    Okay, thanks for the confirmation.

    Now, how to fix...

    Firstly, it seems we can conclude that any expectancy of a short PAT
    list is simply destroyed by AGP. I believe the best thing migh be to
    look into "fixing" AGP rather than PAT for now?

    In a sense the entire purpose of the AGP GART is collecting non
    contiguous pages but given that in practice it's generally still just
    one or at most a few regions, going to multi-page allocs sounds most
    appetising to me.

    All in tree AGP drivers except sgi-agp use agp_generic_alloc_page(), ali
    via m1541_alloc_page and i460 via i460_alloc_page.

    Rene.
    --
    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/

  2. Re: AGP and PAT (induced?) problem (on AMD family 6)

    On Wed, Aug 20, 2008 at 02:46:49PM -0700, Dave Airlie wrote:
    > On Thu, Aug 21, 2008 at 7:40 AM, Rene Herman wrote:
    > > On 20-08-08 21:41, Venki Pallipadi wrote:
    > >
    > >> OK. I have reproduced this list size issue locally and this order 1
    > >> allocation and set_memory_uc on that allocation is actually coming
    > >> from agp_allocate_memory() -> agp_generic_alloc_page() ->
    > >> map_page_into_agp() agp_allocate_memory breaks higher order page
    > >> requests into order 1 allocs.
    > >>
    > >> On my system I see multiple agp_allocate_memory requests for nrpages 8841,
    > >> 1020, 16, 2160, 2160, 8192. Together they end up resulting in more than 22K
    > >> entries in PAT pages.

    > >
    > > Okay, thanks for the confirmation.
    > >
    > > Now, how to fix...
    > >
    > > Firstly, it seems we can conclude that any expectancy of a short PAT list is
    > > simply destroyed by AGP. I believe the best thing migh be to look into
    > > "fixing" AGP rather than PAT for now?
    > >
    > > In a sense the entire purpose of the AGP GART is collecting non contiguous
    > > pages but given that in practice it's generally still just one or at most a
    > > few regions, going to multi-page allocs sounds most appetising to me.
    > >
    > > All in tree AGP drivers except sgi-agp use agp_generic_alloc_page(), ali via
    > > m1541_alloc_page and i460 via i460_alloc_page.

    >
    > In the future we will be getting more smaller AGP allocs, so the other
    > problem needs a fix as well.
    >
    > http://git.kernel.org/?p=linux/kerne...=agp-pageattr2
    >
    > contains some code I started on before that moves the interfaces
    > around, Shaohua has been looking at
    > it as it needs the changes to the set_pages interface as well, which
    > is where I ran out of time/steam last time.
    >
    > However with alloc/free pages we could change to a higher order
    > allocation function as long as it fell back to lower
    > orders internally.
    >


    Yes. Atleast during the bootup, we should be able to get higher order pages.
    And if we cannot get that, agp can internally fall back to zero order pages.
    That will reduce the number of times set_memory_* gets called, even without
    pat.

    We are also looking at changing the reserve_memtype in PAT, not to use linked
    list for RAM backed pages and track them in page struct. That way we will be
    using the list only for reserved region which should still be less in
    number and agp set_memory_* call will not have the list overhead.

    BTW, Rene: Did the patch from yday, caching the last list add, help in
    eliminating the slowdown in X startup?

    Thanks,
    Venki

    --
    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/

  3. Re: AGP and PAT (induced?) problem (on AMD family 6)

    Venki Pallipadi writes:
    >
    > We are also looking at changing the reserve_memtype in PAT, not to use linked
    > list for RAM backed pages and track them in page struct.


    Back when I hacked on this I explicitely chose to not do this because
    it would make it impossible to put any normal anonymous pages into
    the PAT list. While that's not done today there's no reason it couldn't
    be done in the future.

    Also it doesn't fix the scalability of the data structure anyways
    (a list is a list), just saves some memory.

    -Andi
    --
    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/

  4. Re: AGP and PAT (induced?) problem (on AMD family 6)


    * Venki Pallipadi wrote:

    > BTW, Rene: Did the patch from yday, caching the last list add, help in
    > eliminating the slowdown in X startup?


    Would be nice to test tip/master - it has both that patch included and
    the latest pageattr-array API (with enablement in AGP drivers) patchset,
    done by Shaohua Li, based on Dave's original patch.

    Ingo
    --
    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/

  5. Re: AGP and PAT (induced?) problem (on AMD family 6)

    On 21-08-08 14:06, Ingo Molnar wrote:

    > * Venki Pallipadi wrote:
    >
    >> BTW, Rene: Did the patch from yday, caching the last list add, help in
    >> eliminating the slowdown in X startup?


    Yes, it does. I was reluctant to say so as I feel it works around the
    problem instead of solving it but yes, it does (free_memtype() would
    need a similar entry cache for shutdown).

    > Would be nice to test tip/master - it has both that patch included and
    > the latest pageattr-array API (with enablement in AGP drivers) patchset,
    > done by Shaohua Li, based on Dave's original patch.


    That patch by itself doesn't help any -- the new set_memory_array_uc()
    still calls reserve_memtype() for each single page in the array. To
    help, it needs to coalesce adjacent entries, which is itself easy to do
    were it not for the need to keep the list of reserved (base,end) pairs
    around for free_memtype() time (and halfway fail time).

    Also just now looked at multipage allocs in AGP but that has the same
    issue really -- if you do multipage allocs, you then need to keep the
    list of regions around for free time.

    With worst case lists of AGPSize / 4K entries, this is not nice either
    (althought worst case is veryvery much worst and unlikely and best case
    is a 1 entry list).

    Should PAT just coalesce the memtype list itself?

    Rene.

    P.S.

    The pageattr-array patch that you currently have in tip/master only
    enables it for intel-agp, not the others. The attached enables it for
    all drivers currently directly using agp_generic_alloc_page() and
    agp_generic_destroy_page() (ocal driver is amd-k7-agp).


    From c7f1e98ee8796ca2812363e88dc9ffbf9020528b Mon Sep 17 00:00:00 2001
    From: Rene Herman
    Date: Thu, 21 Aug 2008 19:06:36 +0200
    Subject: [PATCH] AGP: make AGP drivers use new generic_alloc_pages() interface

    The new agp_generic_alloc_pages() interface uses the also new
    pageattr array interface API. This makes all AGP drivers that
    up to now used generic_{alloc,destroy}_page() use it.

    Signed-off-by: Rene Herman
    ---
    drivers/char/agp/alpha-agp.c | 2 ++
    drivers/char/agp/amd-k7-agp.c | 2 ++
    drivers/char/agp/amd64-agp.c | 2 ++
    drivers/char/agp/ati-agp.c | 2 ++
    drivers/char/agp/efficeon-agp.c | 2 ++
    drivers/char/agp/hp-agp.c | 2 ++
    drivers/char/agp/i460-agp.c | 2 ++
    drivers/char/agp/nvidia-agp.c | 2 ++
    drivers/char/agp/parisc-agp.c | 2 ++
    drivers/char/agp/sis-agp.c | 2 ++
    drivers/char/agp/sworks-agp.c | 2 ++
    drivers/char/agp/uninorth-agp.c | 4 ++++
    drivers/char/agp/via-agp.c | 4 ++++
    13 files changed, 30 insertions(+), 0 deletions(-)

    diff --git a/drivers/char/agp/alpha-agp.c b/drivers/char/agp/alpha-agp.c
    index 5da89f6..5ea4da8 100644
    --- a/drivers/char/agp/alpha-agp.c
    +++ b/drivers/char/agp/alpha-agp.c
    @@ -143,7 +143,9 @@ struct agp_bridge_driver alpha_core_agp_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    };

    diff --git a/drivers/char/agp/amd-k7-agp.c b/drivers/char/agp/amd-k7-agp.c
    index e280531..603a986 100644
    --- a/drivers/char/agp/amd-k7-agp.c
    +++ b/drivers/char/agp/amd-k7-agp.c
    @@ -386,7 +386,9 @@ static const struct agp_bridge_driver amd_irongate_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    };

    diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
    index 7495c52..2812ee2 100644
    --- a/drivers/char/agp/amd64-agp.c
    +++ b/drivers/char/agp/amd64-agp.c
    @@ -224,7 +224,9 @@ static const struct agp_bridge_driver amd_8151_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    };

    diff --git a/drivers/char/agp/ati-agp.c b/drivers/char/agp/ati-agp.c
    index 6ecbcaf..ae2791b 100644
    --- a/drivers/char/agp/ati-agp.c
    +++ b/drivers/char/agp/ati-agp.c
    @@ -418,7 +418,9 @@ static const struct agp_bridge_driver ati_generic_bridge = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    };

    diff --git a/drivers/char/agp/efficeon-agp.c b/drivers/char/agp/efficeon-agp.c
    index 8ca6f26..453543a 100644
    --- a/drivers/char/agp/efficeon-agp.c
    +++ b/drivers/char/agp/efficeon-agp.c
    @@ -335,7 +335,9 @@ static const struct agp_bridge_driver efficeon_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    };

    diff --git a/drivers/char/agp/hp-agp.c b/drivers/char/agp/hp-agp.c
    index 80d7317..183ac3f 100644
    --- a/drivers/char/agp/hp-agp.c
    +++ b/drivers/char/agp/hp-agp.c
    @@ -435,7 +435,9 @@ const struct agp_bridge_driver hp_zx1_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    .cant_use_aperture = true,
    };
    diff --git a/drivers/char/agp/i460-agp.c b/drivers/char/agp/i460-agp.c
    index e587eeb..10da687 100644
    --- a/drivers/char/agp/i460-agp.c
    +++ b/drivers/char/agp/i460-agp.c
    @@ -575,7 +575,9 @@ const struct agp_bridge_driver intel_i460_driver = {
    .insert_memory = i460_insert_memory_small_io_page,
    .remove_memory = i460_remove_memory_small_io_page,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    #endif
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    diff --git a/drivers/char/agp/nvidia-agp.c b/drivers/char/agp/nvidia-agp.c
    index eaceb61..dc70d37 100644
    --- a/drivers/char/agp/nvidia-agp.c
    +++ b/drivers/char/agp/nvidia-agp.c
    @@ -312,7 +312,9 @@ static const struct agp_bridge_driver nvidia_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    };

    diff --git a/drivers/char/agp/parisc-agp.c b/drivers/char/agp/parisc-agp.c
    index 8c42dcc..f2492ec 100644
    --- a/drivers/char/agp/parisc-agp.c
    +++ b/drivers/char/agp/parisc-agp.c
    @@ -224,7 +224,9 @@ static const struct agp_bridge_driver parisc_agp_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    .cant_use_aperture = true,
    };
    diff --git a/drivers/char/agp/sis-agp.c b/drivers/char/agp/sis-agp.c
    index 2587ef9..6c3837a 100644
    --- a/drivers/char/agp/sis-agp.c
    +++ b/drivers/char/agp/sis-agp.c
    @@ -140,7 +140,9 @@ static struct agp_bridge_driver sis_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    };

    diff --git a/drivers/char/agp/sworks-agp.c b/drivers/char/agp/sworks-agp.c
    index 2fb27fe..6224df8 100644
    --- a/drivers/char/agp/sworks-agp.c
    +++ b/drivers/char/agp/sworks-agp.c
    @@ -437,7 +437,9 @@ static const struct agp_bridge_driver sworks_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    };

    diff --git a/drivers/char/agp/uninorth-agp.c b/drivers/char/agp/uninorth-agp.c
    index eef7270..2accc97 100644
    --- a/drivers/char/agp/uninorth-agp.c
    +++ b/drivers/char/agp/uninorth-agp.c
    @@ -509,7 +509,9 @@ const struct agp_bridge_driver uninorth_agp_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    .cant_use_aperture = true,
    };
    @@ -534,7 +536,9 @@ const struct agp_bridge_driver u3_agp_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_paged = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    .cant_use_aperture = true,
    .needs_scratch_page = true,
    diff --git a/drivers/char/agp/via-agp.c b/drivers/char/agp/via-agp.c
    index 7b36476..9f4d49e 100644
    --- a/drivers/char/agp/via-agp.c
    +++ b/drivers/char/agp/via-agp.c
    @@ -190,7 +190,9 @@ static const struct agp_bridge_driver via_agp3_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    };

    @@ -214,7 +216,9 @@ static const struct agp_bridge_driver via_driver = {
    .alloc_by_type = agp_generic_alloc_by_type,
    .free_by_type = agp_generic_free_by_type,
    .agp_alloc_page = agp_generic_alloc_page,
    + .agp_alloc_pages = agp_generic_alloc_pages,
    .agp_destroy_page = agp_generic_destroy_page,
    + .agp_destroy_pages = agp_generic_destroy_pages,
    .agp_type_to_mask_type = agp_generic_type_to_mask_type,
    };

    --
    1.5.5



  6. Re: AGP and PAT (induced?) problem (on AMD family 6)

    On Wed, Aug 20, 2008 at 08:42:18PM -0700, Andi Kleen wrote:
    > Venki Pallipadi writes:
    > >
    > > We are also looking at changing the reserve_memtype in PAT, not to use linked
    > > list for RAM backed pages and track them in page struct.

    >
    > Back when I hacked on this I explicitely chose to not do this because
    > it would make it impossible to put any normal anonymous pages into
    > the PAT list. While that's not done today there's no reason it couldn't
    > be done in the future.


    Andi, we are planning to add couple of page flags which will track
    the memory attribute of the page. We need to do some checks like,
    allow the memory attribute of the page to be changed, only if it is not
    mapped any where and not on free lists(like the in the X driver case,
    where they allocate the page and then change the attribute). Similarly,
    in generic -mm, we need to ensure that the page before it gets added to free
    list, has the right memory attribute etc. If the driver is exposing this
    page with special attribute, then it is drivers responsibility to
    use the same attribute across all the mappings.

    Is there a reason why this won't work with anonymous pages? Can you please
    elaborate.

    > Also it doesn't fix the scalability of the data structure anyways
    > (a list is a list), just saves some memory.


    With this, we will track only the reserved regions using the linked list
    and typically these reserved regions will be small number (may be huge
    contiguous chunks but total number of such chunks will be reasonably smaller).

    thanks,
    suresh
    --
    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/

  7. [PATCH] x86: {reverve,free}_memtype() take a physical address

    On 21-08-08 19:15, Rene Herman wrote:

    > On 21-08-08 14:06, Ingo Molnar wrote:


    >> Would be nice to test tip/master - it has both that patch included and
    >> the latest pageattr-array API (with enablement in AGP drivers)
    >> patchset, done by Shaohua Li, based on Dave's original patch.

    >
    > That patch by itself doesn't help any -- the new set_memory_array_uc()
    > still calls reserve_memtype() for each single page in the array.


    Worse yet, it appears to be broken. {reserve,free}_memtype() expect phys
    addresses but it's being passed virtual ones...

    Rene.



    From 48b9d479e149dffac24f98f9491174fdfc19d26b Mon Sep 17 00:00:00 2001
    From: Rene Herman
    Date: Thu, 21 Aug 2008 23:32:25 +0200
    Subject: [PATCH] x86: {reverve,free}_memtype() take a physical address

    The new set_memory_array_{uc,wb}() pass virtual addresses to
    {reserve,free}_memtype() it seems.

    Signed-off-by: Rene Herman
    ---
    arch/x86/mm/pageattr.c | 6 +++---
    1 files changed, 3 insertions(+), 3 deletions(-)

    diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
    index b351c4f..d49e4db 100644
    --- a/arch/x86/mm/pageattr.c
    +++ b/arch/x86/mm/pageattr.c
    @@ -947,7 +947,7 @@ int set_memory_array_uc(unsigned long *addr, int addrinarray)
    * for now UC MINUS. see comments in ioremap_nocache()
    */
    for (i = 0; i < addrinarray; i++) {
    - if (reserve_memtype(addr[i], addr[i] + PAGE_SIZE,
    + if (reserve_memtype(__pa(addr[i]), __pa(addr[i]) + PAGE_SIZE,
    _PAGE_CACHE_UC_MINUS, NULL))
    goto out;
    }
    @@ -956,7 +956,7 @@ int set_memory_array_uc(unsigned long *addr, int addrinarray)
    __pgprot(_PAGE_CACHE_UC_MINUS), 1);
    out:
    while (--i >= 0)
    - free_memtype(addr[i], addr[i] + PAGE_SIZE);
    + free_memtype(__pa(addr[i]), __pa(addr[i]) + PAGE_SIZE);
    return -EINVAL;
    }
    EXPORT_SYMBOL(set_memory_array_uc);
    @@ -998,7 +998,7 @@ int set_memory_array_wb(unsigned long *addr, int addrinarray)
    {
    int i;
    for (i = 0; i < addrinarray; i++)
    - free_memtype(addr[i], addr[i] + PAGE_SIZE);
    + free_memtype(__pa(addr[i]), __pa(addr[i]) + PAGE_SIZE);

    return change_page_attr_clear(addr, addrinarray,
    __pgprot(_PAGE_CACHE_MASK), 1);
    --
    1.5.5



  8. RE: [PATCH] x86: {reverve,free}_memtype() take a physical address


    >-----Original Message-----
    >From: Rene Herman [mailto:rene.herman@keyaccess.nl]
    >Sent: Thursday, August 21, 2008 3:10 PM
    >To: Ingo Molnar; Li, Shaohua
    >Cc: Pallipadi, Venkatesh; Dave Airlie; Yinghai Lu; Andreas
    >Herrmann; Arjan van de Ven; Linux Kernel; Siddha, Suresh B;
    >Thomas Gleixner; H. Peter Anvin; Dave Jones
    >Subject: [PATCH] x86: {reverve,free}_memtype() take a physical address
    >
    >On 21-08-08 19:15, Rene Herman wrote:
    >
    >> On 21-08-08 14:06, Ingo Molnar wrote:

    >
    >>> Would be nice to test tip/master - it has both that patch

    >included and
    >>> the latest pageattr-array API (with enablement in AGP drivers)
    >>> patchset, done by Shaohua Li, based on Dave's original patch.

    >>
    >> That patch by itself doesn't help any -- the new

    >set_memory_array_uc()
    >> still calls reserve_memtype() for each single page in the array.

    >
    >Worse yet, it appears to be broken. {reserve,free}_memtype()
    >expect phys
    >addresses but it's being passed virtual ones...
    >
    >Rene.
    >


    Yes. Noticed that too and sent a patch here for x86/tip.

    http://www.ussg.iu.edu/hypermail/lin...08.2/2270.html

    It is not very critical as it sounds as only set_memory_uc sets PAT
    bits for RAM pages. Most other users (devmem mmap, ioramep, pci)
    set PAT bits on the reserved memory. And there will not be conflicts
    across RAM and reserveed regions. Regardless, this was a stupid
    bug that we had missed earlier.

    Thanks,
    Venki
    --
    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/

  9. Re: [PATCH] x86: {reverve,free}_memtype() take a physical address

    On 22-08-08 00:16, Pallipadi, Venkatesh wrote:

    > Yes. Noticed that too and sent a patch here for x86/tip.
    >
    > http://www.ussg.iu.edu/hypermail/lin...08.2/2270.html
    >
    > It is not very critical as it sounds as only set_memory_uc sets PAT
    > bits for RAM pages. Most other users (devmem mmap, ioramep, pci)
    > set PAT bits on the reserved memory. And there will not be conflicts
    > across RAM and reserveed regions. Regardless, this was a stupid
    > bug that we had missed earlier.


    And unfortunately I don't think the above fully fixes it for AGP. __pa()
    gets the real physical address and the memtypes should be on the GART
    remapped physical addresses it seems.

    Rene.
    --
    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/

  10. RE: [PATCH] x86: {reverve,free}_memtype() take a physical address



    >-----Original Message-----
    >From: Rene Herman [mailto:rene.herman@keyaccess.nl]
    >Sent: Thursday, August 21, 2008 3:27 PM
    >To: Pallipadi, Venkatesh
    >Cc: Ingo Molnar; Li, Shaohua; Dave Airlie; Yinghai Lu; Andreas
    >Herrmann; Arjan van de Ven; Linux Kernel; Siddha, Suresh B;
    >Thomas Gleixner; H. Peter Anvin; Dave Jones
    >Subject: Re: [PATCH] x86: {reverve,free}_memtype() take a
    >physical address
    >
    >On 22-08-08 00:16, Pallipadi, Venkatesh wrote:
    >
    >> Yes. Noticed that too and sent a patch here for x86/tip.
    >>
    >> http://www.ussg.iu.edu/hypermail/lin...08.2/2270.html
    >>
    >> It is not very critical as it sounds as only set_memory_uc sets PAT
    >> bits for RAM pages. Most other users (devmem mmap, ioramep, pci)
    >> set PAT bits on the reserved memory. And there will not be conflicts
    >> across RAM and reserveed regions. Regardless, this was a stupid
    >> bug that we had missed earlier.

    >
    >And unfortunately I don't think the above fully fixes it for
    >AGP. __pa()
    >gets the real physical address and the memtypes should be on the GART
    >remapped physical addresses it seems.
    >


    Page being marked here as uncached is the page got from alloc_page().
    We are not really marking GART physical address as uncacheable. And
    that page returned from alloc_page is what we are tracking with
    reserve and free.
    IOW, the tracking is only to keep CPU accesses consistent across
    different va->pa and va across different CPUs and has nothing to do
    with GART physical address here.

    Thanks,
    Venki
    --
    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/

  11. [PATCH] x86: have set_memory_array_{uc,wb} coalesce memtypes.

    On 21-08-08 19:15, Rene Herman wrote:

    > On 21-08-08 14:06, Ingo Molnar wrote:


    >> Would be nice to test tip/master - it has both that patch included and
    >> the latest pageattr-array API (with enablement in AGP drivers)
    >> patchset, done by Shaohua Li, based on Dave's original patch.

    >
    > That patch by itself doesn't help any -- the new set_memory_array_uc()
    > still calls reserve_memtype() for each single page in the array. To
    > help, it needs to coalesce adjacent entries, which is itself easy to do
    > were it not for the need to keep the list of reserved (base,end) pairs
    > around for free_memtype() time (and halfway fail time).


    Actually, might as well simply reconstruct the memtype list at free time
    I guess. How is this for a coalescing version of the array functions?

    NOTE: I am posting this because I'm going to bed but haven't stared
    comfortably long at this and might be buggy. Compiles, boots and
    provides me with:

    root@7ixe4:~# wc -l /debug/x86/pat_memtype_list
    53 /debug/x86/pat_memtype_list

    otherwise (down from 16384+).



    Rene.

    From b5dc6e481b38cf4e7792bcb9a8f5dd9aab0e5590 Mon Sep 17 00:00:00 2001
    From: Rene Herman
    Date: Fri, 22 Aug 2008 00:56:00 +0200
    Subject: [PATCH] x86: have set_memory_array_{uc,wb} coalesce memtypes.

    ---
    arch/x86/mm/pageattr.c | 38 ++++++++++++++++++++++++++++++++------
    1 files changed, 32 insertions(+), 6 deletions(-)

    diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
    index d49e4db..a2a497a 100644
    --- a/arch/x86/mm/pageattr.c
    +++ b/arch/x86/mm/pageattr.c
    @@ -942,21 +942,38 @@ EXPORT_SYMBOL(set_memory_uc);

    int set_memory_array_uc(unsigned long *addr, int addrinarray)
    {
    + unsigned long start;
    + unsigned long end;
    int i;
    /*
    * for now UC MINUS. see comments in ioremap_nocache()
    */
    for (i = 0; i < addrinarray; i++) {
    - if (reserve_memtype(__pa(addr[i]), __pa(addr[i]) + PAGE_SIZE,
    - _PAGE_CACHE_UC_MINUS, NULL))
    + start = __pa(addr[i]);
    + for (end = start + PAGE_SIZE; i < addrinarray - 1; end += PAGE_SIZE) {
    + if (end != __pa(addr[i + 1]))
    + break;
    + i++;
    + }
    + if (reserve_memtype(start, end, _PAGE_CACHE_UC_MINUS, NULL))
    goto out;
    }

    return change_page_attr_set(addr, addrinarray,
    __pgprot(_PAGE_CACHE_UC_MINUS), 1);
    out:
    - while (--i >= 0)
    - free_memtype(__pa(addr[i]), __pa(addr[i]) + PAGE_SIZE);
    + for (i = 0; i < addrinarray; i++) {
    + unsigned long tmp = __pa(addr[i]);
    +
    + if (tmp == start)
    + break;
    + for (end = start + PAGE_SIZE; i < addrinarray - 1; end += PAGE_SIZE) {
    + if (end != __pa(addr[i + 1]))
    + break;
    + i++;
    + }
    + free_memtype(tmp, end);
    + }
    return -EINVAL;
    }
    EXPORT_SYMBOL(set_memory_array_uc);
    @@ -997,9 +1014,18 @@ EXPORT_SYMBOL(set_memory_wb);
    int set_memory_array_wb(unsigned long *addr, int addrinarray)
    {
    int i;
    - for (i = 0; i < addrinarray; i++)
    - free_memtype(__pa(addr[i]), __pa(addr[i]) + PAGE_SIZE);

    + for (i = 0; i < addrinarray; i++) {
    + unsigned long start = __pa(addr[i]);
    + unsigned long end;
    +
    + for (end = start + PAGE_SIZE; i < addrinarray - 1; end += PAGE_SIZE) {
    + if (end != __pa(addr[i + 1]))
    + break;
    + i++;
    + }
    + free_memtype(start, end);
    + }
    return change_page_attr_clear(addr, addrinarray,
    __pgprot(_PAGE_CACHE_MASK), 1);
    }
    --
    1.5.5



  12. Re: [PATCH] x86: {reverve,free}_memtype() take a physical address

    On 22-08-08 00:57, Pallipadi, Venkatesh wrote:
    >
    >> -----Original Message-----
    >> From: Rene Herman [mailto:rene.herman@keyaccess.nl]
    >> Sent: Thursday, August 21, 2008 3:27 PM
    >> To: Pallipadi, Venkatesh
    >> Cc: Ingo Molnar; Li, Shaohua; Dave Airlie; Yinghai Lu; Andreas
    >> Herrmann; Arjan van de Ven; Linux Kernel; Siddha, Suresh B;
    >> Thomas Gleixner; H. Peter Anvin; Dave Jones
    >> Subject: Re: [PATCH] x86: {reverve,free}_memtype() take a
    >> physical address
    >>
    >> On 22-08-08 00:16, Pallipadi, Venkatesh wrote:
    >>
    >>> Yes. Noticed that too and sent a patch here for x86/tip.
    >>>
    >>> http://www.ussg.iu.edu/hypermail/lin...08.2/2270.html
    >>>
    >>> It is not very critical as it sounds as only set_memory_uc sets PAT
    >>> bits for RAM pages. Most other users (devmem mmap, ioramep, pci)
    >>> set PAT bits on the reserved memory. And there will not be conflicts
    >>> across RAM and reserveed regions. Regardless, this was a stupid
    >>> bug that we had missed earlier.

    >> And unfortunately I don't think the above fully fixes it for
    >> AGP. __pa()
    >> gets the real physical address and the memtypes should be on the GART
    >> remapped physical addresses it seems.
    >>

    >
    > Page being marked here as uncached is the page got from alloc_page().
    > We are not really marking GART physical address as uncacheable. And
    > that page returned from alloc_page is what we are tracking with
    > reserve and free.
    > IOW, the tracking is only to keep CPU accesses consistent across
    > different va->pa and va across different CPUs and has nothing to do
    > with GART physical address here.


    Okay, if you say so... it _used_ to be before this array change to AGP
    that the GART addresses were in the memtype list, but I'll take your
    word for that being okay.

    Rene
    --
    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/

  13. Re: AGP and PAT (induced?) problem (on AMD family 6)

    > Andi, we are planning to add couple of page flags which will track

    page flags are still scarce unfortunately, at least on 32bit.
    It's a little better now than it used to be (at some point
    they were nearly out before some were reclaimed), but adding a lot
    of flags is still difficult.

    In interest of full disclosure I need at least two for other
    work too.

    On 64bit adding a lot of new flags is fine, but 64bit only
    solutions are not good in this case.

    > the memory attribute of the page. We need to do some checks like,
    > allow the memory attribute of the page to be changed, only if it is not
    > mapped any where and not on free lists(like the in the X driver case,
    > where they allocate the page and then change the attribute). Similarly,
    > in generic -mm, we need to ensure that the page before it gets added to free
    > list, has the right memory attribute etc.


    You want to handle that in __free_pages?

    I would have thought that should be handled in some higher level
    function which could just check the memattr.

    If the driver is exposing this
    > page with special attribute, then it is drivers responsibility to
    > use the same attribute across all the mappings.
    >
    > Is there a reason why this won't work with anonymous pages? Can you please
    > elaborate.


    The issue is just if you reuse the two list heads in struct page
    because they're already used by the

    Adding flags does not conflict here of course.

    > > Also it doesn't fix the scalability of the data structure anyways
    > > (a list is a list), just saves some memory.

    >
    > With this, we will track only the reserved regions using the linked list
    > and typically these reserved regions will be small number (may be huge
    > contiguous chunks but total number of such chunks will be reasonably smaller).


    Reserved region defined how exactly?

    -Andi
    --
    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/

  14. Re: [PATCH] x86: have set_memory_array_{uc,wb} coalesce memtypes.


    * Rene Herman wrote:

    > Actually, might as well simply reconstruct the memtype list at free
    > time I guess. How is this for a coalescing version of the array
    > functions?


    impressive! Rarely do we get this much bang for such a low linecount :-)

    > NOTE: I am posting this because I'm going to bed but haven't stared
    > comfortably long at this and might be buggy. Compiles, boots and
    > provides me with:
    >
    > root@7ixe4:~# wc -l /debug/x86/pat_memtype_list
    > 53 /debug/x86/pat_memtype_list
    >
    > otherwise (down from 16384+).
    >
    >


    cool!

    I'd do this in v2.6.27 but i forced myself to be reasonable and applied
    your patches to tip/x86/pat instead, for tentative v2.6.28 merging
    (assuming it all passes testing, etc.):

    # 9a79f4f: x86: {reverve,free}_memtype() take a physical address
    # c5e147c: x86: have set_memory_array_{uc,wb} coalesce memtypes.
    # 5f310b6: agp: enable optimized agp_alloc_pages methods

    ( note that i flipped them around a bit and have put your
    enable-agp_alloc_pages()-widely patch last, so that we get better
    bisection behavior. )

    The frontside cache itself is in x86/urgent:

    # 80c5e73: x86: fix Xorg startup/shutdown slowdown with PAT

    .... and should at least solve the symptom that you've hit in practice
    (the slowdown), without changing the underlying PAT machinery. (which
    would be way too dangerous for v2.6.27)

    And it's all merged up in tip/master, you might want to test that too to
    check whether all the pieces fit together nicely.

    Tens of thousands of page granular memtypes was Not Nice.

    Venki, Suresh, Shaohua, Dave, Arjan - any observations about this line
    of action?

    Ingo
    --
    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/

  15. Re: [PATCH] x86: have set_memory_array_{uc,wb} coalesce memtypes.

    On Thu, Aug 21, 2008 at 09:15:44PM -0700, Ingo Molnar wrote:
    >
    > * Rene Herman wrote:
    >
    > > Actually, might as well simply reconstruct the memtype list at free
    > > time I guess. How is this for a coalescing version of the array
    > > functions?

    >
    > impressive! Rarely do we get this much bang for such a low linecount :-)
    >
    > > NOTE: I am posting this because I'm going to bed but haven't stared
    > > comfortably long at this and might be buggy. Compiles, boots and
    > > provides me with:
    > >
    > > root@7ixe4:~# wc -l /debug/x86/pat_memtype_list
    > > 53 /debug/x86/pat_memtype_list
    > >
    > > otherwise (down from 16384+).
    > >
    > >

    >
    > cool!
    >
    > I'd do this in v2.6.27 but i forced myself to be reasonable and applied
    > your patches to tip/x86/pat instead, for tentative v2.6.28 merging
    > (assuming it all passes testing, etc.):
    >
    > # 9a79f4f: x86: {reverve,free}_memtype() take a physical address
    > # c5e147c: x86: have set_memory_array_{uc,wb} coalesce memtypes.
    > # 5f310b6: agp: enable optimized agp_alloc_pages methods
    >
    > ( note that i flipped them around a bit and have put your
    > enable-agp_alloc_pages()-widely patch last, so that we get better
    > bisection behavior. )
    >
    > The frontside cache itself is in x86/urgent:
    >
    > # 80c5e73: x86: fix Xorg startup/shutdown slowdown with PAT
    >
    > ... and should at least solve the symptom that you've hit in practice
    > (the slowdown), without changing the underlying PAT machinery. (which
    > would be way too dangerous for v2.6.27)
    >
    > And it's all merged up in tip/master, you might want to test that too to
    > check whether all the pieces fit together nicely.
    >
    > Tens of thousands of page granular memtypes was Not Nice.
    >
    > Venki, Suresh, Shaohua, Dave, Arjan - any observations about this line
    > of action?


    The concern I have here is that the coalescing is not guaranteed to work.
    We may still end up having horrible worst case latency, even though this
    improves the normal case (boot the system, start X, exit X, reboot the system).
    It depends on how pages are allocated and how much memory is there in the
    system and what else is running etc.

    Here on my test system, without this coalescing change I see

    [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    19528

    With the coalescing change I see
    [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    135

    quit and restart X
    [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    985
    quit and restart X
    [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    1468
    quit and restart X
    [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    1749
    quit and restart X
    [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    1916
    quit and restart X
    [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    2085

    untar a kernel tar.bz2 and restart X
    [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    2737

    compile the kernel and restart X
    [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    3089


    I think this as a good workaround for now. But, for long run we still need to
    look at other ways of eliminating this overhead (like using page struct
    that Suresh mentioned in the other thread).


    Also, there seems to be a bug in the error path of the patch. Below should
    fix it.

    Thanks,
    Venki

    Fix the start addr for free_memtype calls in the error path.

    Signed-off-by: Venkatesh Pallipadi

    ---
    arch/x86/mm/pageattr.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    Index: t/arch/x86/mm/pageattr.c
    ================================================== =================
    --- t.orig/arch/x86/mm/pageattr.c 2008-08-22 10:38:35.000000000 -0700
    +++ t/arch/x86/mm/pageattr.c 2008-08-22 11:27:27.000000000 -0700
    @@ -967,7 +967,7 @@ out:

    if (tmp == start)
    break;
    - for (end = start + PAGE_SIZE; i < addrinarray - 1; end += PAGE_SIZE) {
    + for (end = tmp + PAGE_SIZE; i < addrinarray - 1; end += PAGE_SIZE) {
    if (end != __pa(addr[i + 1]))
    break;
    i++;
    --
    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/

  16. [PATCH] x86: have set_memory_array_{uc,wb} coalesce memtypes.

    On 22-08-08 06:15, Ingo Molnar wrote:

    >> Actually, might as well simply reconstruct the memtype list at free
    >> time I guess. How is this for a coalescing version of the array
    >> functions?

    >
    > impressive! Rarely do we get this much bang for such a low linecount :-)


    Thanks. Stared at it a little longer now and there was a small cut and
    paste error in the error path (s/start/tmp/) but other than that, yes,
    I'll stand by this. set_memory_array_{uc,wb}() set all pages to the same
    type, so coalescing them makes sense in any usage case it seems.

    The attached version fixes the out path, is otherwise identical and this
    time comes with a proper changelog and a sign-off. Given that you needed
    the changelog and the sign-off anyway, I thought there wouldn't be much
    point in doing that incrementally, but if you disagree and refactor -- a
    changelog for the out path fix would be a simple:

    ===
    x86: fix "have set_memory_array_{uc,wb} coalesce memtypes".

    Fix copy and paste error in out path

    Signed-off-by: Rene Herman
    ===

    > I'd do this in v2.6.27 but i forced myself to be reasonable and applied
    > your patches to tip/x86/pat instead, for tentative v2.6.28 merging
    > (assuming it all passes testing, etc.):


    Yes, I agree not for .27

    > # 9a79f4f: x86: {reverve,free}_memtype() take a physical address
    > # c5e147c: x86: have set_memory_array_{uc,wb} coalesce memtypes.
    > # 5f310b6: agp: enable optimized agp_alloc_pages methods
    >
    > ( note that i flipped them around a bit and have put your
    > enable-agp_alloc_pages()-widely patch last, so that we get better
    > bisection behavior. )
    >
    > The frontside cache itself is in x86/urgent:
    >
    > # 80c5e73: x86: fix Xorg startup/shutdown slowdown with PAT
    >
    > ... and should at least solve the symptom that you've hit in practice
    > (the slowdown), without changing the underlying PAT machinery. (which
    > would be way too dangerous for v2.6.27)


    Well, please note that that specific commit only fixes X startup -- it
    doesn't do anything for shutdown. With only that one, I'm still at 14
    seconds for X shutdown (first time after boot that is, 5 seconds
    subsequent shutdowns) versus 1 (or sub 1, feels immediate) normally.

    It's also a black-screen "hang", so we'll probably be getting a lot of
    "long hang at shutdown" reports without something additionally for .27.

    Venki?

    > And it's all merged up in tip/master, you might want to test that too to
    > check whether all the pieces fit together nicely.


    Wasn't in tip/master when I just now fetched it. It does indeed sit in
    tip/x86/pat.

    Rene.

    From 1a9bfbada3769e1bd9eecddd43ade9ebc4671c3d Mon Sep 17 00:00:00 2001
    From: Rene Herman
    Date: Fri, 22 Aug 2008 21:27:45 +0200
    Subject: [PATCH] x86: have set_memory_array_{uc,wb} coalesce memtypes.

    Have set_memory_array_{uc,wb}() coalesce memtype entries so as to
    avoid having unusefully many of them.

    Especially in the case of AGP with its order 0 allocations for the
    AGP memory the memtype list otherwise ends up with tens of thousands
    of entries, slowing processing to a crawl. With this (and the former
    changes to make AGP use this interface) AGP memory will just take as
    many entries as needed -- which often means just one.

    Note that the error path in set_memory_array_uc() just reconstructs
    the entries from start again: no need to keep an expensive list of
    regions around for an error condition which isn't going to happen.

    Signed-off-by: Rene Herman
    ---
    arch/x86/mm/pageattr.c | 38 ++++++++++++++++++++++++++++++++------
    1 files changed, 32 insertions(+), 6 deletions(-)

    diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
    index d49e4db..c7563b6 100644
    --- a/arch/x86/mm/pageattr.c
    +++ b/arch/x86/mm/pageattr.c
    @@ -942,21 +942,38 @@ EXPORT_SYMBOL(set_memory_uc);

    int set_memory_array_uc(unsigned long *addr, int addrinarray)
    {
    + unsigned long start;
    + unsigned long end;
    int i;
    /*
    * for now UC MINUS. see comments in ioremap_nocache()
    */
    for (i = 0; i < addrinarray; i++) {
    - if (reserve_memtype(__pa(addr[i]), __pa(addr[i]) + PAGE_SIZE,
    - _PAGE_CACHE_UC_MINUS, NULL))
    + start = __pa(addr[i]);
    + for (end = start + PAGE_SIZE; i < addrinarray - 1; end += PAGE_SIZE) {
    + if (end != __pa(addr[i + 1]))
    + break;
    + i++;
    + }
    + if (reserve_memtype(start, end, _PAGE_CACHE_UC_MINUS, NULL))
    goto out;
    }

    return change_page_attr_set(addr, addrinarray,
    __pgprot(_PAGE_CACHE_UC_MINUS), 1);
    out:
    - while (--i >= 0)
    - free_memtype(__pa(addr[i]), __pa(addr[i]) + PAGE_SIZE);
    + for (i = 0; i < addrinarray; i++) {
    + unsigned long tmp = __pa(addr[i]);
    +
    + if (tmp == start)
    + break;
    + for (end = tmp + PAGE_SIZE; i < addrinarray - 1; end += PAGE_SIZE) {
    + if (end != __pa(addr[i + 1]))
    + break;
    + i++;
    + }
    + free_memtype(tmp, end);
    + }
    return -EINVAL;
    }
    EXPORT_SYMBOL(set_memory_array_uc);
    @@ -997,9 +1014,18 @@ EXPORT_SYMBOL(set_memory_wb);
    int set_memory_array_wb(unsigned long *addr, int addrinarray)
    {
    int i;
    - for (i = 0; i < addrinarray; i++)
    - free_memtype(__pa(addr[i]), __pa(addr[i]) + PAGE_SIZE);

    + for (i = 0; i < addrinarray; i++) {
    + unsigned long start = __pa(addr[i]);
    + unsigned long end;
    +
    + for (end = start + PAGE_SIZE; i < addrinarray - 1; end += PAGE_SIZE) {
    + if (end != __pa(addr[i + 1]))
    + break;
    + i++;
    + }
    + free_memtype(start, end);
    + }
    return change_page_attr_clear(addr, addrinarray,
    __pgprot(_PAGE_CACHE_MASK), 1);
    }
    --
    1.5.5



  17. Re: [PATCH] x86: have set_memory_array_{uc,wb} coalesce memtypes.

    On 22-08-08 21:08, Venki Pallipadi wrote:

    > On Thu, Aug 21, 2008 at 09:15:44PM -0700, Ingo Molnar wrote:


    >> Venki, Suresh, Shaohua, Dave, Arjan - any observations about this
    >> line of action?

    >
    > The concern I have here is that the coalescing is not guaranteed to
    > work. We may still end up having horrible worst case latency, even
    > though this improves the normal case (boot the system, start X, exit
    > X, reboot the system). It depends on how pages are allocated and how
    > much memory is there in the system and what else is running etc.


    Yes, I agree. Independent of the current trigger PAT wants a more
    scalable approach and yes, worst case is still single page entries.

    That worst case is the guaranteed case now though, so I do feel it's a
    generic fix. After all, there wouldn't seem to be a reason to _not_
    coalesce in set_memory_array_{uc,wb}().

    > Here on my test system, without this coalescing change I see
    >
    > [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    > 19528
    >
    > With the coalescing change I see
    > [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    > 135
    >
    > quit and restart X
    > [root@localhost ~]# cat /proc/sys/debug/x86/pat_memtype_list | wc -l
    > 985


    [ constantly growing number of entries ]

    Yes, absolutely right, PAT definitely needs something other than the
    simple linked list. I do believe we also want the coalescing change
    though - it seems to make sense regardless of trigger and it's only
    little code.

    > I think this as a good workaround for now. But, for long run we still need to
    > look at other ways of eliminating this overhead (like using page struct
    > that Suresh mentioned in the other thread).
    >
    >
    > Also, there seems to be a bug in the error path of the patch. Below should
    > fix it.


    Ah, yes, thanks, just sent out a final version with this fixed as well.

    Rene.

    --
    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/

  18. Re: [PATCH] x86: have set_memory_array_{uc,wb} coalesce memtypes.


    * Rene Herman wrote:

    >> Also, there seems to be a bug in the error path of the patch. Below
    >> should fix it.

    >
    > Ah, yes, thanks, just sent out a final version with this fixed as
    > well.


    applied to tip/x86/pat, thanks.

    Ingo
    --
    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/

  19. AGP PAT issue.

    On 22-08-08 22:02, Rene Herman wrote:

    > On 22-08-08 06:15, Ingo Molnar wrote:


    >> The frontside cache itself is in x86/urgent:
    >>
    >> # 80c5e73: x86: fix Xorg startup/shutdown slowdown with PAT
    >>
    >> ... and should at least solve the symptom that you've hit in practice
    >> (the slowdown), without changing the underlying PAT machinery. (which
    >> would be way too dangerous for v2.6.27)

    >
    > Well, please note that that specific commit only fixes X startup -- it
    > doesn't do anything for shutdown. With only that one, I'm still at 14
    > seconds for X shutdown (first time after boot that is, 5 seconds
    > subsequent shutdowns) versus 1 (or sub 1, feels immediate) normally.
    >
    > It's also a black-screen "hang", so we'll probably be getting a lot of
    > "long hang at shutdown" reports without something additionally for .27.
    >
    > Venki?


    Haven't been subcribed to any lists recently and someone was talking
    about "the other thread" before but just noticed that an -rc6 was cut.

    Please note that the shutdown issue remains unfixed in it (I'm doing my
    coalescing changes locally).

    Rene
    --
    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/

  20. Re: AGP PAT issue.


    * Rene Herman wrote:

    > On 22-08-08 22:02, Rene Herman wrote:
    >
    >> On 22-08-08 06:15, Ingo Molnar wrote:

    >
    >>> The frontside cache itself is in x86/urgent:
    >>>
    >>> # 80c5e73: x86: fix Xorg startup/shutdown slowdown with PAT
    >>>
    >>> ... and should at least solve the symptom that you've hit in practice
    >>> (the slowdown), without changing the underlying PAT machinery. (which
    >>> would be way too dangerous for v2.6.27)

    >>
    >> Well, please note that that specific commit only fixes X startup -- it
    >> doesn't do anything for shutdown. With only that one, I'm still at 14
    >> seconds for X shutdown (first time after boot that is, 5 seconds
    >> subsequent shutdowns) versus 1 (or sub 1, feels immediate) normally.
    >>
    >> It's also a black-screen "hang", so we'll probably be getting a lot of
    >> "long hang at shutdown" reports without something additionally for .27.
    >>
    >> Venki?

    >
    > Haven't been subcribed to any lists recently and someone was talking
    > about "the other thread" before but just noticed that an -rc6 was cut.
    >
    > Please note that the shutdown issue remains unfixed in it (I'm doing
    > my coalescing changes locally).


    here's what is pending in tip/x86/pat for v2.6.28:

    110e035: x86: make sure the CPA test code's use of _PAGE_UNUSED1 is obvious
    c09ff7e: linux-next: fix x86 tree build failure
    01de05a: x86: have set_memory_array_{uc,wb} coalesce memtypes, fix
    5f310b6: agp: enable optimized agp_alloc_pages methods
    c5e147c: x86: have set_memory_array_{uc,wb} coalesce memtypes.
    9a79f4f: x86: {reverve,free}_memtype() take a physical address
    8b53b57: Merge branch 'x86/urgent' into x86/pat
    ab7e792: x86: fix pageattr-test
    bd07928: agp: add agp_generic_destroy_pages()
    37acee1: agp: generic_alloc_pages()
    d75586a: x86, pageattr: introduce APIs to change pageattr of a page array
    cacf890: Revert "introduce two APIs for page attribute"
    9326d61: Revert "reduce tlb/cache flush times of agpgart memory allocation"
    5843d9a: x86, pat: avoid highmem cache attribute aliasing
    466ae83: reduce tlb/cache flush times of agpgart memory allocation
    1ac2f7d: introduce two APIs for page attribute
    012f09e: x86: compile pat debugfs interface only if CONFIG_X86_PAT is set

    that includes your coalescing optimizations and fixes. None of that is
    really v2.6.27 material i think.

    Btw., if you track -tip as per:

    http://people.redhat.com/mingo/tip.git/README

    you can merge these changes alone into latest -git just by doing:

    git merge tip/x86/pat

    or you can use tip/master for the full range of pending features and
    fixes.

    Ingo
    --
    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/

+ Reply to Thread
Page 2 of 3 FirstFirst 1 2 3 LastLast