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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161201075356.GA12804@quack2.suse.cz>
Date:   Thu, 1 Dec 2016 08:53:56 +0100
From:   Jan Kara <jack@...e.cz>
To:     Ross Zwisler <ross.zwisler@...ux.intel.com>
Cc:     Jan Kara <jack@...e.cz>, linux-kernel@...r.kernel.org,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Christoph Hellwig <hch@....de>,
        Dan Williams <dan.j.williams@...el.com>,
        Dave Chinner <david@...morbit.com>,
        Ingo Molnar <mingo@...hat.com>,
        Matthew Wilcox <mawilcox@...rosoft.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-mm@...ck.org, linux-nvdimm@...ts.01.org
Subject: Re: [PATCH 1/6] dax: fix build breakage with ext4, dax and !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@...ux.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@...e.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@...e.com>
SUSE Labs, CR