linux-kernel - Re: [PATCH 6/7] mm, fs: introduce file_operations->post

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: <CAPcyv4jtO028KeZK7SdkOUsgMLGqgttLzBCYgH0M+RP3eAXf4A@mail.gmail.com>
Date:   Mon, 25 Sep 2017 16:38:45 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Ross Zwisler <ross.zwisler@...ux.intel.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        "J. Bruce Fields" <bfields@...ldses.org>,
        Christoph Hellwig <hch@....de>,
        Dave Chinner <david@...morbit.com>, Jan Kara <jack@...e.cz>,
        Jeff Layton <jlayton@...chiereds.net>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux MM <linux-mm@...ck.org>,
        "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        linux-xfs@...r.kernel.org
Subject: Re: [PATCH 6/7] mm, fs: introduce file_operations->post_mmap()

On Mon, Sep 25, 2017 at 4:14 PM, Ross Zwisler
<ross.zwisler@...ux.intel.com> wrote:
> When mappings are created the vma->vm_flags that they use vary based on
> whether the inode being mapped is using DAX or not.  This setup happens in
> XFS via mmap_region()=>call_mmap()=>xfs_file_mmap().
>
> For us to be able to safely use the DAX per-inode flag we need to prevent
> S_DAX transitions when any mappings are present, and we will do that by
> looking at the address_space->i_mmap tree and returning -EBUSY if any
> mappings are present.
>
> Unfortunately at the time that the filesystem's file_operations->mmap()
> entry point is called the mapping has not yet been added to the
> address_space->i_mmap tree.  This means that at that point in time we
> cannot determine whether or not the mapping will be set up to support DAX.
>
> Fix this by adding a new file_operations entry called post_mmap() which is
> called after the mapping has been added to the address_space->i_mmap tree.
> This post_mmap() op now happens at a time when we can be sure whether the
> mapping will use DAX or not, and we can set up the vma->vm_flags
> appropriately.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@...ux.intel.com>
> ---
>  fs/xfs/xfs_file.c  | 15 ++++++++++++++-
>  include/linux/fs.h |  1 +
>  mm/mmap.c          |  2 ++
>  3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 2816858..9d66aaa 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1087,9 +1087,21 @@ xfs_file_mmap(
>  {
>         file_accessed(filp);
>         vma->vm_ops = &xfs_file_vm_ops;
> +       return 0;
> +}
> +
> +/* This call happens during mmap(), after the vma has been inserted into the
> + * inode->i_mapping->i_mmap tree.  At this point the decision on whether or
> + * not to use DAX for this mapping has been set and will not change for the
> + * duration of the mapping.
> + */
> +STATIC void
> +xfs_file_post_mmap(
> +       struct file     *filp,
> +       struct vm_area_struct *vma)
> +{
>         if (IS_DAX(file_inode(filp)))
>                 vma->vm_flags |= VM_MIXEDMAP | VM_HUGEPAGE;

It's not clear to me what this is actually protecting? vma_is_dax()
returns true regardless of the vm_flags state , so what is the benefit
to delaying the vm_flags setting to ->post_mmap()?

Also, why is this a file_operation and not a vm_operation?