Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1531545 > unrolled thread

Re: [PATCH 1/6] dax: fix build breakage with ext4, dax and !iomap

Started byRoss Zwisler <ross.zwisler@linux.intel.com>
First post2016-11-28 20:20 +0100
Last post2016-12-01 09:00 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/6] dax: fix build breakage with ext4, dax and !iomap Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-11-28 20:20 +0100
    Re: [PATCH 1/6] dax: fix build breakage with ext4, dax and !iomap Jan Kara <jack@suse.cz> - 2016-11-29 10:00 +0100
      Re: [PATCH 1/6] dax: fix build breakage with ext4, dax and !iomap Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-11-30 20:10 +0100
        Re: [PATCH 1/6] dax: fix build breakage with ext4, dax and !iomap Jan Kara <jack@suse.cz> - 2016-12-01 09:00 +0100

#1531545 — Re: [PATCH 1/6] dax: fix build breakage with ext4, dax and !iomap

FromRoss Zwisler <ross.zwisler@linux.intel.com>
Date2016-11-28 20:20 +0100
SubjectRe: [PATCH 1/6] dax: fix build breakage with ext4, dax and !iomap
Message-ID<sIzD3-6fj-21@gated-at.bofh.it>
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.

[toc] | [next] | [standalone]

#1532007

FromJan Kara <jack@suse.cz>
Date2016-11-29 10:00 +0100
Message-ID<sIMqB-62E-19@gated-at.bofh.it>
In reply to#1531545
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
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [next] | [standalone]

#1533514

FromRoss Zwisler <ross.zwisler@linux.intel.com>
Date2016-11-30 20:10 +0100
Message-ID<sJiqu-1CH-17@gated-at.bofh.it>
In reply to#1532007
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.

[toc] | [prev] | [next] | [standalone]

#1533881

FromJan Kara <jack@suse.cz>
Date2016-12-01 09:00 +0100
Message-ID<sJurE-T0-13@gated-at.bofh.it>
In reply to#1533514
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
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web