Re: [PATCH 1/2] Export get_user_pages_fast

[Adrian Bunk]

On Wed, Jul 30, 2008 at 10:14:38AM -0700, Andrew Morton wrote:
> On Wed, 30 Jul 2008 20:09:39 +0300 Adrian Bunk <bunk@xxxxxxxxxx> wrote:
> 
> > On Wed, Jul 30, 2008 at 10:03:55AM -0700, Andrew Morton wrote:
> > > On Wed, 30 Jul 2008 18:47:36 +0300 Adrian Bunk <bunk@xxxxxxxxxx> wrote:
> > > 
> > > > On Wed, Jul 30, 2008 at 03:35:23AM -0700, Andrew Morton wrote:
> > > > > On Wed, 30 Jul 2008 14:38:55 +1000 Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote:
> > > > > 
> > > > > > After all, it was made for lguest wasn't it?  Still think it should be
> > > > > > called get_current_pages() though.
> > > > > > 
> > > > > > Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> > > > > > ---
> > > > > >  arch/x86/mm/gup.c |    2 ++
> > > > > >  1 file changed, 2 insertions(+)
> > > > > > 
> > > > > > diff -r cb465381f6d5 arch/x86/mm/gup.c
> > > > > > --- a/arch/x86/mm/gup.c	Wed Jul 30 10:18:44 2008 +1000
> > > > > > +++ b/arch/x86/mm/gup.c	Wed Jul 30 14:22:53 2008 +1000
> > > > > > @@ -8,6 +8,7 @@
> > > > > >  #include <linux/mm.h>
> > > > > >  #include <linux/vmstat.h>
> > > > > >  #include <linux/highmem.h>
> > > > > > +#include <linux/module.h>
> > > > > >  
> > > > > >  #include <asm/pgtable.h>
> > > > > >  
> > > > > > @@ -296,3 +303,4 @@ slow_irqon:
> > > > > >  		return ret;
> > > > > >  	}
> > > > > >  }
> > > > > > +EXPORT_SYMBOL_GPL(get_user_pages_fast);
> > > > > 
> > > > > A regularly-occurring problem with an export like this is that someone
> > > > > writes a driver and tests it on x86, then the driver explodes on an
> > > > > architecture which didn't export the same symbol.
> > > > > 
> > > > > So a better implementation might be to put
> > > > > 
> > > > > #ifdef CONFIG_HAVE_GET_USER_PAGES_FAST
> > > > > EXPORT_SYMBOL_GPL(get_user_pages_fast);
> > > > > #endif
> > > > > 
> > > > > into generic code somewhere.
> > > > 
> > > > How would that help?
> > > 
> > > It would fix the problem which I described.
> > 
> > The export would still only be available on x86 since no other arch 
> > would define CONFIG_HAVE_GET_USER_PAGES_FAST.
> > 
> 
> That's presently true in Linus mainline.  The powerpc implementation is
> in -mm, but apprarently needs a bit more work.

I still don't get the advantage of your suggestion.

With the powerpc implementation included your suggestion doesn't change 
the fact that the export is available only on x86 and powerpc, and if a 
driver author mistakenly uses it and only tests it's driver on x86 it 
will still break on architectures other than x86 or powerpc.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/