| Message ID | 1479926662-21718-2-git-send-email-ross.zwisler@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed 23-11-16 11:44:17, Ross Zwisler wrote: > With the current Kconfig setup it is possible to have the following: > > CONFIG_EXT4_FS=y > CONFIG_FS_DAX=y > CONFIG_FS_IOMAP=n # this is in fs/Kconfig & isn't user accessible > > With this config we get build failures in ext4_dax_fault() because the > iomap functions in fs/dax.c are missing: > > fs/built-in.o: In function `ext4_dax_fault': > file.c:(.text+0x7f3ac): undefined reference to `dax_iomap_fault' > file.c:(.text+0x7f404): undefined reference to `dax_iomap_fault' > fs/built-in.o: In function `ext4_file_read_iter': > file.c:(.text+0x7fc54): undefined reference to `dax_iomap_rw' > fs/built-in.o: In function `ext4_file_write_iter': > file.c:(.text+0x7fe9a): undefined reference to `dax_iomap_rw' > file.c:(.text+0x7feed): undefined reference to `dax_iomap_rw' > fs/built-in.o: In function `ext4_block_zero_page_range': > inode.c:(.text+0x85c0d): undefined reference to `iomap_zero_range' > > Now that the struct buffer_head based DAX fault paths and I/O path have > been removed we really depend on iomap support being present for DAX. Make > this explicit by selecting FS_IOMAP if we compile in DAX support. > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> I've sent the same patch to Ted yesterday and he will probably queue it on top of ext4 iomap patches. If it doesn't happen for some reason, feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/Kconfig | 1 + > fs/dax.c | 2 -- > fs/ext2/Kconfig | 1 - > 3 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/fs/Kconfig b/fs/Kconfig > index 8e9e5f41..18024bf 100644 > --- a/fs/Kconfig > +++ b/fs/Kconfig > @@ -38,6 +38,7 @@ config FS_DAX > bool "Direct Access (DAX) support" > depends on MMU > depends on !(ARM || MIPS || SPARC) > + select FS_IOMAP > help > Direct Access (DAX) can be used on memory-backed block devices. > If the block device supports DAX and the filesystem supports DAX, > diff --git a/fs/dax.c b/fs/dax.c > index be39633..d8fe3eb 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -968,7 +968,6 @@ int __dax_zero_page_range(struct block_device *bdev, sector_t sector, > } > EXPORT_SYMBOL_GPL(__dax_zero_page_range); > > -#ifdef CONFIG_FS_IOMAP > static sector_t dax_iomap_sector(struct iomap *iomap, loff_t pos) > { > return iomap->blkno + (((pos & PAGE_MASK) - iomap->offset) >> 9); > @@ -1405,4 +1404,3 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, unsigned long address, > } > EXPORT_SYMBOL_GPL(dax_iomap_pmd_fault); > #endif /* CONFIG_FS_DAX_PMD */ > -#endif /* CONFIG_FS_IOMAP */ > diff --git a/fs/ext2/Kconfig b/fs/ext2/Kconfig > index 36bea5a..c634874e 100644 > --- a/fs/ext2/Kconfig > +++ b/fs/ext2/Kconfig > @@ -1,6 +1,5 @@ > config EXT2_FS > tristate "Second extended fs support" > - select FS_IOMAP if FS_DAX > help > Ext2 is a standard Linux file system for hard disks. > > -- > 2.7.4 >
On Thu, Nov 24, 2016 at 10:02:39AM +0100, Jan Kara wrote: > On Wed 23-11-16 11:44:17, Ross Zwisler wrote: > > With the current Kconfig setup it is possible to have the following: > > > > CONFIG_EXT4_FS=y > > CONFIG_FS_DAX=y > > CONFIG_FS_IOMAP=n # this is in fs/Kconfig & isn't user accessible > > > > With this config we get build failures in ext4_dax_fault() because the > > iomap functions in fs/dax.c are missing: > > > > fs/built-in.o: In function `ext4_dax_fault': > > file.c:(.text+0x7f3ac): undefined reference to `dax_iomap_fault' > > file.c:(.text+0x7f404): undefined reference to `dax_iomap_fault' > > fs/built-in.o: In function `ext4_file_read_iter': > > file.c:(.text+0x7fc54): undefined reference to `dax_iomap_rw' > > fs/built-in.o: In function `ext4_file_write_iter': > > file.c:(.text+0x7fe9a): undefined reference to `dax_iomap_rw' > > file.c:(.text+0x7feed): undefined reference to `dax_iomap_rw' > > fs/built-in.o: In function `ext4_block_zero_page_range': > > inode.c:(.text+0x85c0d): undefined reference to `iomap_zero_range' > > > > Now that the struct buffer_head based DAX fault paths and I/O path have > > been removed we really depend on iomap support being present for DAX. Make > > this explicit by selecting FS_IOMAP if we compile in DAX support. > > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > > I've sent the same patch to Ted yesterday and he will probably queue it on > top of ext4 iomap patches. If it doesn't happen for some reason, feel free > to add: > > Reviewed-by: Jan Kara <jack@suse.cz> Cool, looks like Ted has pulled in your patch. I think we still eventually want this patch because it cleans up our handling of FS_IOMAP. With your patch we select it separately in both ext4 & ext2 based on whether we include DAX, and we still have #ifdefs in fs/dax.c for FS_IOMAP. I'll pull your most recent patch into my baseline & rework this patch.
On Mon 28-11-16 12:15:04, Ross Zwisler wrote: > On Thu, Nov 24, 2016 at 10:02:39AM +0100, Jan Kara wrote: > > On Wed 23-11-16 11:44:17, Ross Zwisler wrote: > > > With the current Kconfig setup it is possible to have the following: > > > > > > CONFIG_EXT4_FS=y > > > CONFIG_FS_DAX=y > > > CONFIG_FS_IOMAP=n # this is in fs/Kconfig & isn't user accessible > > > > > > With this config we get build failures in ext4_dax_fault() because the > > > iomap functions in fs/dax.c are missing: > > > > > > fs/built-in.o: In function `ext4_dax_fault': > > > file.c:(.text+0x7f3ac): undefined reference to `dax_iomap_fault' > > > file.c:(.text+0x7f404): undefined reference to `dax_iomap_fault' > > > fs/built-in.o: In function `ext4_file_read_iter': > > > file.c:(.text+0x7fc54): undefined reference to `dax_iomap_rw' > > > fs/built-in.o: In function `ext4_file_write_iter': > > > file.c:(.text+0x7fe9a): undefined reference to `dax_iomap_rw' > > > file.c:(.text+0x7feed): undefined reference to `dax_iomap_rw' > > > fs/built-in.o: In function `ext4_block_zero_page_range': > > > inode.c:(.text+0x85c0d): undefined reference to `iomap_zero_range' > > > > > > Now that the struct buffer_head based DAX fault paths and I/O path have > > > been removed we really depend on iomap support being present for DAX. Make > > > this explicit by selecting FS_IOMAP if we compile in DAX support. > > > > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > > > > I've sent the same patch to Ted yesterday and he will probably queue it on > > top of ext4 iomap patches. If it doesn't happen for some reason, feel free > > to add: > > > > Reviewed-by: Jan Kara <jack@suse.cz> > > Cool, looks like Ted has pulled in your patch. > > I think we still eventually want this patch because it cleans up our handling > of FS_IOMAP. With your patch we select it separately in both ext4 & ext2 > based on whether we include DAX, and we still have #ifdefs in fs/dax.c for > FS_IOMAP. Actually, based on Dave's request I've also sent Ted updated version which did select FS_IOMAP in CONFIG_DAX section. However Ted didn't pull that patch (yet?). Anyway, I don't care whose patch gets merged, I just wanted to notify you of possible conflict. Honza
On Tue, Nov 29, 2016 at 09:53:03AM +0100, Jan Kara wrote: > On Mon 28-11-16 12:15:04, Ross Zwisler wrote: > > On Thu, Nov 24, 2016 at 10:02:39AM +0100, Jan Kara wrote: > > > On Wed 23-11-16 11:44:17, Ross Zwisler wrote: > > > > With the current Kconfig setup it is possible to have the following: > > > > > > > > CONFIG_EXT4_FS=y > > > > CONFIG_FS_DAX=y > > > > CONFIG_FS_IOMAP=n # this is in fs/Kconfig & isn't user accessible > > > > > > > > With this config we get build failures in ext4_dax_fault() because the > > > > iomap functions in fs/dax.c are missing: > > > > > > > > fs/built-in.o: In function `ext4_dax_fault': > > > > file.c:(.text+0x7f3ac): undefined reference to `dax_iomap_fault' > > > > file.c:(.text+0x7f404): undefined reference to `dax_iomap_fault' > > > > fs/built-in.o: In function `ext4_file_read_iter': > > > > file.c:(.text+0x7fc54): undefined reference to `dax_iomap_rw' > > > > fs/built-in.o: In function `ext4_file_write_iter': > > > > file.c:(.text+0x7fe9a): undefined reference to `dax_iomap_rw' > > > > file.c:(.text+0x7feed): undefined reference to `dax_iomap_rw' > > > > fs/built-in.o: In function `ext4_block_zero_page_range': > > > > inode.c:(.text+0x85c0d): undefined reference to `iomap_zero_range' > > > > > > > > Now that the struct buffer_head based DAX fault paths and I/O path have > > > > been removed we really depend on iomap support being present for DAX. Make > > > > this explicit by selecting FS_IOMAP if we compile in DAX support. > > > > > > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > > > > > > I've sent the same patch to Ted yesterday and he will probably queue it on > > > top of ext4 iomap patches. If it doesn't happen for some reason, feel free > > > to add: > > > > > > Reviewed-by: Jan Kara <jack@suse.cz> > > > > Cool, looks like Ted has pulled in your patch. > > > > I think we still eventually want this patch because it cleans up our handling > > of FS_IOMAP. With your patch we select it separately in both ext4 & ext2 > > based on whether we include DAX, and we still have #ifdefs in fs/dax.c for > > FS_IOMAP. > > Actually, based on Dave's request I've also sent Ted updated version which > did select FS_IOMAP in CONFIG_DAX section. However Ted didn't pull that > patch (yet?). Anyway, I don't care whose patch gets merged, I just wanted > to notify you of possible conflict. Can you please CC me on these patches in the future? I also don't care whose patches end up fixing this, but I want to make sure we end up in a world where the "select FS_IOMAP" just happens directly for FS_DAX in fs/Kconfig so that I can get rid of the unnecessary #ifdefs in fs/dax.c for CONFIG_FS_IOMAP.
On Wed 30-11-16 12:04:31, Ross Zwisler wrote: > On Tue, Nov 29, 2016 at 09:53:03AM +0100, Jan Kara wrote: > > On Mon 28-11-16 12:15:04, Ross Zwisler wrote: > > > On Thu, Nov 24, 2016 at 10:02:39AM +0100, Jan Kara wrote: > > > > On Wed 23-11-16 11:44:17, Ross Zwisler wrote: > > > > > With the current Kconfig setup it is possible to have the following: > > > > > > > > > > CONFIG_EXT4_FS=y > > > > > CONFIG_FS_DAX=y > > > > > CONFIG_FS_IOMAP=n # this is in fs/Kconfig & isn't user accessible > > > > > > > > > > With this config we get build failures in ext4_dax_fault() because the > > > > > iomap functions in fs/dax.c are missing: > > > > > > > > > > fs/built-in.o: In function `ext4_dax_fault': > > > > > file.c:(.text+0x7f3ac): undefined reference to `dax_iomap_fault' > > > > > file.c:(.text+0x7f404): undefined reference to `dax_iomap_fault' > > > > > fs/built-in.o: In function `ext4_file_read_iter': > > > > > file.c:(.text+0x7fc54): undefined reference to `dax_iomap_rw' > > > > > fs/built-in.o: In function `ext4_file_write_iter': > > > > > file.c:(.text+0x7fe9a): undefined reference to `dax_iomap_rw' > > > > > file.c:(.text+0x7feed): undefined reference to `dax_iomap_rw' > > > > > fs/built-in.o: In function `ext4_block_zero_page_range': > > > > > inode.c:(.text+0x85c0d): undefined reference to `iomap_zero_range' > > > > > > > > > > Now that the struct buffer_head based DAX fault paths and I/O path have > > > > > been removed we really depend on iomap support being present for DAX. Make > > > > > this explicit by selecting FS_IOMAP if we compile in DAX support. > > > > > > > > > > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> > > > > > > > > I've sent the same patch to Ted yesterday and he will probably queue it on > > > > top of ext4 iomap patches. If it doesn't happen for some reason, feel free > > > > to add: > > > > > > > > Reviewed-by: Jan Kara <jack@suse.cz> > > > > > > Cool, looks like Ted has pulled in your patch. > > > > > > I think we still eventually want this patch because it cleans up our handling > > > of FS_IOMAP. With your patch we select it separately in both ext4 & ext2 > > > based on whether we include DAX, and we still have #ifdefs in fs/dax.c for > > > FS_IOMAP. > > > > Actually, based on Dave's request I've also sent Ted updated version which > > did select FS_IOMAP in CONFIG_DAX section. However Ted didn't pull that > > patch (yet?). Anyway, I don't care whose patch gets merged, I just wanted > > to notify you of possible conflict. > > Can you please CC me on these patches in the future? I also don't care whose > patches end up fixing this, but I want to make sure we end up in a world where > the "select FS_IOMAP" just happens directly for FS_DAX in fs/Kconfig so that > I can get rid of the unnecessary #ifdefs in fs/dax.c for CONFIG_FS_IOMAP. Sure, will do. Honza
diff --git a/fs/Kconfig b/fs/Kconfig index 8e9e5f41..18024bf 100644 --- a/fs/Kconfig +++ b/fs/Kconfig @@ -38,6 +38,7 @@ config FS_DAX bool "Direct Access (DAX) support" depends on MMU depends on !(ARM || MIPS || SPARC) + select FS_IOMAP help Direct Access (DAX) can be used on memory-backed block devices. If the block device supports DAX and the filesystem supports DAX, diff --git a/fs/dax.c b/fs/dax.c index be39633..d8fe3eb 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -968,7 +968,6 @@ int __dax_zero_page_range(struct block_device *bdev, sector_t sector, } EXPORT_SYMBOL_GPL(__dax_zero_page_range); -#ifdef CONFIG_FS_IOMAP static sector_t dax_iomap_sector(struct iomap *iomap, loff_t pos) { return iomap->blkno + (((pos & PAGE_MASK) - iomap->offset) >> 9); @@ -1405,4 +1404,3 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, unsigned long address, } EXPORT_SYMBOL_GPL(dax_iomap_pmd_fault); #endif /* CONFIG_FS_DAX_PMD */ -#endif /* CONFIG_FS_IOMAP */ diff --git a/fs/ext2/Kconfig b/fs/ext2/Kconfig index 36bea5a..c634874e 100644 --- a/fs/ext2/Kconfig +++ b/fs/ext2/Kconfig @@ -1,6 +1,5 @@ config EXT2_FS tristate "Second extended fs support" - select FS_IOMAP if FS_DAX help Ext2 is a standard Linux file system for hard disks.
With the current Kconfig setup it is possible to have the following: CONFIG_EXT4_FS=y CONFIG_FS_DAX=y CONFIG_FS_IOMAP=n # this is in fs/Kconfig & isn't user accessible With this config we get build failures in ext4_dax_fault() because the iomap functions in fs/dax.c are missing: fs/built-in.o: In function `ext4_dax_fault': file.c:(.text+0x7f3ac): undefined reference to `dax_iomap_fault' file.c:(.text+0x7f404): undefined reference to `dax_iomap_fault' fs/built-in.o: In function `ext4_file_read_iter': file.c:(.text+0x7fc54): undefined reference to `dax_iomap_rw' fs/built-in.o: In function `ext4_file_write_iter': file.c:(.text+0x7fe9a): undefined reference to `dax_iomap_rw' file.c:(.text+0x7feed): undefined reference to `dax_iomap_rw' fs/built-in.o: In function `ext4_block_zero_page_range': inode.c:(.text+0x85c0d): undefined reference to `iomap_zero_range' Now that the struct buffer_head based DAX fault paths and I/O path have been removed we really depend on iomap support being present for DAX. Make this explicit by selecting FS_IOMAP if we compile in DAX support. Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com> --- fs/Kconfig | 1 + fs/dax.c | 2 -- fs/ext2/Kconfig | 1 - 3 files changed, 1 insertion(+), 3 deletions(-)