| Message ID | 20210101042914.5313-1-rdunlap@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] fs/dax: include <asm/page.h> to fix build error on ARC | expand |
On Thu, Dec 31, 2020 at 08:29:14PM -0800, Randy Dunlap wrote: > fs/dax.c uses copy_user_page() but ARC does not provide that interface, > resulting in a build error. > > Provide copy_user_page() in <asm/page.h> (beside copy_page()) and > add <asm/page.h> to fs/dax.c to fix the build error. > > ../fs/dax.c: In function 'copy_cow_page_dax': > ../fs/dax.c:702:2: error: implicit declaration of function 'copy_user_page'; did you mean 'copy_to_user_page'? [-Werror=implicit-function-declaration] > > Fixes: cccbce671582 ("filesystem-dax: convert to dax_direct_access()") > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> Looks reasonable Reviewed-by: Ira Weiny <ira.weiny@intel.com> > Cc: Vineet Gupta <vgupta@synopsys.com> > Cc: linux-snps-arc@lists.infradead.org > Cc: Dan Williams <dan.j.williams@intel.com> > Acked-by: Vineet Gupta <vgupts@synopsys.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Jan Kara <jack@suse.cz> > Cc: linux-fsdevel@vger.kernel.org > Cc: linux-nvdimm@lists.01.org > --- > v2: rebase, add more Cc: > > arch/arc/include/asm/page.h | 1 + > fs/dax.c | 1 + > 2 files changed, 2 insertions(+) > > --- lnx-511-rc1.orig/fs/dax.c > +++ lnx-511-rc1/fs/dax.c > @@ -25,6 +25,7 @@ > #include <linux/sizes.h> > #include <linux/mmu_notifier.h> > #include <linux/iomap.h> > +#include <asm/page.h> > #include <asm/pgalloc.h> > > #define CREATE_TRACE_POINTS > --- lnx-511-rc1.orig/arch/arc/include/asm/page.h > +++ lnx-511-rc1/arch/arc/include/asm/page.h > @@ -10,6 +10,7 @@ > #ifndef __ASSEMBLY__ > > #define clear_page(paddr) memset((paddr), 0, PAGE_SIZE) > +#define copy_user_page(to, from, vaddr, pg) copy_page(to, from) > #define copy_page(to, from) memcpy((to), (from), PAGE_SIZE) > > struct vm_area_struct; > _______________________________________________ > Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org > To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
On Thu, Dec 31, 2020 at 8:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > fs/dax.c uses copy_user_page() but ARC does not provide that interface, > resulting in a build error. > > Provide copy_user_page() in <asm/page.h> (beside copy_page()) and > add <asm/page.h> to fs/dax.c to fix the build error. > > ../fs/dax.c: In function 'copy_cow_page_dax': > ../fs/dax.c:702:2: error: implicit declaration of function 'copy_user_page'; did you mean 'copy_to_user_page'? [-Werror=implicit-function-declaration] > > Fixes: cccbce671582 ("filesystem-dax: convert to dax_direct_access()") > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > Cc: Vineet Gupta <vgupta@synopsys.com> > Cc: linux-snps-arc@lists.infradead.org > Cc: Dan Williams <dan.j.williams@intel.com> > Acked-by: Vineet Gupta <vgupts@synopsys.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Jan Kara <jack@suse.cz> > Cc: linux-fsdevel@vger.kernel.org > Cc: linux-nvdimm@lists.01.org > --- > v2: rebase, add more Cc: > > arch/arc/include/asm/page.h | 1 + > fs/dax.c | 1 + > 2 files changed, 2 insertions(+) > > --- lnx-511-rc1.orig/fs/dax.c > +++ lnx-511-rc1/fs/dax.c > @@ -25,6 +25,7 @@ > #include <linux/sizes.h> > #include <linux/mmu_notifier.h> > #include <linux/iomap.h> > +#include <asm/page.h> I would expect this to come from one of the linux/ includes like linux/mm.h. asm/ headers are implementation linux/ headers are api. Once you drop that then the subject of this patch can just be "arc: add a copy_user_page() implementation", and handled by the arc maintainer (or I can take it with Vineet's ack). > #include <asm/pgalloc.h> Yes, this one should have a linux/ api header to front it, but that's a cleanup for another day. > > #define CREATE_TRACE_POINTS > --- lnx-511-rc1.orig/arch/arc/include/asm/page.h > +++ lnx-511-rc1/arch/arc/include/asm/page.h > @@ -10,6 +10,7 @@ > #ifndef __ASSEMBLY__ > > #define clear_page(paddr) memset((paddr), 0, PAGE_SIZE) > +#define copy_user_page(to, from, vaddr, pg) copy_page(to, from) > #define copy_page(to, from) memcpy((to), (from), PAGE_SIZE) > > struct vm_area_struct;
On Mon, Jan 04, 2021 at 12:13:02PM -0800, Dan Williams wrote: > On Thu, Dec 31, 2020 at 8:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > +++ lnx-511-rc1/fs/dax.c > > @@ -25,6 +25,7 @@ > > #include <linux/sizes.h> > > #include <linux/mmu_notifier.h> > > #include <linux/iomap.h> > > +#include <asm/page.h> > > I would expect this to come from one of the linux/ includes like > linux/mm.h. asm/ headers are implementation linux/ headers are api. It does indeed come from linux/mm.h already. And a number of other random places, including linux/serial.h. Our headers are a mess, but they shouldn't be made worse by adding _this_ include. So I second Dan's objection here. > Once you drop that then the subject of this patch can just be "arc: > add a copy_user_page() implementation", and handled by the arc > maintainer (or I can take it with Vineet's ack). > > > #include <asm/pgalloc.h> > > Yes, this one should have a linux/ api header to front it, but that's > a cleanup for another day. Definitely more involved.
On 1/4/21 12:13 PM, Dan Williams wrote: > On Thu, Dec 31, 2020 at 8:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: >> >> fs/dax.c uses copy_user_page() but ARC does not provide that interface, >> resulting in a build error. >> >> Provide copy_user_page() in <asm/page.h> (beside copy_page()) and >> add <asm/page.h> to fs/dax.c to fix the build error. >> >> ../fs/dax.c: In function 'copy_cow_page_dax': >> ../fs/dax.c:702:2: error: implicit declaration of function 'copy_user_page'; did you mean 'copy_to_user_page'? [-Werror=implicit-function-declaration] >> >> Fixes: cccbce671582 ("filesystem-dax: convert to dax_direct_access()") >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> >> Cc: Vineet Gupta <vgupta@synopsys.com> >> Cc: linux-snps-arc@lists.infradead.org >> Cc: Dan Williams <dan.j.williams@intel.com> >> Acked-by: Vineet Gupta <vgupts@synopsys.com> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: Matthew Wilcox <willy@infradead.org> >> Cc: Jan Kara <jack@suse.cz> >> Cc: linux-fsdevel@vger.kernel.org >> Cc: linux-nvdimm@lists.01.org >> --- >> v2: rebase, add more Cc: >> >> arch/arc/include/asm/page.h | 1 + >> fs/dax.c | 1 + >> 2 files changed, 2 insertions(+) >> >> --- lnx-511-rc1.orig/fs/dax.c >> +++ lnx-511-rc1/fs/dax.c >> @@ -25,6 +25,7 @@ >> #include <linux/sizes.h> >> #include <linux/mmu_notifier.h> >> #include <linux/iomap.h> >> +#include <asm/page.h> > > I would expect this to come from one of the linux/ includes like > linux/mm.h. asm/ headers are implementation linux/ headers are api. > > Once you drop that then the subject of this patch can just be "arc: > add a copy_user_page() implementation", and handled by the arc > maintainer (or I can take it with Vineet's ack). Got it. Thanks. Vineet is copied. I expect that he will take the v3 patch. >> #include <asm/pgalloc.h> > > Yes, this one should have a linux/ api header to front it, but that's > a cleanup for another day. That line is only part of the contextual diff in this patch. I guess you are just commenting in general, along with your earlier paragraph. >> >> #define CREATE_TRACE_POINTS >> --- lnx-511-rc1.orig/arch/arc/include/asm/page.h >> +++ lnx-511-rc1/arch/arc/include/asm/page.h >> @@ -10,6 +10,7 @@ >> #ifndef __ASSEMBLY__ >> >> #define clear_page(paddr) memset((paddr), 0, PAGE_SIZE) >> +#define copy_user_page(to, from, vaddr, pg) copy_page(to, from) >> #define copy_page(to, from) memcpy((to), (from), PAGE_SIZE) >> >> struct vm_area_struct;
On Mon, Jan 4, 2021 at 7:47 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > On 1/4/21 12:13 PM, Dan Williams wrote: > > On Thu, Dec 31, 2020 at 8:29 PM Randy Dunlap <rdunlap@infradead.org> wrote: > >> > >> fs/dax.c uses copy_user_page() but ARC does not provide that interface, > >> resulting in a build error. > >> > >> Provide copy_user_page() in <asm/page.h> (beside copy_page()) and > >> add <asm/page.h> to fs/dax.c to fix the build error. > >> > >> ../fs/dax.c: In function 'copy_cow_page_dax': > >> ../fs/dax.c:702:2: error: implicit declaration of function 'copy_user_page'; did you mean 'copy_to_user_page'? [-Werror=implicit-function-declaration] > >> > >> Fixes: cccbce671582 ("filesystem-dax: convert to dax_direct_access()") > >> Reported-by: kernel test robot <lkp@intel.com> > >> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > >> Cc: Vineet Gupta <vgupta@synopsys.com> > >> Cc: linux-snps-arc@lists.infradead.org > >> Cc: Dan Williams <dan.j.williams@intel.com> > >> Acked-by: Vineet Gupta <vgupts@synopsys.com> > >> Cc: Andrew Morton <akpm@linux-foundation.org> > >> Cc: Matthew Wilcox <willy@infradead.org> > >> Cc: Jan Kara <jack@suse.cz> > >> Cc: linux-fsdevel@vger.kernel.org > >> Cc: linux-nvdimm@lists.01.org > >> --- > >> v2: rebase, add more Cc: > >> > >> arch/arc/include/asm/page.h | 1 + > >> fs/dax.c | 1 + > >> 2 files changed, 2 insertions(+) > >> > >> --- lnx-511-rc1.orig/fs/dax.c > >> +++ lnx-511-rc1/fs/dax.c > >> @@ -25,6 +25,7 @@ > >> #include <linux/sizes.h> > >> #include <linux/mmu_notifier.h> > >> #include <linux/iomap.h> > >> +#include <asm/page.h> > > > > I would expect this to come from one of the linux/ includes like > > linux/mm.h. asm/ headers are implementation linux/ headers are api. > > > > Once you drop that then the subject of this patch can just be "arc: > > add a copy_user_page() implementation", and handled by the arc > > maintainer (or I can take it with Vineet's ack). > > Got it. Thanks. > Vineet is copied. I expect that he will take the v3 patch. > > >> #include <asm/pgalloc.h> > > > > Yes, this one should have a linux/ api header to front it, but that's > > a cleanup for another day. > > That line is only part of the contextual diff in this patch. > I guess you are just commenting in general, along with your earlier > paragraph. Yes, I was prefetching a "...hey, but, there's an asm/ include right below this one?"
On Thu, Dec 31, 2020 at 08:29:14PM -0800, Randy Dunlap wrote: > fs/dax.c uses copy_user_page() but ARC does not provide that interface, > resulting in a build error. > > Provide copy_user_page() in <asm/page.h> (beside copy_page()) and > add <asm/page.h> to fs/dax.c to fix the build error. > > ../fs/dax.c: In function 'copy_cow_page_dax': > ../fs/dax.c:702:2: error: implicit declaration of function 'copy_user_page'; did you mean 'copy_to_user_page'? [-Werror=implicit-function-declaration] Could somebody explain what the force-cast is doing in there? I mean, the call is copy_user_page(vto, (void __force *)kaddr, vaddr, to); kaddr is a local variable there, declared as void *; AFAICS, that had been pure cargo-cult since commit 7a9eb20666317794d0279843fbd091af93907780 Author: Dan Williams <dan.j.williams@intel.com> Date: Fri Jun 3 18:06:47 2016 -0700 pmem: kill __pmem address space I mean, it's been more than 4 years, time to bury that body...
--- lnx-511-rc1.orig/fs/dax.c +++ lnx-511-rc1/fs/dax.c @@ -25,6 +25,7 @@ #include <linux/sizes.h> #include <linux/mmu_notifier.h> #include <linux/iomap.h> +#include <asm/page.h> #include <asm/pgalloc.h> #define CREATE_TRACE_POINTS --- lnx-511-rc1.orig/arch/arc/include/asm/page.h +++ lnx-511-rc1/arch/arc/include/asm/page.h @@ -10,6 +10,7 @@ #ifndef __ASSEMBLY__ #define clear_page(paddr) memset((paddr), 0, PAGE_SIZE) +#define copy_user_page(to, from, vaddr, pg) copy_page(to, from) #define copy_page(to, from) memcpy((to), (from), PAGE_SIZE) struct vm_area_struct;
fs/dax.c uses copy_user_page() but ARC does not provide that interface, resulting in a build error. Provide copy_user_page() in <asm/page.h> (beside copy_page()) and add <asm/page.h> to fs/dax.c to fix the build error. ../fs/dax.c: In function 'copy_cow_page_dax': ../fs/dax.c:702:2: error: implicit declaration of function 'copy_user_page'; did you mean 'copy_to_user_page'? [-Werror=implicit-function-declaration] Fixes: cccbce671582 ("filesystem-dax: convert to dax_direct_access()") Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> Cc: Vineet Gupta <vgupta@synopsys.com> Cc: linux-snps-arc@lists.infradead.org Cc: Dan Williams <dan.j.williams@intel.com> Acked-by: Vineet Gupta <vgupts@synopsys.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Matthew Wilcox <willy@infradead.org> Cc: Jan Kara <jack@suse.cz> Cc: linux-fsdevel@vger.kernel.org Cc: linux-nvdimm@lists.01.org --- v2: rebase, add more Cc: arch/arc/include/asm/page.h | 1 + fs/dax.c | 1 + 2 files changed, 2 insertions(+)