From: Dan Williams <dan.j.williams@intel.com>
To: Dave Chinner <david@fromorbit.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>, Jan Kara <jack@suse.cz>,
Arnd Bergmann <arnd@arndb.de>,
"Darrick J. Wong" <darrick.wong@oracle.com>,
linux-rdma@vger.kernel.org, Linux API <linux-api@vger.kernel.org>,
"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
linux-xfs@vger.kernel.org, Linux MM <linux-mm@kvack.org>,
iommu@lists.linux-foundation.org,
Alexander Viro <viro@zeniv.linux.org.uk>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Jeff Layton <jlayton@poochiereds.net>,
Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v8 06/14] xfs: wire up MAP_DIRECT
Date: Tue, 10 Oct 2017 19:12:50 -0700 [thread overview]
Message-ID: <CAPcyv4jXRUFA1b7+7DMhZn2BZnxq_zfQ0vH-hQ8pKzKyaN4qag@mail.gmail.com> (raw)
In-Reply-To: <20171011010922.GY3666@dastard>
On Tue, Oct 10, 2017 at 6:09 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Tue, Oct 10, 2017 at 07:49:30AM -0700, Dan Williams wrote:
>> @@ -1009,6 +1019,22 @@ xfs_file_llseek(
>> }
>>
>> /*
>> + * MAP_DIRECT faults can only be serviced while the FL_LAYOUT lease is
>> + * valid. See map_direct_invalidate.
>> + */
>> +static int
>> +xfs_can_fault_direct(
>> + struct vm_area_struct *vma)
>> +{
>> + if (!xfs_vma_is_direct(vma))
>> + return 0;
>> +
>> + if (!test_map_direct_valid(vma->vm_private_data))
>> + return VM_FAULT_SIGBUS;
>> + return 0;
>> +}
>
> Better, but I'm going to be an annoying pedant here: a "can
> <something>" check should return a boolean true/false.
>
> Also, it's a bit jarring to see that a non-direct VMA that /can't/
> do direct faults returns the same thing as a direct-vma that /can/
> do direct faults, so a couple of extra comments for people who will
> quickly forget how this code works (i.e. me) will be helpful. Say
> something like this:
>
> /*
> * MAP_DIRECT faults can only be serviced while the FL_LAYOUT lease is
> * valid. See map_direct_invalidate.
> */
> static bool
> xfs_vma_has_direct_lease(
> struct vm_area_struct *vma)
> {
> /* Non MAP_DIRECT vmas do not require layout leases */
> if (!xfs_vma_is_direct(vma))
> return true;
>
> if (!test_map_direct_valid(vma->vm_private_data))
> return false;
>
> /* We have a valid lease */
> return true;
> }
>
> .....
> if (!xfs_vma_has_direct_lease(vma)) {
> ret = VM_FAULT_SIGBUS;
> goto out_unlock;
> }
> ....
Looks good to me.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
To: Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org>
Cc: "J. Bruce Fields"
<bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>,
Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>,
Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
"Darrick J. Wong"
<darrick.wong-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux API <linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
"linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org"
<linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org>,
linux-xfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux MM <linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org>,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
Alexander Viro
<viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>,
linux-fsdevel
<linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>,
Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Subject: Re: [PATCH v8 06/14] xfs: wire up MAP_DIRECT
Date: Tue, 10 Oct 2017 19:12:50 -0700 [thread overview]
Message-ID: <CAPcyv4jXRUFA1b7+7DMhZn2BZnxq_zfQ0vH-hQ8pKzKyaN4qag@mail.gmail.com> (raw)
In-Reply-To: <20171011010922.GY3666@dastard>
On Tue, Oct 10, 2017 at 6:09 PM, Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> wrote:
> On Tue, Oct 10, 2017 at 07:49:30AM -0700, Dan Williams wrote:
>> @@ -1009,6 +1019,22 @@ xfs_file_llseek(
>> }
>>
>> /*
>> + * MAP_DIRECT faults can only be serviced while the FL_LAYOUT lease is
>> + * valid. See map_direct_invalidate.
>> + */
>> +static int
>> +xfs_can_fault_direct(
>> + struct vm_area_struct *vma)
>> +{
>> + if (!xfs_vma_is_direct(vma))
>> + return 0;
>> +
>> + if (!test_map_direct_valid(vma->vm_private_data))
>> + return VM_FAULT_SIGBUS;
>> + return 0;
>> +}
>
> Better, but I'm going to be an annoying pedant here: a "can
> <something>" check should return a boolean true/false.
>
> Also, it's a bit jarring to see that a non-direct VMA that /can't/
> do direct faults returns the same thing as a direct-vma that /can/
> do direct faults, so a couple of extra comments for people who will
> quickly forget how this code works (i.e. me) will be helpful. Say
> something like this:
>
> /*
> * MAP_DIRECT faults can only be serviced while the FL_LAYOUT lease is
> * valid. See map_direct_invalidate.
> */
> static bool
> xfs_vma_has_direct_lease(
> struct vm_area_struct *vma)
> {
> /* Non MAP_DIRECT vmas do not require layout leases */
> if (!xfs_vma_is_direct(vma))
> return true;
>
> if (!test_map_direct_valid(vma->vm_private_data))
> return false;
>
> /* We have a valid lease */
> return true;
> }
>
> .....
> if (!xfs_vma_has_direct_lease(vma)) {
> ret = VM_FAULT_SIGBUS;
> goto out_unlock;
> }
> ....
Looks good to me.
WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams@intel.com>
To: Dave Chinner <david@fromorbit.com>
Cc: "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
linux-xfs@vger.kernel.org, Jan Kara <jack@suse.cz>,
Arnd Bergmann <arnd@arndb.de>,
"Darrick J. Wong" <darrick.wong@oracle.com>,
linux-rdma@vger.kernel.org, Linux API <linux-api@vger.kernel.org>,
iommu@lists.linux-foundation.org, Christoph Hellwig <hch@lst.de>,
"J. Bruce Fields" <bfields@fieldses.org>,
Linux MM <linux-mm@kvack.org>, Jeff Moyer <jmoyer@redhat.com>,
Alexander Viro <viro@zeniv.linux.org.uk>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Jeff Layton <jlayton@poochiereds.net>,
Ross Zwisler <ross.zwisler@linux.intel.com>
Subject: Re: [PATCH v8 06/14] xfs: wire up MAP_DIRECT
Date: Tue, 10 Oct 2017 19:12:50 -0700 [thread overview]
Message-ID: <CAPcyv4jXRUFA1b7+7DMhZn2BZnxq_zfQ0vH-hQ8pKzKyaN4qag@mail.gmail.com> (raw)
In-Reply-To: <20171011010922.GY3666@dastard>
On Tue, Oct 10, 2017 at 6:09 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Tue, Oct 10, 2017 at 07:49:30AM -0700, Dan Williams wrote:
>> @@ -1009,6 +1019,22 @@ xfs_file_llseek(
>> }
>>
>> /*
>> + * MAP_DIRECT faults can only be serviced while the FL_LAYOUT lease is
>> + * valid. See map_direct_invalidate.
>> + */
>> +static int
>> +xfs_can_fault_direct(
>> + struct vm_area_struct *vma)
>> +{
>> + if (!xfs_vma_is_direct(vma))
>> + return 0;
>> +
>> + if (!test_map_direct_valid(vma->vm_private_data))
>> + return VM_FAULT_SIGBUS;
>> + return 0;
>> +}
>
> Better, but I'm going to be an annoying pedant here: a "can
> <something>" check should return a boolean true/false.
>
> Also, it's a bit jarring to see that a non-direct VMA that /can't/
> do direct faults returns the same thing as a direct-vma that /can/
> do direct faults, so a couple of extra comments for people who will
> quickly forget how this code works (i.e. me) will be helpful. Say
> something like this:
>
> /*
> * MAP_DIRECT faults can only be serviced while the FL_LAYOUT lease is
> * valid. See map_direct_invalidate.
> */
> static bool
> xfs_vma_has_direct_lease(
> struct vm_area_struct *vma)
> {
> /* Non MAP_DIRECT vmas do not require layout leases */
> if (!xfs_vma_is_direct(vma))
> return true;
>
> if (!test_map_direct_valid(vma->vm_private_data))
> return false;
>
> /* We have a valid lease */
> return true;
> }
>
> .....
> if (!xfs_vma_has_direct_lease(vma)) {
> ret = VM_FAULT_SIGBUS;
> goto out_unlock;
> }
> ....
Looks good to me.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams@intel.com>
To: Dave Chinner <david@fromorbit.com>
Cc: "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
linux-xfs@vger.kernel.org, Jan Kara <jack@suse.cz>,
Arnd Bergmann <arnd@arndb.de>,
"Darrick J. Wong" <darrick.wong@oracle.com>,
linux-rdma@vger.kernel.org, Linux API <linux-api@vger.kernel.org>,
iommu@lists.linux-foundation.org, Christoph Hellwig <hch@lst.de>,
"J. Bruce Fields" <bfields@fieldses.org>,
Linux MM <linux-mm@kvack.org>, Jeff Moyer <jmoyer@redhat.com>,
Alexander Viro <viro@zeniv.linux.org.uk>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Jeff Layton <jlayton@poochiereds.net>,
Ross Zwisler <ross.zwisler@linux.intel.com>
Subject: Re: [PATCH v8 06/14] xfs: wire up MAP_DIRECT
Date: Tue, 10 Oct 2017 19:12:50 -0700 [thread overview]
Message-ID: <CAPcyv4jXRUFA1b7+7DMhZn2BZnxq_zfQ0vH-hQ8pKzKyaN4qag@mail.gmail.com> (raw)
In-Reply-To: <20171011010922.GY3666@dastard>
On Tue, Oct 10, 2017 at 6:09 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Tue, Oct 10, 2017 at 07:49:30AM -0700, Dan Williams wrote:
>> @@ -1009,6 +1019,22 @@ xfs_file_llseek(
>> }
>>
>> /*
>> + * MAP_DIRECT faults can only be serviced while the FL_LAYOUT lease is
>> + * valid. See map_direct_invalidate.
>> + */
>> +static int
>> +xfs_can_fault_direct(
>> + struct vm_area_struct *vma)
>> +{
>> + if (!xfs_vma_is_direct(vma))
>> + return 0;
>> +
>> + if (!test_map_direct_valid(vma->vm_private_data))
>> + return VM_FAULT_SIGBUS;
>> + return 0;
>> +}
>
> Better, but I'm going to be an annoying pedant here: a "can
> <something>" check should return a boolean true/false.
>
> Also, it's a bit jarring to see that a non-direct VMA that /can't/
> do direct faults returns the same thing as a direct-vma that /can/
> do direct faults, so a couple of extra comments for people who will
> quickly forget how this code works (i.e. me) will be helpful. Say
> something like this:
>
> /*
> * MAP_DIRECT faults can only be serviced while the FL_LAYOUT lease is
> * valid. See map_direct_invalidate.
> */
> static bool
> xfs_vma_has_direct_lease(
> struct vm_area_struct *vma)
> {
> /* Non MAP_DIRECT vmas do not require layout leases */
> if (!xfs_vma_is_direct(vma))
> return true;
>
> if (!test_map_direct_valid(vma->vm_private_data))
> return false;
>
> /* We have a valid lease */
> return true;
> }
>
> .....
> if (!xfs_vma_has_direct_lease(vma)) {
> ret = VM_FAULT_SIGBUS;
> goto out_unlock;
> }
> ....
Looks good to me.
next prev parent reply other threads:[~2017-10-11 2:09 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-10 14:48 [PATCH v8 00/14] MAP_DIRECT for DAX RDMA and userspace flush Dan Williams
2017-10-10 14:48 ` Dan Williams
2017-10-10 14:48 ` Dan Williams
2017-10-10 14:48 ` Dan Williams
2017-10-10 14:49 ` [PATCH v8 01/14] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-11 7:43 ` Jan Kara
2017-10-11 7:43 ` Jan Kara
2017-10-11 7:43 ` Jan Kara
2017-10-11 14:15 ` Dan Williams
2017-10-11 14:15 ` Dan Williams
2017-10-11 14:15 ` Dan Williams
2017-10-11 14:15 ` Dan Williams
2017-10-10 14:49 ` [PATCH v8 02/14] fs, mm: pass fd to ->mmap_validate() Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` [PATCH v8 03/14] fs: MAP_DIRECT core Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` [PATCH v8 04/14] xfs: prepare xfs_break_layouts() for reuse with MAP_DIRECT Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-11 0:46 ` Dave Chinner
2017-10-11 0:46 ` Dave Chinner
2017-10-11 2:12 ` Dan Williams
2017-10-11 2:12 ` Dan Williams
2017-10-11 2:12 ` Dan Williams
2017-10-10 14:49 ` [PATCH v8 05/14] fs, xfs, iomap: introduce iomap_can_allocate() Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` [PATCH v8 06/14] xfs: wire up MAP_DIRECT Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-11 1:09 ` Dave Chinner
2017-10-11 1:09 ` Dave Chinner
2017-10-11 1:09 ` Dave Chinner
2017-10-11 2:12 ` Dan Williams [this message]
2017-10-11 2:12 ` Dan Williams
2017-10-11 2:12 ` Dan Williams
2017-10-11 2:12 ` Dan Williams
2017-10-10 14:49 ` [PATCH v8 07/14] iommu, dma-mapping: introduce dma_get_iommu_domain() Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` [PATCH v8 08/14] fs, mapdirect: introduce ->lease_direct() Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` [PATCH v8 09/14] xfs: wire up ->lease_direct() Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:49 ` [PATCH v8 10/14] device-dax: " Dan Williams
2017-10-10 14:49 ` Dan Williams
2017-10-10 14:50 ` [PATCH v8 11/14] iommu: up-level sg_num_pages() from amd-iommu Dan Williams
2017-10-10 14:50 ` Dan Williams
2017-10-10 14:50 ` Dan Williams
2017-10-10 14:50 ` [PATCH v8 12/14] iommu/vt-d: use iommu_num_sg_pages Dan Williams
2017-10-10 14:50 ` Dan Williams
2017-10-10 14:50 ` Dan Williams
2017-10-10 14:50 ` Dan Williams
2017-10-10 14:50 ` [PATCH v8 13/14] IB/core: use MAP_DIRECT to fix / enable RDMA to DAX mappings Dan Williams
2017-10-10 14:50 ` Dan Williams
2017-10-10 14:50 ` Dan Williams
2017-10-11 11:54 ` Joerg Roedel
2017-10-11 11:54 ` Joerg Roedel
2017-10-11 16:01 ` Dan Williams
2017-10-11 16:01 ` Dan Williams
2017-10-11 16:01 ` Dan Williams
2017-10-11 16:01 ` Dan Williams
2017-10-10 14:50 ` [PATCH v8 14/14] tools/testing/nvdimm: enable rdma unit tests Dan Williams
2017-10-10 14:50 ` Dan Williams
2017-10-10 14:50 ` Dan Williams
2017-10-10 14:50 ` Dan Williams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPcyv4jXRUFA1b7+7DMhZn2BZnxq_zfQ0vH-hQ8pKzKyaN4qag@mail.gmail.com \
--to=dan.j.williams@intel.com \
--cc=arnd@arndb.de \
--cc=bfields@fieldses.org \
--cc=darrick.wong@oracle.com \
--cc=david@fromorbit.com \
--cc=hch@lst.de \
--cc=iommu@lists.linux-foundation.org \
--cc=jack@suse.cz \
--cc=jlayton@poochiereds.net \
--cc=linux-api@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-nvdimm@lists.01.org \
--cc=linux-rdma@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.