linux-kernel - Re: X-freeze after clflush changes [Was: 2.6.23-rc6-mm1]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <21d7e9970709191718q1695e96bgbd280c2590c1aebd@mail.gmail.com>
Date:	Thu, 20 Sep 2007 10:18:54 +1000
From:	"Dave Airlie" <airlied@...il.com>
To:	"Andi Kleen" <andi@...stfloor.org>
Cc:	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Jiri Slaby" <jirislaby@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: X-freeze after clflush changes [Was: 2.6.23-rc6-mm1]

On 9/20/07, Andi Kleen <andi@...stfloor.org> wrote:
> On Wed, Sep 19, 2007 at 12:10:17PM -0700, Andrew Morton wrote:
> > On Wed, 19 Sep 2007 16:59:04 +0200 Jiri Slaby <jirislaby@...il.com> wrote:
> >
> > > ---------8<---------8<---------8<---------8<---------8<---------8<----
> > > That means
> > > void agp_generic_destroy_page(void *addr)
> > > {
> > >         struct page *page;
> > >
> > >         if (addr == NULL)
> > >                 return;
> > >
> > >         page = virt_to_page(addr);
> > > (1)     unmap_page_from_agp(page);
> > >         put_page(page);
> > > (2)     free_page((unsigned long)addr);
> > >         atomic_dec(&agp_bridge->current_memory_agp);
> > > }
> > >
> > > (1) unmap_page_from_agp -> change_page_attr -> change_page_attr_addr ->
> > > __change_page_attr -> save_page -> list_add(&fpage->lru, &deferred_pages);
> > > (2) free_page -> free_pages -> __free_pages -> free_hot_page ->
> > > free_hot_cold_page -> list_add(&page->lru, &pcp->list);
> >
> > that'll hurt.
> >
> > > any ideas how to fix this?
> >
> > We should hold a single reference on the page for its membership in
> > deferred_pages.
>
> The code is broken anyways. If you free pages without flushing
> them first some other innocent user allocating them will end up
> with possible uncached pages for some time.
>
> Does this simple patch help?
>
> -Andi
>
>
> Flush uncached AGP pages before freeing

In theory this should be handled by the caller, so as to avoid the
overhead of continuous flushing however I can see a potential race
condition here if the pages are put back into the kernel before the
caller flushes the mappings..

Do we need some sort of two step approach here? as flushing after each
page would be a major overhead for dynamic agp stuff in the new memory
manager..

Dave.

>
> Signed-off-by: Andi Kleen <ak@...e.de>
>
> Index: linux/drivers/char/agp/generic.c
> ===================================================================
> --- linux.orig/drivers/char/agp/generic.c
> +++ linux/drivers/char/agp/generic.c
> @@ -1185,6 +1185,7 @@ void agp_generic_destroy_page(void *addr
>
>         page = virt_to_page(addr);
>         unmap_page_from_agp(page);
> +       flush_agp_mappings();
>         put_page(page);
>         free_page((unsigned long)addr);
>         atomic_dec(&agp_bridge->current_memory_agp);
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/