Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Wed, Dec 9, 2020 at 6:31 PM Steven Rostedt wrote:
>
> On Wed, 9 Dec 2020 17:12:43 -0800
> Alexei Starovoitov wrote:
>
> > > > > > FWIW, I intend to do some consolidation/renaming in this area. I
> > > > > > trust that will not be a problem?
> > > > >
> > > > > If it does not break anything, it will be not a problem ;-)
> > > > >
> > > > > It's possible that __add_to_page_cache_locked() can be a global symbol
> > > > > with add_to_page_cache_lru() + add_to_page_cache_locked() being just
> > > > > static/inline wrappers around it.
> > > >
> > > > So what happens to BTF if we change this area entirely? Your IDs
> > > > sound like some kind of ABI to me, which is extremely scary.
> > >
> > > Is BTF becoming the new tracepoint? That is, random additions of things
> > > like:
> > >
> > >BTF_ID(func,__add_to_page_cache_locked)
> > >
> > > Like was done in commit 1e6c62a882155 ("bpf: Introduce sleepable BPF
> > > programs") without any notification to the maintainers of the
> > > __add_to_page_cache_locked code, will suddenly become an API?
> >
> > huh? what api/abi you're talking about?
>
> If the function __add_to_page_cache_locked were to be removed due to
> the code being rewritten, would it break any user space? If not, then
> there's nothing to worry about. ;-)
That function is marked with ALLOW_ERROR_INJECTION.
So any script that exercises it via debugfs (or via bpf) will not work.
That's nothing new. Same "breakage" happens with kprobes, etc.
The function was marked with error_inject for a reason though.
The refactoring or renaming of this code ideally should provide a way to do
similar pattern of injecting errors in this code path.
It could be a completely new function, of course.
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Wed, 9 Dec 2020 17:12:43 -0800
Alexei Starovoitov wrote:
> > > > > FWIW, I intend to do some consolidation/renaming in this area. I
> > > > > trust that will not be a problem?
> > > >
> > > > If it does not break anything, it will be not a problem ;-)
> > > >
> > > > It's possible that __add_to_page_cache_locked() can be a global symbol
> > > > with add_to_page_cache_lru() + add_to_page_cache_locked() being just
> > > > static/inline wrappers around it.
> > >
> > > So what happens to BTF if we change this area entirely? Your IDs
> > > sound like some kind of ABI to me, which is extremely scary.
> >
> > Is BTF becoming the new tracepoint? That is, random additions of things
> > like:
> >
> >BTF_ID(func,__add_to_page_cache_locked)
> >
> > Like was done in commit 1e6c62a882155 ("bpf: Introduce sleepable BPF
> > programs") without any notification to the maintainers of the
> > __add_to_page_cache_locked code, will suddenly become an API?
>
> huh? what api/abi you're talking about?
If the function __add_to_page_cache_locked were to be removed due to
the code being rewritten, would it break any user space? If not, then
there's nothing to worry about. ;-)
-- Steve
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Wed, Dec 9, 2020 at 2:32 PM Steven Rostedt wrote:
>
> On Wed, Dec 09, 2020 at 06:05:52PM +, Christoph Hellwig wrote:
> > On Wed, Dec 09, 2020 at 04:51:48PM +0100, Stanislaw Gruszka wrote:
> > > On Wed, Dec 09, 2020 at 03:08:26PM +, Matthew Wilcox wrote:
> > > > On Wed, Dec 09, 2020 at 03:46:28PM +0100, Stanislaw Gruszka wrote:
> > > > > At this point of release cycle we should probably go with revert,
> > > > > but I think the main problem is that BPF and ERROR_INJECTION use
> > > > > function that is not intended to be used externally. For external
> > > > > users
> > > > > add_to_page_cache_lru() and add_to_page_cache_locked() are exported
> > > > > and I think those should be used (see the patch below).
> > > >
> > > > FWIW, I intend to do some consolidation/renaming in this area. I
> > > > trust that will not be a problem?
> > >
> > > If it does not break anything, it will be not a problem ;-)
> > >
> > > It's possible that __add_to_page_cache_locked() can be a global symbol
> > > with add_to_page_cache_lru() + add_to_page_cache_locked() being just
> > > static/inline wrappers around it.
> >
> > So what happens to BTF if we change this area entirely? Your IDs
> > sound like some kind of ABI to me, which is extremely scary.
>
> Is BTF becoming the new tracepoint? That is, random additions of things like:
>
>BTF_ID(func,__add_to_page_cache_locked)
>
> Like was done in commit 1e6c62a882155 ("bpf: Introduce sleepable BPF
> programs") without any notification to the maintainers of the
> __add_to_page_cache_locked code, will suddenly become an API?
huh? what api/abi you're talking about?
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Wed, Dec 09, 2020 at 06:05:52PM +, Christoph Hellwig wrote:
> On Wed, Dec 09, 2020 at 04:51:48PM +0100, Stanislaw Gruszka wrote:
> > On Wed, Dec 09, 2020 at 03:08:26PM +, Matthew Wilcox wrote:
> > > On Wed, Dec 09, 2020 at 03:46:28PM +0100, Stanislaw Gruszka wrote:
> > > > At this point of release cycle we should probably go with revert,
> > > > but I think the main problem is that BPF and ERROR_INJECTION use
> > > > function that is not intended to be used externally. For external users
> > > > add_to_page_cache_lru() and add_to_page_cache_locked() are exported
> > > > and I think those should be used (see the patch below).
> > >
> > > FWIW, I intend to do some consolidation/renaming in this area. I
> > > trust that will not be a problem?
> >
> > If it does not break anything, it will be not a problem ;-)
> >
> > It's possible that __add_to_page_cache_locked() can be a global symbol
> > with add_to_page_cache_lru() + add_to_page_cache_locked() being just
> > static/inline wrappers around it.
>
> So what happens to BTF if we change this area entirely? Your IDs
> sound like some kind of ABI to me, which is extremely scary.
Is BTF becoming the new tracepoint? That is, random additions of things like:
BTF_ID(func,__add_to_page_cache_locked)
Like was done in commit 1e6c62a882155 ("bpf: Introduce sleepable BPF
programs") without any notification to the maintainers of the
__add_to_page_cache_locked code, will suddenly become an API?
There's no mention in the change log to why __add_to_page_cache_locked was
added. And interesting enough, __add_to_page_cache_locked is not in any header
file, which is why it was switched to static.
-- Steve
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Mon, Dec 7, 2020 at 4:36 PM Michal Kubecek wrote:
> Not removal, commit 3351b16af494 ("mm/filemap: add static for function
> __add_to_page_cache_locked") made the function static which breaks the
> build in btfids phase - but it seems to happen only on some
> architectures. In our case, ppc64, ppc64le and riscv64 are broken,
> x86_64, i586 and s390x succeed. (I made a mistake above, aarch64 did not
> fail - but only because it was not built at all.)
x86_64 fails for me:
LD vmlinux
BTFIDS vmlinux
FAILED unresolved symbol __add_to_page_cache_locked
make: *** [Makefile:1170: vmlinux] Error 255
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Wed, Dec 09, 2020 at 04:51:48PM +0100, Stanislaw Gruszka wrote: > On Wed, Dec 09, 2020 at 03:08:26PM +, Matthew Wilcox wrote: > > On Wed, Dec 09, 2020 at 03:46:28PM +0100, Stanislaw Gruszka wrote: > > > At this point of release cycle we should probably go with revert, > > > but I think the main problem is that BPF and ERROR_INJECTION use > > > function that is not intended to be used externally. For external users > > > add_to_page_cache_lru() and add_to_page_cache_locked() are exported > > > and I think those should be used (see the patch below). > > > > FWIW, I intend to do some consolidation/renaming in this area. I > > trust that will not be a problem? > > If it does not break anything, it will be not a problem ;-) > > It's possible that __add_to_page_cache_locked() can be a global symbol > with add_to_page_cache_lru() + add_to_page_cache_locked() being just > static/inline wrappers around it. So what happens to BTF if we change this area entirely? Your IDs sound like some kind of ABI to me, which is extremely scary.
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Wed, Dec 09, 2020 at 03:08:26PM +, Matthew Wilcox wrote: > On Wed, Dec 09, 2020 at 03:46:28PM +0100, Stanislaw Gruszka wrote: > > At this point of release cycle we should probably go with revert, > > but I think the main problem is that BPF and ERROR_INJECTION use > > function that is not intended to be used externally. For external users > > add_to_page_cache_lru() and add_to_page_cache_locked() are exported > > and I think those should be used (see the patch below). > > FWIW, I intend to do some consolidation/renaming in this area. I > trust that will not be a problem? If it does not break anything, it will be not a problem ;-) It's possible that __add_to_page_cache_locked() can be a global symbol with add_to_page_cache_lru() + add_to_page_cache_locked() being just static/inline wrappers around it. Stanislaw
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Wed, Dec 09, 2020 at 03:46:28PM +0100, Stanislaw Gruszka wrote: > At this point of release cycle we should probably go with revert, > but I think the main problem is that BPF and ERROR_INJECTION use > function that is not intended to be used externally. For external users > add_to_page_cache_lru() and add_to_page_cache_locked() are exported > and I think those should be used (see the patch below). FWIW, I intend to do some consolidation/renaming in this area. I trust that will not be a problem?
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Mon, Dec 07, 2020 at 05:12:52PM -0800, Alexei Starovoitov wrote:
> > > > > > > >>> -noinline int __add_to_page_cache_locked(struct page *page,
> > > > > > > >>> +static noinline int __add_to_page_cache_locked(struct page
> > > > > > > >>> *page,
> > > > > > > >>> struct address_space
> > > > > > > >>> *mapping,
> > > > > > > >>> pgoff_t offset, gfp_t
> > > > > > > >>> gfp,
> > > > > > > >>> void **shadowp)
> > > > > > > >
> > > > > > > > It's unclear to me whether BTF_ID() requires that the target
> > > > > > > > symbol be
> > > > > > > > non-static. It doesn't actually reference the symbol:
> > > > > > > >
> > > > > > > > #define BTF_ID(prefix, name) \
> > > > > > > > __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__))
[snip]
> > > __add_to_page_cache_locked") made the function static which breaks the
> > > build in btfids phase - but it seems to happen only on some
> > > architectures. In our case, ppc64, ppc64le and riscv64 are broken,
> > > x86_64, i586 and s390x succeed. (I made a mistake above, aarch64 did not
> > > fail - but only because it was not built at all.)
> > >
> > > The thread starts with
> > > http://lkml.kernel.org/r/1604661895-5495-1-git-send-email-alex@linux.alibaba.com
I have 5.10-rc7 build failure because of this on x86_64:
BTFIDS vmlinux
FAILED unresolved symbol __add_to_page_cache_locked
make: *** [Makefile:1170: vmlinux] Error 255
> > Got it. So the above commit is wrong.
> > The addition of "static" is incorrect here.
> > Regardless of btf_id generation.
> > "static noinline" means that the error injection in that spot is unreliable.
> > Even when bpf is completely compiled out of the kernel.
>
> I finally realized that the addition of 'static' was pushed into Linus's tree
> :(
> Please revert commit 3351b16af494 ("mm/filemap: add static for
> function __add_to_page_cache_locked")
At this point of release cycle we should probably go with revert,
but I think the main problem is that BPF and ERROR_INJECTION use
function that is not intended to be used externally. For external users
add_to_page_cache_lru() and add_to_page_cache_locked() are exported
and I think those should be used (see the patch below).
Stanislaw
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1388bf733071..dd6357802504 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11487,7 +11487,8 @@ BTF_SET_START(btf_non_sleepable_error_inject)
/* Three functions below can be called from sleepable and non-sleepable
context.
* Assume non-sleepable from bpf safety point of view.
*/
-BTF_ID(func, __add_to_page_cache_locked)
+BTF_ID(func, add_to_page_cache_locked)
+BTF_ID(func, add_to_page_cache_lru)
BTF_ID(func, should_fail_alloc_page)
BTF_ID(func, should_failslab)
BTF_SET_END(btf_non_sleepable_error_inject)
diff --git a/mm/filemap.c b/mm/filemap.c
index 331f4261d723..168deec64a10 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -827,10 +827,10 @@ int replace_page_cache_page(struct page *old, struct page
*new, gfp_t gfp_mask)
}
EXPORT_SYMBOL_GPL(replace_page_cache_page);
-static noinline int __add_to_page_cache_locked(struct page *page,
- struct address_space *mapping,
- pgoff_t offset, gfp_t gfp,
- void **shadowp)
+static int __add_to_page_cache_locked(struct page *page,
+ struct address_space *mapping,
+ pgoff_t offset, gfp_t gfp,
+ void **shadowp)
{
XA_STATE(xas, >i_pages, offset);
int huge = PageHuge(page);
@@ -907,7 +907,6 @@ static noinline int __add_to_page_cache_locked(struct page
*page,
put_page(page);
return error;
}
-ALLOW_ERROR_INJECTION(__add_to_page_cache_locked, ERRNO);
/**
* add_to_page_cache_locked - add a locked page to the pagecache
@@ -928,6 +927,7 @@ int add_to_page_cache_locked(struct page *page, struct
address_space *mapping,
gfp_mask, NULL);
}
EXPORT_SYMBOL(add_to_page_cache_locked);
+ALLOW_ERROR_INJECTION(add_to_page_cache_locked, ERRNO);
int add_to_page_cache_lru(struct page *page, struct address_space *mapping,
pgoff_t offset, gfp_t gfp_mask)
@@ -957,6 +957,7 @@ int add_to_page_cache_lru(struct page *page, struct
address_space *mapping,
return ret;
}
EXPORT_SYMBOL_GPL(add_to_page_cache_lru);
+ALLOW_ERROR_INJECTION(add_to_page_cache_lru, ERRNO);
#ifdef CONFIG_NUMA
struct page *__page_cache_alloc(gfp_t gfp)
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Mon, Dec 7, 2020 at 2:59 PM Alexei Starovoitov
wrote:
>
> On Mon, Dec 7, 2020 at 2:53 PM Michal Kubecek wrote:
> >
> > On Mon, Dec 07, 2020 at 02:44:22PM -0800, Alexei Starovoitov wrote:
> > > On Mon, Dec 7, 2020 at 10:36 AM Justin Forbes
> > > wrote:
> > > >
> > > > On Mon, Dec 7, 2020 at 2:16 AM Michal Kubecek wrote:
> > > > >
> > > > > On Thu, Nov 12, 2020 at 08:18:57AM +0800, Alex Shi wrote:
> > > > > >
> > > > > >
> > > > > > 在 2020/11/11 上午3:50, Andrew Morton 写道:
> > > > > > > On Tue, 10 Nov 2020 08:39:24 +0530 Souptick Joarder
> > > > > > > wrote:
> > > > > > >
> > > > > > >> On Fri, Nov 6, 2020 at 4:55 PM Alex Shi
> > > > > > >> wrote:
> > > > > > >>>
> > > > > > >>> Otherwise it cause gcc warning:
> > > > > > >>> ^~~
> > > > > > >>> ../mm/filemap.c:830:14: warning: no previous prototype for
> > > > > > >>> ‘__add_to_page_cache_locked’ [-Wmissing-prototypes]
> > > > > > >>> noinline int __add_to_page_cache_locked(struct page *page,
> > > > > > >>> ^~
> > > > > > >>
> > > > > > >> Is CONFIG_DEBUG_INFO_BTF enabled in your .config ?
> > > > > > >
> > > > > > > hm, yes.
> > > > > >
> > > > > > When the config enabled, compiling looks good untill pahole tool
> > > > > > used to get BTF info, but I still failed on a right version pahole
> > > > > > > 1.16. Sorry.
> > > > > >
> > > > > > >
> > > > > > >>>
> > > > > > >>> Signed-off-by: Alex Shi
> > > > > > >>> Cc: Andrew Morton
> > > > > > >>> Cc: linux...@kvack.org
> > > > > > >>> Cc: linux-kernel@vger.kernel.org
> > > > > > >>> ---
> > > > > > >>> mm/filemap.c | 2 +-
> > > > > > >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >>>
> > > > > > >>> diff --git a/mm/filemap.c b/mm/filemap.c
> > > > > > >>> index d90614f501da..249cf489f5df 100644
> > > > > > >>> --- a/mm/filemap.c
> > > > > > >>> +++ b/mm/filemap.c
> > > > > > >>> @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page
> > > > > > >>> *old, struct page *new, gfp_t gfp_mask)
> > > > > > >>> }
> > > > > > >>> EXPORT_SYMBOL_GPL(replace_page_cache_page);
> > > > > > >>>
> > > > > > >>> -noinline int __add_to_page_cache_locked(struct page *page,
> > > > > > >>> +static noinline int __add_to_page_cache_locked(struct page
> > > > > > >>> *page,
> > > > > > >>> struct address_space
> > > > > > >>> *mapping,
> > > > > > >>> pgoff_t offset, gfp_t
> > > > > > >>> gfp,
> > > > > > >>> void **shadowp)
> > > > > > >
> > > > > > > It's unclear to me whether BTF_ID() requires that the target
> > > > > > > symbol be
> > > > > > > non-static. It doesn't actually reference the symbol:
> > > > > > >
> > > > > > > #define BTF_ID(prefix, name) \
> > > > > > > __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__))
> > > > > > >
> > > > > >
> > > > > > The above usage make me thought BTF don't require the symbol, though
> > > > > > the symbol still exist in vmlinux with 'static'.
> > > > > >
> > > > > > So any comments of this, Alexei?
> > >
> > > Sorry. I've completely missed this thread.
> > > Now I have a hard time finding context in archives.
> > > If I understood what's going on the removal of "static" cases issues?
> > > Yes. That's expected.
> > > noinline alone is not enough to work reliably.
> >
> > Not removal, commit 3351b16af494 ("mm/filemap: add static for function
> > __add_to_page_cache_locked") made the function static which breaks the
> > build in btfids phase - but it seems to happen only on some
> > architectures. In our case, ppc64, ppc64le and riscv64 are broken,
> > x86_64, i586 and s390x succeed. (I made a mistake above, aarch64 did not
> > fail - but only because it was not built at all.)
> >
> > The thread starts with
> > http://lkml.kernel.org/r/1604661895-5495-1-git-send-email-alex@linux.alibaba.com
>
> Got it. So the above commit is wrong.
> The addition of "static" is incorrect here.
> Regardless of btf_id generation.
> "static noinline" means that the error injection in that spot is unreliable.
> Even when bpf is completely compiled out of the kernel.
I finally realized that the addition of 'static' was pushed into Linus's tree :(
Please revert commit 3351b16af494 ("mm/filemap: add static for
function __add_to_page_cache_locked")
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Mon, Dec 7, 2020 at 2:53 PM Michal Kubecek wrote:
>
> On Mon, Dec 07, 2020 at 02:44:22PM -0800, Alexei Starovoitov wrote:
> > On Mon, Dec 7, 2020 at 10:36 AM Justin Forbes wrote:
> > >
> > > On Mon, Dec 7, 2020 at 2:16 AM Michal Kubecek wrote:
> > > >
> > > > On Thu, Nov 12, 2020 at 08:18:57AM +0800, Alex Shi wrote:
> > > > >
> > > > >
> > > > > 在 2020/11/11 上午3:50, Andrew Morton 写道:
> > > > > > On Tue, 10 Nov 2020 08:39:24 +0530 Souptick Joarder
> > > > > > wrote:
> > > > > >
> > > > > >> On Fri, Nov 6, 2020 at 4:55 PM Alex Shi
> > > > > >> wrote:
> > > > > >>>
> > > > > >>> Otherwise it cause gcc warning:
> > > > > >>> ^~~
> > > > > >>> ../mm/filemap.c:830:14: warning: no previous prototype for
> > > > > >>> ‘__add_to_page_cache_locked’ [-Wmissing-prototypes]
> > > > > >>> noinline int __add_to_page_cache_locked(struct page *page,
> > > > > >>> ^~
> > > > > >>
> > > > > >> Is CONFIG_DEBUG_INFO_BTF enabled in your .config ?
> > > > > >
> > > > > > hm, yes.
> > > > >
> > > > > When the config enabled, compiling looks good untill pahole tool
> > > > > used to get BTF info, but I still failed on a right version pahole
> > > > > > 1.16. Sorry.
> > > > >
> > > > > >
> > > > > >>>
> > > > > >>> Signed-off-by: Alex Shi
> > > > > >>> Cc: Andrew Morton
> > > > > >>> Cc: linux...@kvack.org
> > > > > >>> Cc: linux-kernel@vger.kernel.org
> > > > > >>> ---
> > > > > >>> mm/filemap.c | 2 +-
> > > > > >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >>>
> > > > > >>> diff --git a/mm/filemap.c b/mm/filemap.c
> > > > > >>> index d90614f501da..249cf489f5df 100644
> > > > > >>> --- a/mm/filemap.c
> > > > > >>> +++ b/mm/filemap.c
> > > > > >>> @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old,
> > > > > >>> struct page *new, gfp_t gfp_mask)
> > > > > >>> }
> > > > > >>> EXPORT_SYMBOL_GPL(replace_page_cache_page);
> > > > > >>>
> > > > > >>> -noinline int __add_to_page_cache_locked(struct page *page,
> > > > > >>> +static noinline int __add_to_page_cache_locked(struct page *page,
> > > > > >>> struct address_space
> > > > > >>> *mapping,
> > > > > >>> pgoff_t offset, gfp_t gfp,
> > > > > >>> void **shadowp)
> > > > > >
> > > > > > It's unclear to me whether BTF_ID() requires that the target symbol
> > > > > > be
> > > > > > non-static. It doesn't actually reference the symbol:
> > > > > >
> > > > > > #define BTF_ID(prefix, name) \
> > > > > > __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__))
> > > > > >
> > > > >
> > > > > The above usage make me thought BTF don't require the symbol, though
> > > > > the symbol still exist in vmlinux with 'static'.
> > > > >
> > > > > So any comments of this, Alexei?
> >
> > Sorry. I've completely missed this thread.
> > Now I have a hard time finding context in archives.
> > If I understood what's going on the removal of "static" cases issues?
> > Yes. That's expected.
> > noinline alone is not enough to work reliably.
>
> Not removal, commit 3351b16af494 ("mm/filemap: add static for function
> __add_to_page_cache_locked") made the function static which breaks the
> build in btfids phase - but it seems to happen only on some
> architectures. In our case, ppc64, ppc64le and riscv64 are broken,
> x86_64, i586 and s390x succeed. (I made a mistake above, aarch64 did not
> fail - but only because it was not built at all.)
>
> The thread starts with
> http://lkml.kernel.org/r/1604661895-5495-1-git-send-email-alex@linux.alibaba.com
Got it. So the above commit is wrong.
The addition of "static" is incorrect here.
Regardless of btf_id generation.
"static noinline" means that the error injection in that spot is unreliable.
Even when bpf is completely compiled out of the kernel.
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Mon, Dec 07, 2020 at 02:44:22PM -0800, Alexei Starovoitov wrote:
> On Mon, Dec 7, 2020 at 10:36 AM Justin Forbes wrote:
> >
> > On Mon, Dec 7, 2020 at 2:16 AM Michal Kubecek wrote:
> > >
> > > On Thu, Nov 12, 2020 at 08:18:57AM +0800, Alex Shi wrote:
> > > >
> > > >
> > > > 在 2020/11/11 上午3:50, Andrew Morton 写道:
> > > > > On Tue, 10 Nov 2020 08:39:24 +0530 Souptick Joarder
> > > > > wrote:
> > > > >
> > > > >> On Fri, Nov 6, 2020 at 4:55 PM Alex Shi
> > > > >> wrote:
> > > > >>>
> > > > >>> Otherwise it cause gcc warning:
> > > > >>> ^~~
> > > > >>> ../mm/filemap.c:830:14: warning: no previous prototype for
> > > > >>> ‘__add_to_page_cache_locked’ [-Wmissing-prototypes]
> > > > >>> noinline int __add_to_page_cache_locked(struct page *page,
> > > > >>> ^~
> > > > >>
> > > > >> Is CONFIG_DEBUG_INFO_BTF enabled in your .config ?
> > > > >
> > > > > hm, yes.
> > > >
> > > > When the config enabled, compiling looks good untill pahole tool
> > > > used to get BTF info, but I still failed on a right version pahole
> > > > > 1.16. Sorry.
> > > >
> > > > >
> > > > >>>
> > > > >>> Signed-off-by: Alex Shi
> > > > >>> Cc: Andrew Morton
> > > > >>> Cc: linux...@kvack.org
> > > > >>> Cc: linux-kernel@vger.kernel.org
> > > > >>> ---
> > > > >>> mm/filemap.c | 2 +-
> > > > >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >>>
> > > > >>> diff --git a/mm/filemap.c b/mm/filemap.c
> > > > >>> index d90614f501da..249cf489f5df 100644
> > > > >>> --- a/mm/filemap.c
> > > > >>> +++ b/mm/filemap.c
> > > > >>> @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old,
> > > > >>> struct page *new, gfp_t gfp_mask)
> > > > >>> }
> > > > >>> EXPORT_SYMBOL_GPL(replace_page_cache_page);
> > > > >>>
> > > > >>> -noinline int __add_to_page_cache_locked(struct page *page,
> > > > >>> +static noinline int __add_to_page_cache_locked(struct page *page,
> > > > >>> struct address_space
> > > > >>> *mapping,
> > > > >>> pgoff_t offset, gfp_t gfp,
> > > > >>> void **shadowp)
> > > > >
> > > > > It's unclear to me whether BTF_ID() requires that the target symbol be
> > > > > non-static. It doesn't actually reference the symbol:
> > > > >
> > > > > #define BTF_ID(prefix, name) \
> > > > > __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__))
> > > > >
> > > >
> > > > The above usage make me thought BTF don't require the symbol, though
> > > > the symbol still exist in vmlinux with 'static'.
> > > >
> > > > So any comments of this, Alexei?
>
> Sorry. I've completely missed this thread.
> Now I have a hard time finding context in archives.
> If I understood what's going on the removal of "static" cases issues?
> Yes. That's expected.
> noinline alone is not enough to work reliably.
Not removal, commit 3351b16af494 ("mm/filemap: add static for function
__add_to_page_cache_locked") made the function static which breaks the
build in btfids phase - but it seems to happen only on some
architectures. In our case, ppc64, ppc64le and riscv64 are broken,
x86_64, i586 and s390x succeed. (I made a mistake above, aarch64 did not
fail - but only because it was not built at all.)
The thread starts with
http://lkml.kernel.org/r/1604661895-5495-1-git-send-email-alex@linux.alibaba.com
Michal
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Mon, Dec 7, 2020 at 10:36 AM Justin Forbes wrote: > > On Mon, Dec 7, 2020 at 2:16 AM Michal Kubecek wrote: > > > > On Thu, Nov 12, 2020 at 08:18:57AM +0800, Alex Shi wrote: > > > > > > > > > 在 2020/11/11 上午3:50, Andrew Morton 写道: > > > > On Tue, 10 Nov 2020 08:39:24 +0530 Souptick Joarder > > > > wrote: > > > > > > > >> On Fri, Nov 6, 2020 at 4:55 PM Alex Shi > > > >> wrote: > > > >>> > > > >>> Otherwise it cause gcc warning: > > > >>> ^~~ > > > >>> ../mm/filemap.c:830:14: warning: no previous prototype for > > > >>> ‘__add_to_page_cache_locked’ [-Wmissing-prototypes] > > > >>> noinline int __add_to_page_cache_locked(struct page *page, > > > >>> ^~ > > > >> > > > >> Is CONFIG_DEBUG_INFO_BTF enabled in your .config ? > > > > > > > > hm, yes. > > > > > > When the config enabled, compiling looks good untill pahole tool > > > used to get BTF info, but I still failed on a right version pahole > > > > 1.16. Sorry. > > > > > > > > > > >>> > > > >>> Signed-off-by: Alex Shi > > > >>> Cc: Andrew Morton > > > >>> Cc: linux...@kvack.org > > > >>> Cc: linux-kernel@vger.kernel.org > > > >>> --- > > > >>> mm/filemap.c | 2 +- > > > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > > > >>> > > > >>> diff --git a/mm/filemap.c b/mm/filemap.c > > > >>> index d90614f501da..249cf489f5df 100644 > > > >>> --- a/mm/filemap.c > > > >>> +++ b/mm/filemap.c > > > >>> @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old, > > > >>> struct page *new, gfp_t gfp_mask) > > > >>> } > > > >>> EXPORT_SYMBOL_GPL(replace_page_cache_page); > > > >>> > > > >>> -noinline int __add_to_page_cache_locked(struct page *page, > > > >>> +static noinline int __add_to_page_cache_locked(struct page *page, > > > >>> struct address_space *mapping, > > > >>> pgoff_t offset, gfp_t gfp, > > > >>> void **shadowp) > > > > > > > > It's unclear to me whether BTF_ID() requires that the target symbol be > > > > non-static. It doesn't actually reference the symbol: > > > > > > > > #define BTF_ID(prefix, name) \ > > > > __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__)) > > > > > > > > > > The above usage make me thought BTF don't require the symbol, though > > > the symbol still exist in vmlinux with 'static'. > > > > > > So any comments of this, Alexei? Sorry. I've completely missed this thread. Now I have a hard time finding context in archives. If I understood what's going on the removal of "static" cases issues? Yes. That's expected. noinline alone is not enough to work reliably.
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Mon, Dec 7, 2020 at 2:16 AM Michal Kubecek wrote: > > On Thu, Nov 12, 2020 at 08:18:57AM +0800, Alex Shi wrote: > > > > > > 在 2020/11/11 上午3:50, Andrew Morton 写道: > > > On Tue, 10 Nov 2020 08:39:24 +0530 Souptick Joarder > > > wrote: > > > > > >> On Fri, Nov 6, 2020 at 4:55 PM Alex Shi > > >> wrote: > > >>> > > >>> Otherwise it cause gcc warning: > > >>> ^~~ > > >>> ../mm/filemap.c:830:14: warning: no previous prototype for > > >>> ‘__add_to_page_cache_locked’ [-Wmissing-prototypes] > > >>> noinline int __add_to_page_cache_locked(struct page *page, > > >>> ^~ > > >> > > >> Is CONFIG_DEBUG_INFO_BTF enabled in your .config ? > > > > > > hm, yes. > > > > When the config enabled, compiling looks good untill pahole tool > > used to get BTF info, but I still failed on a right version pahole > > > 1.16. Sorry. > > > > > > > >>> > > >>> Signed-off-by: Alex Shi > > >>> Cc: Andrew Morton > > >>> Cc: linux...@kvack.org > > >>> Cc: linux-kernel@vger.kernel.org > > >>> --- > > >>> mm/filemap.c | 2 +- > > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > > >>> > > >>> diff --git a/mm/filemap.c b/mm/filemap.c > > >>> index d90614f501da..249cf489f5df 100644 > > >>> --- a/mm/filemap.c > > >>> +++ b/mm/filemap.c > > >>> @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old, > > >>> struct page *new, gfp_t gfp_mask) > > >>> } > > >>> EXPORT_SYMBOL_GPL(replace_page_cache_page); > > >>> > > >>> -noinline int __add_to_page_cache_locked(struct page *page, > > >>> +static noinline int __add_to_page_cache_locked(struct page *page, > > >>> struct address_space *mapping, > > >>> pgoff_t offset, gfp_t gfp, > > >>> void **shadowp) > > > > > > It's unclear to me whether BTF_ID() requires that the target symbol be > > > non-static. It doesn't actually reference the symbol: > > > > > > #define BTF_ID(prefix, name) \ > > > __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__)) > > > > > > > The above usage make me thought BTF don't require the symbol, though > > the symbol still exist in vmlinux with 'static'. > > > > So any comments of this, Alexei? > > It's probably more complicated: our v5.10-rc7 builds with > CONFIG_DEBUG_INFO_BTF=y fail on ppc64 and ppc64le with > > BTFIDS vmlinux >FAILED unresolved symbol __add_to_page_cache_locked > > > but succeed on x86_64, i586, aarch64 and s390x. So far I don't see why > this should depend on architecture. > Fedora is failing with rc7 on the same issue on PPC only. Justin
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Thu, Nov 12, 2020 at 08:18:57AM +0800, Alex Shi wrote: > > > 在 2020/11/11 上午3:50, Andrew Morton 写道: > > On Tue, 10 Nov 2020 08:39:24 +0530 Souptick Joarder > > wrote: > > > >> On Fri, Nov 6, 2020 at 4:55 PM Alex Shi wrote: > >>> > >>> Otherwise it cause gcc warning: > >>> ^~~ > >>> ../mm/filemap.c:830:14: warning: no previous prototype for > >>> ‘__add_to_page_cache_locked’ [-Wmissing-prototypes] > >>> noinline int __add_to_page_cache_locked(struct page *page, > >>> ^~ > >> > >> Is CONFIG_DEBUG_INFO_BTF enabled in your .config ? > > > > hm, yes. > > When the config enabled, compiling looks good untill pahole tool > used to get BTF info, but I still failed on a right version pahole > > 1.16. Sorry. > > > > >>> > >>> Signed-off-by: Alex Shi > >>> Cc: Andrew Morton > >>> Cc: linux...@kvack.org > >>> Cc: linux-kernel@vger.kernel.org > >>> --- > >>> mm/filemap.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/mm/filemap.c b/mm/filemap.c > >>> index d90614f501da..249cf489f5df 100644 > >>> --- a/mm/filemap.c > >>> +++ b/mm/filemap.c > >>> @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old, struct > >>> page *new, gfp_t gfp_mask) > >>> } > >>> EXPORT_SYMBOL_GPL(replace_page_cache_page); > >>> > >>> -noinline int __add_to_page_cache_locked(struct page *page, > >>> +static noinline int __add_to_page_cache_locked(struct page *page, > >>> struct address_space *mapping, > >>> pgoff_t offset, gfp_t gfp, > >>> void **shadowp) > > > > It's unclear to me whether BTF_ID() requires that the target symbol be > > non-static. It doesn't actually reference the symbol: > > > > #define BTF_ID(prefix, name) \ > > __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__)) > > > > The above usage make me thought BTF don't require the symbol, though > the symbol still exist in vmlinux with 'static'. > > So any comments of this, Alexei? It's probably more complicated: our v5.10-rc7 builds with CONFIG_DEBUG_INFO_BTF=y fail on ppc64 and ppc64le with BTFIDS vmlinux FAILED unresolved symbol __add_to_page_cache_locked but succeed on x86_64, i586, aarch64 and s390x. So far I don't see why this should depend on architecture. Michal
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
Alex Shi wrote:
> 在 2020/11/11 上午3:50, Andrew Morton 写道:
>> On Tue, 10 Nov 2020 08:39:24 +0530 Souptick Joarder
>> wrote:
>>
>>> On Fri, Nov 6, 2020 at 4:55 PM Alex Shi wrote:
Otherwise it cause gcc warning:
^~~
../mm/filemap.c:830:14: warning: no previous prototype for
‘__add_to_page_cache_locked’ [-Wmissing-prototypes]
noinline int __add_to_page_cache_locked(struct page *page,
^~
>>>
>>> Is CONFIG_DEBUG_INFO_BTF enabled in your .config ?
>>
>> hm, yes.
>
> When the config enabled, compiling looks good untill pahole tool
> used to get BTF info, but I still failed on a right version pahole
>> 1.16. Sorry.
I'm seeing an issue with this patch. My build system has pahole v1.17,
but I don't think the pahole version is key.
$ git checkout 3351b16af494 # recently added to linus/master
$ make defconfig
$ make menuconfig # set CONFIG_BPF_SYSCALL and CONFIG_DEBUG_INFO_BTF
$ make V=1
+ ./tools/bpf/resolve_btfids/resolve_btfids vmlinux
FAILED unresolved symbol __add_to_page_cache_locked
Reverting 3351b16af494 ("mm/filemap: add static for function
__add_to_page_cache_locked") fixes the issue.
I don't see the warning which motivated this patch, but maybe it
requires particular a .config or gcc version. Perhaps adding a
__add_to_page_cache_locked() prototype would meet avoid it. But I
haven't studied enough on BTF to know if there's a better answer.
Signed-off-by: Alex Shi
Cc: Andrew Morton
Cc: linux...@kvack.org
Cc: linux-kernel@vger.kernel.org
---
mm/filemap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index d90614f501da..249cf489f5df 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old, struct
page *new, gfp_t gfp_mask)
}
EXPORT_SYMBOL_GPL(replace_page_cache_page);
-noinline int __add_to_page_cache_locked(struct page *page,
+static noinline int __add_to_page_cache_locked(struct page *page,
struct address_space *mapping,
pgoff_t offset, gfp_t gfp,
void **shadowp)
>>
>> It's unclear to me whether BTF_ID() requires that the target symbol be
>> non-static. It doesn't actually reference the symbol:
>>
>> #define BTF_ID(prefix, name) \
>> __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__))
>>
>
> The above usage make me thought BTF don't require the symbol, though
> the symbol still exist in vmlinux with 'static'.
>
> So any comments of this, Alexei?
>
>> Alexei, can you please comment?
>>
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
在 2020/11/11 上午3:50, Andrew Morton 写道: > On Tue, 10 Nov 2020 08:39:24 +0530 Souptick Joarder > wrote: > >> On Fri, Nov 6, 2020 at 4:55 PM Alex Shi wrote: >>> >>> Otherwise it cause gcc warning: >>> ^~~ >>> ../mm/filemap.c:830:14: warning: no previous prototype for >>> ‘__add_to_page_cache_locked’ [-Wmissing-prototypes] >>> noinline int __add_to_page_cache_locked(struct page *page, >>> ^~ >> >> Is CONFIG_DEBUG_INFO_BTF enabled in your .config ? > > hm, yes. When the config enabled, compiling looks good untill pahole tool used to get BTF info, but I still failed on a right version pahole > 1.16. Sorry. > >>> >>> Signed-off-by: Alex Shi >>> Cc: Andrew Morton >>> Cc: linux...@kvack.org >>> Cc: linux-kernel@vger.kernel.org >>> --- >>> mm/filemap.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/mm/filemap.c b/mm/filemap.c >>> index d90614f501da..249cf489f5df 100644 >>> --- a/mm/filemap.c >>> +++ b/mm/filemap.c >>> @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old, struct >>> page *new, gfp_t gfp_mask) >>> } >>> EXPORT_SYMBOL_GPL(replace_page_cache_page); >>> >>> -noinline int __add_to_page_cache_locked(struct page *page, >>> +static noinline int __add_to_page_cache_locked(struct page *page, >>> struct address_space *mapping, >>> pgoff_t offset, gfp_t gfp, >>> void **shadowp) > > It's unclear to me whether BTF_ID() requires that the target symbol be > non-static. It doesn't actually reference the symbol: > > #define BTF_ID(prefix, name) \ > __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__)) > The above usage make me thought BTF don't require the symbol, though the symbol still exist in vmlinux with 'static'. So any comments of this, Alexei? > Alexei, can you please comment? >
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Tue, 10 Nov 2020 08:39:24 +0530 Souptick Joarder wrote: > On Fri, Nov 6, 2020 at 4:55 PM Alex Shi wrote: > > > > Otherwise it cause gcc warning: > > ^~~ > > ../mm/filemap.c:830:14: warning: no previous prototype for > > ‘__add_to_page_cache_locked’ [-Wmissing-prototypes] > > noinline int __add_to_page_cache_locked(struct page *page, > > ^~ > > Is CONFIG_DEBUG_INFO_BTF enabled in your .config ? hm, yes. > > > > Signed-off-by: Alex Shi > > Cc: Andrew Morton > > Cc: linux...@kvack.org > > Cc: linux-kernel@vger.kernel.org > > --- > > mm/filemap.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/filemap.c b/mm/filemap.c > > index d90614f501da..249cf489f5df 100644 > > --- a/mm/filemap.c > > +++ b/mm/filemap.c > > @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old, struct > > page *new, gfp_t gfp_mask) > > } > > EXPORT_SYMBOL_GPL(replace_page_cache_page); > > > > -noinline int __add_to_page_cache_locked(struct page *page, > > +static noinline int __add_to_page_cache_locked(struct page *page, > > struct address_space *mapping, > > pgoff_t offset, gfp_t gfp, > > void **shadowp) It's unclear to me whether BTF_ID() requires that the target symbol be non-static. It doesn't actually reference the symbol: #define BTF_ID(prefix, name) \ __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__)) Alexei, can you please comment?
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
在 2020/11/10 上午11:09, Souptick Joarder 写道: > On Fri, Nov 6, 2020 at 4:55 PM Alex Shi wrote: >> >> Otherwise it cause gcc warning: >> ^~~ >> ../mm/filemap.c:830:14: warning: no previous prototype for >> ‘__add_to_page_cache_locked’ [-Wmissing-prototypes] >> noinline int __add_to_page_cache_locked(struct page *page, >> ^~ > > Is CONFIG_DEBUG_INFO_BTF enabled in your .config ? Sorry, I tried to buld the configure with bzImage, but failed on pahole version too low, and compiled pahole still can not run in git://git.kernel.org/pub/scm/devel/pahole/pahole.git #pahole pahole: symbol lookup error: pahole: undefined symbol: tabs > >> >> Signed-off-by: Alex Shi >> Cc: Andrew Morton >> Cc: linux...@kvack.org >> Cc: linux-kernel@vger.kernel.org >> --- >> mm/filemap.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/mm/filemap.c b/mm/filemap.c >> index d90614f501da..249cf489f5df 100644 >> --- a/mm/filemap.c >> +++ b/mm/filemap.c >> @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old, struct >> page *new, gfp_t gfp_mask) >> } >> EXPORT_SYMBOL_GPL(replace_page_cache_page); >> >> -noinline int __add_to_page_cache_locked(struct page *page, >> +static noinline int __add_to_page_cache_locked(struct page *page, >> struct address_space *mapping, >> pgoff_t offset, gfp_t gfp, >> void **shadowp) >> -- >> 1.8.3.1 >> >>
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Fri, Nov 6, 2020 at 4:55 PM Alex Shi wrote: > > Otherwise it cause gcc warning: > ^~~ > ../mm/filemap.c:830:14: warning: no previous prototype for > ‘__add_to_page_cache_locked’ [-Wmissing-prototypes] > noinline int __add_to_page_cache_locked(struct page *page, > ^~ Is CONFIG_DEBUG_INFO_BTF enabled in your .config ? > > Signed-off-by: Alex Shi > Cc: Andrew Morton > Cc: linux...@kvack.org > Cc: linux-kernel@vger.kernel.org > --- > mm/filemap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/filemap.c b/mm/filemap.c > index d90614f501da..249cf489f5df 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old, struct page > *new, gfp_t gfp_mask) > } > EXPORT_SYMBOL_GPL(replace_page_cache_page); > > -noinline int __add_to_page_cache_locked(struct page *page, > +static noinline int __add_to_page_cache_locked(struct page *page, > struct address_space *mapping, > pgoff_t offset, gfp_t gfp, > void **shadowp) > -- > 1.8.3.1 > >
Re: [PATCH] mm/filemap: add static for function __add_to_page_cache_locked
On Fri, Nov 06, 2020 at 07:24:55PM +0800, Alex Shi wrote: > Otherwise it cause gcc warning: > ^~~ > ../mm/filemap.c:830:14: warning: no previous prototype for > ‘__add_to_page_cache_locked’ [-Wmissing-prototypes] > noinline int __add_to_page_cache_locked(struct page *page, > ^~ > Does this affect the use in: kernel/bpf/verifier.c|11478| BTF_ID(func, __add_to_page_cache_locked) ? It does not look like that calls the function but I'm not sure what BTF_ID does? Ira > > Signed-off-by: Alex Shi > Cc: Andrew Morton > Cc: linux...@kvack.org > Cc: linux-kernel@vger.kernel.org > --- > mm/filemap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/filemap.c b/mm/filemap.c > index d90614f501da..249cf489f5df 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old, struct page > *new, gfp_t gfp_mask) > } > EXPORT_SYMBOL_GPL(replace_page_cache_page); > > -noinline int __add_to_page_cache_locked(struct page *page, > +static noinline int __add_to_page_cache_locked(struct page *page, > struct address_space *mapping, > pgoff_t offset, gfp_t gfp, > void **shadowp) > -- > 1.8.3.1 >
[PATCH] mm/filemap: add static for function __add_to_page_cache_locked
Otherwise it cause gcc warning: ^~~ ../mm/filemap.c:830:14: warning: no previous prototype for ‘__add_to_page_cache_locked’ [-Wmissing-prototypes] noinline int __add_to_page_cache_locked(struct page *page, ^~ Signed-off-by: Alex Shi Cc: Andrew Morton Cc: linux...@kvack.org Cc: linux-kernel@vger.kernel.org --- mm/filemap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/filemap.c b/mm/filemap.c index d90614f501da..249cf489f5df 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -827,7 +827,7 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask) } EXPORT_SYMBOL_GPL(replace_page_cache_page); -noinline int __add_to_page_cache_locked(struct page *page, +static noinline int __add_to_page_cache_locked(struct page *page, struct address_space *mapping, pgoff_t offset, gfp_t gfp, void **shadowp) -- 1.8.3.1
