Re: [PATCH v4 04/18] dax: require 'struct page' by default for filesystem dax

[Gerald Schaefer <gerald.schaefer@de.ibm.com>]

On Sat, 23 Dec 2017 16:56:22 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> If a dax buffer from a device that does not map pages is passed to
> read(2) or write(2) as a target for direct-I/O it triggers SIGBUS. If
> gdb attempts to examine the contents of a dax buffer from a device that
> does not map pages it triggers SIGBUS. If fork(2) is called on a process
> with a dax mapping from a device that does not map pages it triggers
> SIGBUS. 'struct page' is required otherwise several kernel code paths
> break in surprising ways. Disable filesystem-dax on devices that do not
> map pages.
> 
> In addition to needing pfn_to_page() to be valid we also require devmap
> pages.  We need this to detect dax pages in the get_user_pages_fast()
> path and so that we can stop managing the VM_MIXEDMAP flag. For DAX
> drivers that have not supported get_user_pages() to date we allow them
> to opt-in to supporting DAX with the CONFIG_FS_DAX_LIMITED configuration
> option which requires ->direct_access() to return pfn_t_special() pfns.
> This leaves DAX support in brd disabled and scheduled for removal.
> 
> Note that when the initial dax support was being merged a few years back
> there was concern that struct page was unsuitable for use with next
> generation persistent memory devices. The theoretical concern was that
> struct page access, being such a hotly used data structure in the
> kernel, would lead to media wear out. While that was a reasonable
> conservative starting position it has not held true in practice. We have
> long since committed to using devm_memremap_pages() to support higher
> order kernel functionality that needs get_user_pages() and
> pfn_to_page().
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  arch/powerpc/platforms/Kconfig |    1 +
>  drivers/dax/super.c            |   10 ++++++++++
>  drivers/s390/block/Kconfig     |    1 +
>  fs/Kconfig                     |    7 +++++++
>  4 files changed, 19 insertions(+)

dcssblk seems to work fine, I did not see any SIGBUS while "executing
in place" from dcssblk with the current upstream kernel, maybe because
we only use dcssblk with fs dax in read-only mode.

Anyway, the dcssblk change is fine with me. I will look into adding
struct pages for dcssblk memory later, to make it work again with
this change, but for now I do not know of anyone needing this in the
upstream kernel.

Reviewed-by: Gerald Schaefer <gerald.schaefer@de.ibm.com>