linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dax: try to avoid unused function warnings
@ 2016-11-28 21:12 Arnd Bergmann
  2016-11-28 21:24 ` Ross Zwisler
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2016-11-28 21:12 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Ross Zwisler, Arnd Bergmann, Alexander Viro, Matthew Wilcox,
	Jan Kara, Andrew Morton, Dave Chinner, Dan Williams,
	linux-fsdevel, linux-kernel

Without the get_block based I/O, we get warnings when CONFIG_FS_IOMAP
is disabled:

fs/dax.c:736:12: error: ‘dax_insert_mapping’ defined but not used [-Werror=unused-function]
fs/dax.c:512:12: error: ‘copy_user_dax’ defined but not used [-Werror=unused-function]
fs/dax.c:490:12: error: ‘dax_load_hole’ defined but not used [-Werror=unused-function]
fs/dax.c:294:14: error: ‘grab_mapping_entry’ defined but not used [-Werror=unused-function]

This patch blindly marks those as __maybe_unused, which avoids the warnings.
However, I suspect that there is actually more code in this file that should
not be provided without CONFIG_FS_IOMAP even though we don't get a warning
for it, and that we actually want a different rework, so please treat this
as a bug report. I have applied the patch locally in my randconfig build
setup to avoid seeing the warnings.

Fixes: 5ac65736f740 ("dax: rip out get_block based IO support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/dax.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index b1fe228cd609..cf844e77b7b7 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -309,7 +309,7 @@ static void put_unlocked_mapping_entry(struct address_space *mapping,
  * persistent memory the benefit is doubtful. We can add that later if we can
  * show it helps.
  */
-static void *grab_mapping_entry(struct address_space *mapping, pgoff_t index,
+static __maybe_unused void * grab_mapping_entry(struct address_space *mapping, pgoff_t index,
 		unsigned long size_flag)
 {
 	bool pmd_downgrade = false; /* splitting 2MiB entry into 4k entries? */
@@ -489,7 +489,7 @@ int dax_delete_mapping_entry(struct address_space *mapping, pgoff_t index)
  * otherwise it will simply fall out of the page cache under memory
  * pressure without ever having been dirtied.
  */
-static int dax_load_hole(struct address_space *mapping, void *entry,
+static int __maybe_unused dax_load_hole(struct address_space *mapping, void *entry,
 			 struct vm_fault *vmf)
 {
 	struct page *page;
@@ -509,7 +509,7 @@ static int dax_load_hole(struct address_space *mapping, void *entry,
 	return VM_FAULT_LOCKED;
 }
 
-static int copy_user_dax(struct block_device *bdev, sector_t sector, size_t size,
+static int __maybe_unused copy_user_dax(struct block_device *bdev, sector_t sector, size_t size,
 		struct page *to, unsigned long vaddr)
 {
 	struct blk_dax_ctl dax = {
@@ -815,7 +815,7 @@ int dax_writeback_mapping_range(struct address_space *mapping,
 }
 EXPORT_SYMBOL_GPL(dax_writeback_mapping_range);
 
-static int dax_insert_mapping(struct address_space *mapping,
+static int __maybe_unused dax_insert_mapping(struct address_space *mapping,
 		struct block_device *bdev, sector_t sector, size_t size,
 		void **entryp, struct vm_area_struct *vma, struct vm_fault *vmf)
 {
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] dax: try to avoid unused function warnings
  2016-11-28 21:12 [PATCH] dax: try to avoid unused function warnings Arnd Bergmann
@ 2016-11-28 21:24 ` Ross Zwisler
  2016-11-28 22:13   ` Dan Williams
  2017-01-10 16:15   ` Arnd Bergmann
  0 siblings, 2 replies; 11+ messages in thread
From: Ross Zwisler @ 2016-11-28 21:24 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Theodore Ts'o, Ross Zwisler, Alexander Viro, Matthew Wilcox,
	Jan Kara, Andrew Morton, Dave Chinner, Dan Williams,
	linux-fsdevel, linux-kernel

On Mon, Nov 28, 2016 at 10:12:17PM +0100, Arnd Bergmann wrote:
> Without the get_block based I/O, we get warnings when CONFIG_FS_IOMAP
> is disabled:
> 
> fs/dax.c:736:12: error: ‘dax_insert_mapping’ defined but not used [-Werror=unused-function]
> fs/dax.c:512:12: error: ‘copy_user_dax’ defined but not used [-Werror=unused-function]
> fs/dax.c:490:12: error: ‘dax_load_hole’ defined but not used [-Werror=unused-function]
> fs/dax.c:294:14: error: ‘grab_mapping_entry’ defined but not used [-Werror=unused-function]
> 
> This patch blindly marks those as __maybe_unused, which avoids the warnings.
> However, I suspect that there is actually more code in this file that should
> not be provided without CONFIG_FS_IOMAP even though we don't get a warning
> for it, and that we actually want a different rework, so please treat this
> as a bug report. I have applied the patch locally in my randconfig build
> setup to avoid seeing the warnings.
> 
> Fixes: 5ac65736f740 ("dax: rip out get_block based IO support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks for the report.  I think the right way to deal with this is to just
select FS_IOMAP when we pull in the DAX code.  I sent out a patch last week
that does this:

https://lkml.org/lkml/2016/11/23/591

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dax: try to avoid unused function warnings
  2016-11-28 21:24 ` Ross Zwisler
@ 2016-11-28 22:13   ` Dan Williams
  2016-11-28 22:51     ` Ross Zwisler
  2017-01-10 16:15   ` Arnd Bergmann
  1 sibling, 1 reply; 11+ messages in thread
From: Dan Williams @ 2016-11-28 22:13 UTC (permalink / raw)
  To: Ross Zwisler, Arnd Bergmann, Theodore Ts'o, Alexander Viro,
	Matthew Wilcox, Jan Kara, Andrew Morton, Dave Chinner,
	Dan Williams, linux-fsdevel, linux-kernel

On Mon, Nov 28, 2016 at 1:24 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> On Mon, Nov 28, 2016 at 10:12:17PM +0100, Arnd Bergmann wrote:
>> Without the get_block based I/O, we get warnings when CONFIG_FS_IOMAP
>> is disabled:
>>
>> fs/dax.c:736:12: error: ‘dax_insert_mapping’ defined but not used [-Werror=unused-function]
>> fs/dax.c:512:12: error: ‘copy_user_dax’ defined but not used [-Werror=unused-function]
>> fs/dax.c:490:12: error: ‘dax_load_hole’ defined but not used [-Werror=unused-function]
>> fs/dax.c:294:14: error: ‘grab_mapping_entry’ defined but not used [-Werror=unused-function]
>>
>> This patch blindly marks those as __maybe_unused, which avoids the warnings.
>> However, I suspect that there is actually more code in this file that should
>> not be provided without CONFIG_FS_IOMAP even though we don't get a warning
>> for it, and that we actually want a different rework, so please treat this
>> as a bug report. I have applied the patch locally in my randconfig build
>> setup to avoid seeing the warnings.
>>
>> Fixes: 5ac65736f740 ("dax: rip out get_block based IO support")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Thanks for the report.  I think the right way to deal with this is to just
> select FS_IOMAP when we pull in the DAX code.  I sent out a patch last week
> that does this:
>
> https://lkml.org/lkml/2016/11/23/591

It seems awkward for both filesystems and the FS_DAX core to be
selecting FS_IOMAP.  In the end FS_DAX and FS_IOMAP are both libraries
of functionality that a filesystem can optionally use.  I think the
longer term FS_DAX stops being an independent user visible setting and
is instead selected by filesystems that want DAX.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dax: try to avoid unused function warnings
  2016-11-28 22:13   ` Dan Williams
@ 2016-11-28 22:51     ` Ross Zwisler
  2016-11-28 23:08       ` Dan Williams
  0 siblings, 1 reply; 11+ messages in thread
From: Ross Zwisler @ 2016-11-28 22:51 UTC (permalink / raw)
  To: Dan Williams
  Cc: Ross Zwisler, Arnd Bergmann, Theodore Ts'o, Alexander Viro,
	Matthew Wilcox, Jan Kara, Andrew Morton, Dave Chinner,
	linux-fsdevel, linux-kernel

On Mon, Nov 28, 2016 at 02:13:29PM -0800, Dan Williams wrote:
> On Mon, Nov 28, 2016 at 1:24 PM, Ross Zwisler
> <ross.zwisler@linux.intel.com> wrote:
> > On Mon, Nov 28, 2016 at 10:12:17PM +0100, Arnd Bergmann wrote:
> >> Without the get_block based I/O, we get warnings when CONFIG_FS_IOMAP
> >> is disabled:
> >>
> >> fs/dax.c:736:12: error: ‘dax_insert_mapping’ defined but not used [-Werror=unused-function]
> >> fs/dax.c:512:12: error: ‘copy_user_dax’ defined but not used [-Werror=unused-function]
> >> fs/dax.c:490:12: error: ‘dax_load_hole’ defined but not used [-Werror=unused-function]
> >> fs/dax.c:294:14: error: ‘grab_mapping_entry’ defined but not used [-Werror=unused-function]
> >>
> >> This patch blindly marks those as __maybe_unused, which avoids the warnings.
> >> However, I suspect that there is actually more code in this file that should
> >> not be provided without CONFIG_FS_IOMAP even though we don't get a warning
> >> for it, and that we actually want a different rework, so please treat this
> >> as a bug report. I have applied the patch locally in my randconfig build
> >> setup to avoid seeing the warnings.
> >>
> >> Fixes: 5ac65736f740 ("dax: rip out get_block based IO support")
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >
> > Thanks for the report.  I think the right way to deal with this is to just
> > select FS_IOMAP when we pull in the DAX code.  I sent out a patch last week
> > that does this:
> >
> > https://lkml.org/lkml/2016/11/23/591
> 
> It seems awkward for both filesystems and the FS_DAX core to be
> selecting FS_IOMAP.  In the end FS_DAX and FS_IOMAP are both libraries
> of functionality that a filesystem can optionally use.  I think the
> longer term FS_DAX stops being an independent user visible setting and
> is instead selected by filesystems that want DAX.

This doesn't make sense to me.  DAX is a user-selectable option that changes
behavior (at the user's request), but FS_IOMAP is a library of functionality
that is required for XFS and for DAX.

The filesystems can all work fine without DAX (hence the user option), but DAX
and XFS at least require FS_IOMAP to behave correctly.

If you made DAX a FS selectable option instead of a user selectable one, when
would a FS know it needs to include DAX support?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dax: try to avoid unused function warnings
  2016-11-28 22:51     ` Ross Zwisler
@ 2016-11-28 23:08       ` Dan Williams
  2016-11-29  2:10         ` Dave Chinner
  0 siblings, 1 reply; 11+ messages in thread
From: Dan Williams @ 2016-11-28 23:08 UTC (permalink / raw)
  To: Ross Zwisler, Dan Williams, Arnd Bergmann, Theodore Ts'o,
	Alexander Viro, Matthew Wilcox, Jan Kara, Andrew Morton,
	Dave Chinner, linux-fsdevel, linux-kernel

On Mon, Nov 28, 2016 at 2:51 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> On Mon, Nov 28, 2016 at 02:13:29PM -0800, Dan Williams wrote:
>> On Mon, Nov 28, 2016 at 1:24 PM, Ross Zwisler
>> <ross.zwisler@linux.intel.com> wrote:
>> > On Mon, Nov 28, 2016 at 10:12:17PM +0100, Arnd Bergmann wrote:
>> >> Without the get_block based I/O, we get warnings when CONFIG_FS_IOMAP
>> >> is disabled:
>> >>
>> >> fs/dax.c:736:12: error: ‘dax_insert_mapping’ defined but not used [-Werror=unused-function]
>> >> fs/dax.c:512:12: error: ‘copy_user_dax’ defined but not used [-Werror=unused-function]
>> >> fs/dax.c:490:12: error: ‘dax_load_hole’ defined but not used [-Werror=unused-function]
>> >> fs/dax.c:294:14: error: ‘grab_mapping_entry’ defined but not used [-Werror=unused-function]
>> >>
>> >> This patch blindly marks those as __maybe_unused, which avoids the warnings.
>> >> However, I suspect that there is actually more code in this file that should
>> >> not be provided without CONFIG_FS_IOMAP even though we don't get a warning
>> >> for it, and that we actually want a different rework, so please treat this
>> >> as a bug report. I have applied the patch locally in my randconfig build
>> >> setup to avoid seeing the warnings.
>> >>
>> >> Fixes: 5ac65736f740 ("dax: rip out get_block based IO support")
>> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> >
>> > Thanks for the report.  I think the right way to deal with this is to just
>> > select FS_IOMAP when we pull in the DAX code.  I sent out a patch last week
>> > that does this:
>> >
>> > https://lkml.org/lkml/2016/11/23/591
>>
>> It seems awkward for both filesystems and the FS_DAX core to be
>> selecting FS_IOMAP.  In the end FS_DAX and FS_IOMAP are both libraries
>> of functionality that a filesystem can optionally use.  I think the
>> longer term FS_DAX stops being an independent user visible setting and
>> is instead selected by filesystems that want DAX.
>
> This doesn't make sense to me.  DAX is a user-selectable option that changes
> behavior (at the user's request), but FS_IOMAP is a library of functionality
> that is required for XFS and for DAX.
>
> The filesystems can all work fine without DAX (hence the user option), but DAX
> and XFS at least require FS_IOMAP to behave correctly.
>
> If you made DAX a FS selectable option instead of a user selectable one, when
> would a FS know it needs to include DAX support?

With a user-selectable DAX knob per-filesystem, XFS_DAX, EXT4_DAX, etc...

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dax: try to avoid unused function warnings
  2016-11-28 23:08       ` Dan Williams
@ 2016-11-29  2:10         ` Dave Chinner
  0 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2016-11-29  2:10 UTC (permalink / raw)
  To: Dan Williams
  Cc: Ross Zwisler, Arnd Bergmann, Theodore Ts'o, Alexander Viro,
	Matthew Wilcox, Jan Kara, Andrew Morton, Dave Chinner,
	linux-fsdevel, linux-kernel

On Mon, Nov 28, 2016 at 03:08:18PM -0800, Dan Williams wrote:
> On Mon, Nov 28, 2016 at 2:51 PM, Ross Zwisler
> <ross.zwisler@linux.intel.com> wrote:
> > On Mon, Nov 28, 2016 at 02:13:29PM -0800, Dan Williams wrote:
> >> On Mon, Nov 28, 2016 at 1:24 PM, Ross Zwisler
> >> <ross.zwisler@linux.intel.com> wrote:
> >> > On Mon, Nov 28, 2016 at 10:12:17PM +0100, Arnd Bergmann wrote:
> >> >> Without the get_block based I/O, we get warnings when CONFIG_FS_IOMAP
> >> >> is disabled:
> >> >>
> >> >> fs/dax.c:736:12: error: ‘dax_insert_mapping’ defined but not used [-Werror=unused-function]
> >> >> fs/dax.c:512:12: error: ‘copy_user_dax’ defined but not used [-Werror=unused-function]
> >> >> fs/dax.c:490:12: error: ‘dax_load_hole’ defined but not used [-Werror=unused-function]
> >> >> fs/dax.c:294:14: error: ‘grab_mapping_entry’ defined but not used [-Werror=unused-function]
> >> >>
> >> >> This patch blindly marks those as __maybe_unused, which avoids the warnings.
> >> >> However, I suspect that there is actually more code in this file that should
> >> >> not be provided without CONFIG_FS_IOMAP even though we don't get a warning
> >> >> for it, and that we actually want a different rework, so please treat this
> >> >> as a bug report. I have applied the patch locally in my randconfig build
> >> >> setup to avoid seeing the warnings.
> >> >>
> >> >> Fixes: 5ac65736f740 ("dax: rip out get_block based IO support")
> >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> >
> >> > Thanks for the report.  I think the right way to deal with this is to just
> >> > select FS_IOMAP when we pull in the DAX code.  I sent out a patch last week
> >> > that does this:
> >> >
> >> > https://lkml.org/lkml/2016/11/23/591
> >>
> >> It seems awkward for both filesystems and the FS_DAX core to be
> >> selecting FS_IOMAP.  In the end FS_DAX and FS_IOMAP are both libraries
> >> of functionality that a filesystem can optionally use.  I think the
> >> longer term FS_DAX stops being an independent user visible setting and
> >> is instead selected by filesystems that want DAX.
> >
> > This doesn't make sense to me.  DAX is a user-selectable option that changes
> > behavior (at the user's request), but FS_IOMAP is a library of functionality
> > that is required for XFS and for DAX.
> >
> > The filesystems can all work fine without DAX (hence the user option), but DAX
> > and XFS at least require FS_IOMAP to behave correctly.
> >
> > If you made DAX a FS selectable option instead of a user selectable one, when
> > would a FS know it needs to include DAX support?
> 
> With a user-selectable DAX knob per-filesystem, XFS_DAX, EXT4_DAX, etc...

That's just silly. Requiring users to configure every filesystem
that can support DAX to support DAX at config time is unneeded
config space bloat. DAX has an iomap config dependency, so just
select it when DAX is selected - everything else should just be
automatically and nobody else needs to care what build dependencies
DAX has.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dax: try to avoid unused function warnings
  2016-11-28 21:24 ` Ross Zwisler
  2016-11-28 22:13   ` Dan Williams
@ 2017-01-10 16:15   ` Arnd Bergmann
  2017-01-10 19:44     ` Ross Zwisler
  2017-01-10 22:29     ` [PATCH] dax: fix build warnings with FS_DAX and !FS_IOMAP Ross Zwisler
  1 sibling, 2 replies; 11+ messages in thread
From: Arnd Bergmann @ 2017-01-10 16:15 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: Theodore Ts'o, Alexander Viro, Matthew Wilcox, Jan Kara,
	Andrew Morton, Dave Chinner, Dan Williams, linux-fsdevel,
	linux-kernel

On Monday, November 28, 2016 2:24:00 PM CET Ross Zwisler wrote:
> On Mon, Nov 28, 2016 at 10:12:17PM +0100, Arnd Bergmann wrote:
> > Without the get_block based I/O, we get warnings when CONFIG_FS_IOMAP
> > is disabled:
> > 
> > fs/dax.c:736:12: error: ‘dax_insert_mapping’ defined but not used [-Werror=unused-function]
> > fs/dax.c:512:12: error: ‘copy_user_dax’ defined but not used [-Werror=unused-function]
> > fs/dax.c:490:12: error: ‘dax_load_hole’ defined but not used [-Werror=unused-function]
> > fs/dax.c:294:14: error: ‘grab_mapping_entry’ defined but not used [-Werror=unused-function]
> > 
> > This patch blindly marks those as __maybe_unused, which avoids the warnings.
> > However, I suspect that there is actually more code in this file that should
> > not be provided without CONFIG_FS_IOMAP even though we don't get a warning
> > for it, and that we actually want a different rework, so please treat this
> > as a bug report. I have applied the patch locally in my randconfig build
> > setup to avoid seeing the warnings.
> > 
> > Fixes: 5ac65736f740 ("dax: rip out get_block based IO support")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Thanks for the report.  I think the right way to deal with this is to just
> select FS_IOMAP when we pull in the DAX code.  I sent out a patch last week
> that does this:
> 
> https://lkml.org/lkml/2016/11/23/591

It seems we never got agreement on the approach, and we still get the
warnings above in v4.10. Should we use my patch to fix up 4.10 and get
a clean build again? It no longer applies, but I have a rebased version
that I can send.

	Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dax: try to avoid unused function warnings
  2017-01-10 16:15   ` Arnd Bergmann
@ 2017-01-10 19:44     ` Ross Zwisler
  2017-01-10 22:29     ` [PATCH] dax: fix build warnings with FS_DAX and !FS_IOMAP Ross Zwisler
  1 sibling, 0 replies; 11+ messages in thread
From: Ross Zwisler @ 2017-01-10 19:44 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Ross Zwisler, Theodore Ts'o, Alexander Viro, Matthew Wilcox,
	Jan Kara, Andrew Morton, Dave Chinner, Dan Williams,
	linux-fsdevel, linux-kernel

On Tue, Jan 10, 2017 at 05:15:57PM +0100, Arnd Bergmann wrote:
> On Monday, November 28, 2016 2:24:00 PM CET Ross Zwisler wrote:
> > On Mon, Nov 28, 2016 at 10:12:17PM +0100, Arnd Bergmann wrote:
> > > Without the get_block based I/O, we get warnings when CONFIG_FS_IOMAP
> > > is disabled:
> > > 
> > > fs/dax.c:736:12: error: ‘dax_insert_mapping’ defined but not used [-Werror=unused-function]
> > > fs/dax.c:512:12: error: ‘copy_user_dax’ defined but not used [-Werror=unused-function]
> > > fs/dax.c:490:12: error: ‘dax_load_hole’ defined but not used [-Werror=unused-function]
> > > fs/dax.c:294:14: error: ‘grab_mapping_entry’ defined but not used [-Werror=unused-function]
> > > 
> > > This patch blindly marks those as __maybe_unused, which avoids the warnings.
> > > However, I suspect that there is actually more code in this file that should
> > > not be provided without CONFIG_FS_IOMAP even though we don't get a warning
> > > for it, and that we actually want a different rework, so please treat this
> > > as a bug report. I have applied the patch locally in my randconfig build
> > > setup to avoid seeing the warnings.
> > > 
> > > Fixes: 5ac65736f740 ("dax: rip out get_block based IO support")
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > 
> > Thanks for the report.  I think the right way to deal with this is to just
> > select FS_IOMAP when we pull in the DAX code.  I sent out a patch last week
> > that does this:
> > 
> > https://lkml.org/lkml/2016/11/23/591
> 
> It seems we never got agreement on the approach, and we still get the
> warnings above in v4.10. Should we use my patch to fix up 4.10 and get
> a clean build again? It no longer applies, but I have a rebased version
> that I can send.
> 
> 	Arnd

Thanks for the reminder, this one slipped through the cracks.  I still think
the right way to go with this is to fix up DAX's dependence on FS_IOMAP.  I'll
send out an updated patch.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] dax: fix build warnings with FS_DAX and !FS_IOMAP
  2017-01-10 16:15   ` Arnd Bergmann
  2017-01-10 19:44     ` Ross Zwisler
@ 2017-01-10 22:29     ` Ross Zwisler
  2017-01-11  8:15       ` Jan Kara
  2017-01-11  9:02       ` Christoph Hellwig
  1 sibling, 2 replies; 11+ messages in thread
From: Ross Zwisler @ 2017-01-10 22:29 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel
  Cc: Ross Zwisler, Darrick J. Wong, Theodore Ts'o, Alexander Viro,
	Andreas Dilger, Christoph Hellwig, Dan Williams, Jan Kara,
	Matthew Wilcox, linux-ext4, linux-fsdevel, linux-nvdimm,
	Arnd Bergmann

As reported by Arnd:

https://lkml.org/lkml/2017/1/10/756

Compiling with the following configuration:

  # CONFIG_EXT2_FS is not set
  # CONFIG_EXT4_FS is not set
  # CONFIG_XFS_FS is not set
  # CONFIG_FS_IOMAP depends on the above filesystems, as is not set
  CONFIG_FS_DAX=y

generates build warnings about unused functions in fs/dax.c:

fs/dax.c:878:12: warning: ‘dax_insert_mapping’ defined but not used [-Wunused-function]
 static int dax_insert_mapping(struct address_space *mapping,
            ^~~~~~~~~~~~~~~~~~
fs/dax.c:572:12: warning: ‘copy_user_dax’ defined but not used [-Wunused-function]
 static int copy_user_dax(struct block_device *bdev, sector_t sector, size_t size,
            ^~~~~~~~~~~~~
fs/dax.c:542:12: warning: ‘dax_load_hole’ defined but not used [-Wunused-function]
 static int dax_load_hole(struct address_space *mapping, void **entry,
            ^~~~~~~~~~~~~
fs/dax.c:312:14: warning: ‘grab_mapping_entry’ defined but not used [-Wunused-function]
 static void *grab_mapping_entry(struct address_space *mapping, pgoff_t index,
              ^~~~~~~~~~~~~~~~~~

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.

This allows us to remove conditional selections of FS_IOMAP when FS_DAX was
present for ext2 and ext4, and to remove an #ifdef in fs/dax.c.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
---
 fs/Kconfig      | 1 +
 fs/dax.c        | 2 --
 fs/ext2/Kconfig | 1 -
 fs/ext4/Kconfig | 1 -
 4 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/Kconfig b/fs/Kconfig
index c2a377c..83eab52 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 5c74f60..083b950 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -969,7 +969,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);
@@ -1407,4 +1406,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.
 
diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig
index 7b90691..e38039f 100644
--- a/fs/ext4/Kconfig
+++ b/fs/ext4/Kconfig
@@ -37,7 +37,6 @@ config EXT4_FS
 	select CRC16
 	select CRYPTO
 	select CRYPTO_CRC32C
-	select FS_IOMAP if FS_DAX
 	help
 	  This is the next generation of the ext3 filesystem.
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] dax: fix build warnings with FS_DAX and !FS_IOMAP
  2017-01-10 22:29     ` [PATCH] dax: fix build warnings with FS_DAX and !FS_IOMAP Ross Zwisler
@ 2017-01-11  8:15       ` Jan Kara
  2017-01-11  9:02       ` Christoph Hellwig
  1 sibling, 0 replies; 11+ messages in thread
From: Jan Kara @ 2017-01-11  8:15 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: Andrew Morton, linux-kernel, Darrick J. Wong, Theodore Ts'o,
	Alexander Viro, Andreas Dilger, Christoph Hellwig, Dan Williams,
	Jan Kara, Matthew Wilcox, linux-ext4, linux-fsdevel,
	linux-nvdimm, Arnd Bergmann

On Tue 10-01-17 15:29:43, Ross Zwisler wrote:
> As reported by Arnd:
> 
> https://lkml.org/lkml/2017/1/10/756
> 
> Compiling with the following configuration:
> 
>   # CONFIG_EXT2_FS is not set
>   # CONFIG_EXT4_FS is not set
>   # CONFIG_XFS_FS is not set
>   # CONFIG_FS_IOMAP depends on the above filesystems, as is not set
>   CONFIG_FS_DAX=y
> 
> generates build warnings about unused functions in fs/dax.c:
> 
> fs/dax.c:878:12: warning: ‘dax_insert_mapping’ defined but not used [-Wunused-function]
>  static int dax_insert_mapping(struct address_space *mapping,
>             ^~~~~~~~~~~~~~~~~~
> fs/dax.c:572:12: warning: ‘copy_user_dax’ defined but not used [-Wunused-function]
>  static int copy_user_dax(struct block_device *bdev, sector_t sector, size_t size,
>             ^~~~~~~~~~~~~
> fs/dax.c:542:12: warning: ‘dax_load_hole’ defined but not used [-Wunused-function]
>  static int dax_load_hole(struct address_space *mapping, void **entry,
>             ^~~~~~~~~~~~~
> fs/dax.c:312:14: warning: ‘grab_mapping_entry’ defined but not used [-Wunused-function]
>  static void *grab_mapping_entry(struct address_space *mapping, pgoff_t index,
>               ^~~~~~~~~~~~~~~~~~
> 
> 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.
> 
> This allows us to remove conditional selections of FS_IOMAP when FS_DAX was
> present for ext2 and ext4, and to remove an #ifdef in fs/dax.c.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Christoph Hellwig <hch@lst.de>

Looks good. I agree that DAX without FS_IOMAP does not make sense. You can
add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
> ---
>  fs/Kconfig      | 1 +
>  fs/dax.c        | 2 --
>  fs/ext2/Kconfig | 1 -
>  fs/ext4/Kconfig | 1 -
>  4 files changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index c2a377c..83eab52 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 5c74f60..083b950 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -969,7 +969,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);
> @@ -1407,4 +1406,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.
>  
> diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig
> index 7b90691..e38039f 100644
> --- a/fs/ext4/Kconfig
> +++ b/fs/ext4/Kconfig
> @@ -37,7 +37,6 @@ config EXT4_FS
>  	select CRC16
>  	select CRYPTO
>  	select CRYPTO_CRC32C
> -	select FS_IOMAP if FS_DAX
>  	help
>  	  This is the next generation of the ext3 filesystem.
>  
> -- 
> 2.7.4
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] dax: fix build warnings with FS_DAX and !FS_IOMAP
  2017-01-10 22:29     ` [PATCH] dax: fix build warnings with FS_DAX and !FS_IOMAP Ross Zwisler
  2017-01-11  8:15       ` Jan Kara
@ 2017-01-11  9:02       ` Christoph Hellwig
  1 sibling, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2017-01-11  9:02 UTC (permalink / raw)
  To: Ross Zwisler
  Cc: Andrew Morton, linux-kernel, Darrick J. Wong, Theodore Ts'o,
	Alexander Viro, Andreas Dilger, Christoph Hellwig, Dan Williams,
	Jan Kara, Matthew Wilcox, linux-ext4, linux-fsdevel,
	linux-nvdimm, Arnd Bergmann

Looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-01-11  9:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-28 21:12 [PATCH] dax: try to avoid unused function warnings Arnd Bergmann
2016-11-28 21:24 ` Ross Zwisler
2016-11-28 22:13   ` Dan Williams
2016-11-28 22:51     ` Ross Zwisler
2016-11-28 23:08       ` Dan Williams
2016-11-29  2:10         ` Dave Chinner
2017-01-10 16:15   ` Arnd Bergmann
2017-01-10 19:44     ` Ross Zwisler
2017-01-10 22:29     ` [PATCH] dax: fix build warnings with FS_DAX and !FS_IOMAP Ross Zwisler
2017-01-11  8:15       ` Jan Kara
2017-01-11  9:02       ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).