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

From: Andrew Morton
Date: Mon Sep 21 2015 - 18:35:24 EST


On Sat, 19 Sep 2015 22:42:59 +0530 Sudip Mukherjee <sudipm.mukherjee@xxxxxxxxx> 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?
--
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/