Re: [PATCH 02/33] mm: overload get_user_pages() functions

[Ingo Molnar]

* Dave Hansen <dave@xxxxxxxx> wrote:

> 
> From: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> 
> The concept here was a suggestion from Ingo.  The implementation
> horrors are all mine.
> 
> This allows get_user_pages(), get_user_pages_unlocked(), and
> get_user_pages_locked() to be called with or without the
> leading tsk/mm arguments.  We will give a compile-time warning
> about the old style being __deprecated and we will also
> WARN_ON() if the non-remote version is used for a remote-style
> access.

So at minimum this should be WARN_ON_ONCE(), to make it easier to recover some 
meaningful kernel log from such incidents.

But:

> Doing this, folks will get nice warnings and will not break the
> build.  This should be nice for -next and will hopefully let
> developers fix up their own code instead of maintainers needing
> to do it at merge time.
> 
> The way we do this is hideous.  It uses the __VA_ARGS__ macro
> functionality to call different functions based on the number
> of arguments passed to the macro.
> 
> There's an additional hack to ensure that our EXPORT_SYMBOL()
> of the deprecated symbols doesn't trigger a warning.
> 
> We should be able to remove this mess as soon as -rc1 hits in
> the release after this is merged.

So when I suggested this then it looked a _lot_ cleanear to me, in my head!

OTOH this, if factored out a bit perhaps, could be the basis for a useful 
technical model to do 'phased in, -next invariant' prototype migrations in the 
future, especially when it involves lots of subsystems.

Strictly only in cases where -rc1 will truly get rid of the __VA_ARGS__ hackery - 
which we'd do in this case.

Nevertheless I'd love to have a high level buy-in from either Linus or Andrew that 
we can do it this way, as the hackery looks very hideous...

The alternative would be to allow the -next churn and to allow the occasional 
(fairly trivial but tester-disruptive) build breakage.

Thanks,

	Ingo