[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1227719999.3387.0.camel@localhost.localdomain>
Date: Wed, 26 Nov 2008 11:19:59 -0600
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: Russell King <rmk+lkml@....linux.org.uk>
Cc: linux-arch@...r.kernel.org,
Linux Kernel List <linux-kernel@...r.kernel.org>,
linux-mm@...ck.org
Subject: Re: [RESEND:PATCH] [ARM] clearpage: provide our own
clear_user_highpage()
On Wed, 2008-11-26 at 17:13 +0000, Russell King wrote:
> I've not had any response to this, so in liu of any response by this
> coming weekend, I'm going to assume that everyone's happy with this
> change (at which point it's going to become buried under a lot of
> merges with other trees.)
>
> ----- Forwarded message from Russell King <rmk+lkml@....linux.org.uk> -----
>
> Date: Thu, 20 Nov 2008 17:50:17 +0000
> From: Russell King <rmk+lkml@....linux.org.uk>
> To: linux-arch@...r.kernel.org,
> Linux Kernel List <linux-kernel@...r.kernel.org>,
> linux-mm@...ck.org
> Subject: [PATCH] [ARM] clearpage: provide our own clear_user_highpage()
>
> This patch is part of a larger ARM specific patch set cleaning up
> aliasing VIPT cache support.
>
> With aliasing VIPT cache support, our implementation of clear_user_page()
> and copy_user_page() sets up a temporary kernel space mapping such that
> we have the same cache colour as the userspace page. This avoids having
> to consider any userspace aliases from this operation.
>
> However, when highmem is enabled, kmap_atomic() have to setup mappings.
> The copy_user_highpage() and clear_user_highpage() call these functions
> before delegating the copies to copy_user_page() and clear_user_page().
>
> The effect of this is that each of the *_user_highpage() functions setup
> their own kmap mapping, followed by the *_user_page() functions setting
> up another mapping. This is rather wasteful.
>
> Thankfully, copy_user_highpage() can be overriden by architectures by
> defining __HAVE_ARCH_COPY_USER_HIGHPAGE. However, replacement of
> clear_user_highpage() is more difficult because its inline definition
> is not conditional. It seems that you're expected to define
> __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE and provide a replacement
> __alloc_zeroed_user_highpage() implementation instead.
>
> The allocation itself is fine, so we don't want to override that. What
> we really want to do is to override clear_user_highpage() with our own
> version which doesn't kmap_atomic() unnecessarily.
>
> However, there are two drivers (drivers/media/video/videobuf-dma-sg.c
> and drivers/staging/go7007/go7007-v4l2.c) which want to provide non-
> highmem clear_user_page()'d pages to userspace.
>
> Requiring an architecture to provide __alloc_zeroed_user_highpage(),
> a sub-optimal clear_user_page(), and keep the sub-optimal
> clear_user_highpage() around seems rather silly and potentially
> error prone.
>
> So, what this patch below does is allow clear_user_highpage() itself
> to be overriden by architectures, so that they can provide just one
> implementation.
>
> What needs to follow on from this is converting those two drivers to
> use clear_user_highpage() instead of clear_user_page() - that should
> be a trivial patch.
>
> Are there any objections to this approach? Can I get any acked-by's
> from any MM folk for the include/linux/highmem.h change?
We'd like to pull this trick on parisc as well (another VIPT
architecture), so you can add my ack.
James
--
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/
Powered by blists - more mailing lists