Re: [PATCH 3/3] page-flags: rectify forward declaration

[Andrew Morton <akpm@linux-foundation.org>]

On Sat, 19 Sep 2015 22:42:59 +0530 Sudip Mukherjee <sudipm.mukherjee@gmail.com> wrote:

> compound_head is defined as inline in page-flags.h but in the forward
> declaration of compound_head in the same file missed "inline". As a result
> we got plenty of build warnings while building for some architecture
> like avr32. The warning showed as:
> warning: 'compound_head' declared inline after being called.
> warning: previous declaration of 'compound_head' was here
> 
> ...
>
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -227,7 +227,7 @@ static inline int __TestClearPage##uname(struct page *page) { return 0; }
>  struct page;
>  static inline int PageCompound(struct page *page);
>  static inline int PageTail(struct page *page);
> -static struct page *compound_head(struct page *page);
> +static inline struct page *compound_head(struct page *page);
>  
>  __PAGEFLAG(Locked, locked, PF_NO_TAIL)
>  PAGEFLAG(Error, error, PF_NO_COMPOUND) TESTCLEARFLAG(Error, error, PF_NO_COMPOUND)

Yes, that's an error, in -mm due to Kirill's page-flags patches.


The code is effectively doing

static inline XXX foo(...);

static inline YYY bar(...)
{
	foo(...);
}

inline XXX foo(...)
{
	...
}

ie: asking gcc to inline a forward-defined function.  That does work,
but it's unusual and unexpected, and it's a bit unwise to expect the
compiler to do unusual and more difficult things.

Is it fixable?  Can we use the traditional define-before-using structure?

Also, I'm finding that the patch series introduces a pretty large
bisection hole:

include/linux/page-flags.h: In function 'PageYoung':
include/linux/page-flags.h:327: error: implicit declaration of function 'PF_ANY'
include/linux/page-flags.h:327: error: invalid type argument of '->' (have 'int')
include/linux/page-flags.h:327: error: invalid type argument of '->' (have 'int')

which later gets fixed up by
page-flags-rectify-forward-declaration.patch.

Maybe it's time to do a wholesale refactoring of the patchset?