From: Shiyang Ruan <[email protected]>
The page fault part of fsdax code is little complex. In order to add CoW
feature and make it easy to understand, I was suggested to factor some
helper functions to simplify the current dax code.
This is separated from the previous patchset called "V3 fsdax,xfs: Add
reflink&dedupe support for fsdax", and the previous comments are here[1].
[1]: https://patchwork.kernel.org/project/linux-nvdimm/patch/[email protected]/
Changes from V2:
- fix the type of 'major' in patch 2
- Rebased on v5.12-rc8
Changes from V1:
- fix Ritesh's email address
- simplify return logic in dax_fault_cow_page()
(Rebased on v5.12-rc8)
==
Shiyang Ruan (3):
fsdax: Factor helpers to simplify dax fault code
fsdax: Factor helper: dax_fault_actor()
fsdax: Output address in dax_iomap_pfn() and rename it
fs/dax.c | 443 +++++++++++++++++++++++++++++--------------------------
1 file changed, 234 insertions(+), 209 deletions(-)
--
2.31.1
Add address output in dax_iomap_pfn() in order to perform a memcpy() in
CoW case. Since this function both output address and pfn, rename it to
dax_iomap_direct_access().
Signed-off-by: Shiyang Ruan <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Ritesh Harjani <[email protected]>
---
fs/dax.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index f99e33de2036..48a97905c0c3 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -998,8 +998,8 @@ static sector_t dax_iomap_sector(struct iomap *iomap, loff_t pos)
return (iomap->addr + (pos & PAGE_MASK) - iomap->offset) >> 9;
}
-static int dax_iomap_pfn(struct iomap *iomap, loff_t pos, size_t size,
- pfn_t *pfnp)
+static int dax_iomap_direct_access(struct iomap *iomap, loff_t pos, size_t size,
+ void **kaddr, pfn_t *pfnp)
{
const sector_t sector = dax_iomap_sector(iomap, pos);
pgoff_t pgoff;
@@ -1011,11 +1011,13 @@ static int dax_iomap_pfn(struct iomap *iomap, loff_t pos, size_t size,
return rc;
id = dax_read_lock();
length = dax_direct_access(iomap->dax_dev, pgoff, PHYS_PFN(size),
- NULL, pfnp);
+ kaddr, pfnp);
if (length < 0) {
rc = length;
goto out;
}
+ if (!pfnp)
+ goto out_check_addr;
rc = -EINVAL;
if (PFN_PHYS(length) < size)
goto out;
@@ -1025,6 +1027,12 @@ static int dax_iomap_pfn(struct iomap *iomap, loff_t pos, size_t size,
if (length > 1 && !pfn_t_devmap(*pfnp))
goto out;
rc = 0;
+
+out_check_addr:
+ if (!kaddr)
+ goto out;
+ if (!*kaddr)
+ rc = -EFAULT;
out:
dax_read_unlock(id);
return rc;
@@ -1389,7 +1397,7 @@ static vm_fault_t dax_fault_actor(struct vm_fault *vmf, pfn_t *pfnp,
return pmd ? VM_FAULT_FALLBACK : VM_FAULT_SIGBUS;
}
- err = dax_iomap_pfn(iomap, pos, size, &pfn);
+ err = dax_iomap_direct_access(iomap, pos, size, NULL, &pfn);
if (err)
return pmd ? VM_FAULT_FALLBACK : dax_fault_return(err);
--
2.31.1
Hi, Dan
Do you have any comments on this?
--
Thanks,
Ruan Shiyang.
> -----Original Message-----
> From: Shiyang Ruan <[email protected]>
> Sent: Thursday, April 22, 2021 9:45 PM
> Subject: [PATCH v3 0/3] fsdax: Factor helper functions to simplify the code
>
> From: Shiyang Ruan <[email protected]>
>
> The page fault part of fsdax code is little complex. In order to add CoW feature
> and make it easy to understand, I was suggested to factor some helper functions
> to simplify the current dax code.
>
> This is separated from the previous patchset called "V3 fsdax,xfs: Add
> reflink&dedupe support for fsdax", and the previous comments are here[1].
>
> [1]:
> https://patchwork.kernel.org/project/linux-nvdimm/patch/20210319015237.99
> [email protected]/
>
> Changes from V2:
> - fix the type of 'major' in patch 2
> - Rebased on v5.12-rc8
>
> Changes from V1:
> - fix Ritesh's email address
> - simplify return logic in dax_fault_cow_page()
>
> (Rebased on v5.12-rc8)
> ==
>
> Shiyang Ruan (3):
> fsdax: Factor helpers to simplify dax fault code
> fsdax: Factor helper: dax_fault_actor()
> fsdax: Output address in dax_iomap_pfn() and rename it
>
> fs/dax.c | 443 +++++++++++++++++++++++++++++--------------------------
> 1 file changed, 234 insertions(+), 209 deletions(-)
>
> --
> 2.31.1
>
> Hi, Dan
>
> Do you have any comments on this?
Ping
>
>
> --
> Thanks,
> Ruan Shiyang.
>
> > -----Original Message-----
> > From: Shiyang Ruan <[email protected]>
> > Sent: Thursday, April 22, 2021 9:45 PM
> > Subject: [PATCH v3 0/3] fsdax: Factor helper functions to simplify the
> > code
> >
> > From: Shiyang Ruan <[email protected]>
> >
> > The page fault part of fsdax code is little complex. In order to add
> > CoW feature and make it easy to understand, I was suggested to factor
> > some helper functions to simplify the current dax code.
> >
> > This is separated from the previous patchset called "V3 fsdax,xfs: Add
> > reflink&dedupe support for fsdax", and the previous comments are here[1].
> >
> > [1]:
> > https://patchwork.kernel.org/project/linux-nvdimm/patch/20210319015237
> > .99
> > [email protected]/
> >
> > Changes from V2:
> > - fix the type of 'major' in patch 2
> > - Rebased on v5.12-rc8
> >
> > Changes from V1:
> > - fix Ritesh's email address
> > - simplify return logic in dax_fault_cow_page()
> >
> > (Rebased on v5.12-rc8)
> > ==
> >
> > Shiyang Ruan (3):
> > fsdax: Factor helpers to simplify dax fault code
> > fsdax: Factor helper: dax_fault_actor()
> > fsdax: Output address in dax_iomap_pfn() and rename it
> >
> > fs/dax.c | 443
> > +++++++++++++++++++++++++++++--------------------------
> > 1 file changed, 234 insertions(+), 209 deletions(-)
> >
> > --
> > 2.31.1
>
>
On Thu, Apr 22, 2021 at 09:45:01PM +0800, Shiyang Ruan wrote:
> Add address output in dax_iomap_pfn() in order to perform a memcpy() in
> CoW case. Since this function both output address and pfn, rename it to
> dax_iomap_direct_access().
>
> Signed-off-by: Shiyang Ruan <[email protected]>
> Reviewed-by: Christoph Hellwig <[email protected]>
> Reviewed-by: Ritesh Harjani <[email protected]>
Looks pretty simple to me,
Reviewed-by: Darrick J. Wong <[email protected]>
--D
> ---
> fs/dax.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index f99e33de2036..48a97905c0c3 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -998,8 +998,8 @@ static sector_t dax_iomap_sector(struct iomap *iomap, loff_t pos)
> return (iomap->addr + (pos & PAGE_MASK) - iomap->offset) >> 9;
> }
>
> -static int dax_iomap_pfn(struct iomap *iomap, loff_t pos, size_t size,
> - pfn_t *pfnp)
> +static int dax_iomap_direct_access(struct iomap *iomap, loff_t pos, size_t size,
> + void **kaddr, pfn_t *pfnp)
> {
> const sector_t sector = dax_iomap_sector(iomap, pos);
> pgoff_t pgoff;
> @@ -1011,11 +1011,13 @@ static int dax_iomap_pfn(struct iomap *iomap, loff_t pos, size_t size,
> return rc;
> id = dax_read_lock();
> length = dax_direct_access(iomap->dax_dev, pgoff, PHYS_PFN(size),
> - NULL, pfnp);
> + kaddr, pfnp);
> if (length < 0) {
> rc = length;
> goto out;
> }
> + if (!pfnp)
> + goto out_check_addr;
> rc = -EINVAL;
> if (PFN_PHYS(length) < size)
> goto out;
> @@ -1025,6 +1027,12 @@ static int dax_iomap_pfn(struct iomap *iomap, loff_t pos, size_t size,
> if (length > 1 && !pfn_t_devmap(*pfnp))
> goto out;
> rc = 0;
> +
> +out_check_addr:
> + if (!kaddr)
> + goto out;
> + if (!*kaddr)
> + rc = -EFAULT;
> out:
> dax_read_unlock(id);
> return rc;
> @@ -1389,7 +1397,7 @@ static vm_fault_t dax_fault_actor(struct vm_fault *vmf, pfn_t *pfnp,
> return pmd ? VM_FAULT_FALLBACK : VM_FAULT_SIGBUS;
> }
>
> - err = dax_iomap_pfn(iomap, pos, size, &pfn);
> + err = dax_iomap_direct_access(iomap, pos, size, NULL, &pfn);
> if (err)
> return pmd ? VM_FAULT_FALLBACK : dax_fault_return(err);
>
> --
> 2.31.1
>
>
>
On Fri, May 14, 2021 at 10:23:25AM +0000, [email protected] wrote:
> >
> > Hi, Dan
> >
> > Do you have any comments on this?
>
> Ping
This patchset has acquired multiple RVB tags but (AFAIK) Dan still
hasn't responded. To get this moving again, it might be time to send
this direct to Al with a note that the maintainer hasn't been
responsive.
--D
> >
> >
> > --
> > Thanks,
> > Ruan Shiyang.
> >
> > > -----Original Message-----
> > > From: Shiyang Ruan <[email protected]>
> > > Sent: Thursday, April 22, 2021 9:45 PM
> > > Subject: [PATCH v3 0/3] fsdax: Factor helper functions to simplify the
> > > code
> > >
> > > From: Shiyang Ruan <[email protected]>
> > >
> > > The page fault part of fsdax code is little complex. In order to add
> > > CoW feature and make it easy to understand, I was suggested to factor
> > > some helper functions to simplify the current dax code.
> > >
> > > This is separated from the previous patchset called "V3 fsdax,xfs: Add
> > > reflink&dedupe support for fsdax", and the previous comments are here[1].
> > >
> > > [1]:
> > > https://patchwork.kernel.org/project/linux-nvdimm/patch/20210319015237
> > > .99
> > > [email protected]/
> > >
> > > Changes from V2:
> > > - fix the type of 'major' in patch 2
> > > - Rebased on v5.12-rc8
> > >
> > > Changes from V1:
> > > - fix Ritesh's email address
> > > - simplify return logic in dax_fault_cow_page()
> > >
> > > (Rebased on v5.12-rc8)
> > > ==
> > >
> > > Shiyang Ruan (3):
> > > fsdax: Factor helpers to simplify dax fault code
> > > fsdax: Factor helper: dax_fault_actor()
> > > fsdax: Output address in dax_iomap_pfn() and rename it
> > >
> > > fs/dax.c | 443
> > > +++++++++++++++++++++++++++++--------------------------
> > > 1 file changed, 234 insertions(+), 209 deletions(-)
> > >
> > > --
> > > 2.31.1
> >
> >
>
> -----Original Message-----
> From: Darrick J. Wong <[email protected]>
> Subject: Re: [PATCH v3 0/3] fsdax: Factor helper functions to simplify the code
>
> On Fri, May 14, 2021 at 10:23:25AM +0000, [email protected] wrote:
> > >
> > > Hi, Dan
> > >
> > > Do you have any comments on this?
> >
> > Ping
>
> This patchset has acquired multiple RVB tags but (AFAIK) Dan still hasn't
> responded. To get this moving again, it might be time to send this direct to Al
> with a note that the maintainer hasn't been responsive.
Thanks a lot, I'll send to him.
--
Ruan.
>
> --D
>
> > >
> > >
> > > --
> > > Thanks,
> > > Ruan Shiyang.
> > >
> > > > -----Original Message-----
> > > > From: Shiyang Ruan <[email protected]>
> > > > Sent: Thursday, April 22, 2021 9:45 PM
> > > > Subject: [PATCH v3 0/3] fsdax: Factor helper functions to simplify
> > > > the code
> > > >
> > > > From: Shiyang Ruan <[email protected]>
> > > >
> > > > The page fault part of fsdax code is little complex. In order to
> > > > add CoW feature and make it easy to understand, I was suggested to
> > > > factor some helper functions to simplify the current dax code.
> > > >
> > > > This is separated from the previous patchset called "V3 fsdax,xfs:
> > > > Add reflink&dedupe support for fsdax", and the previous comments are
> here[1].
> > > >
> > > > [1]:
> > > > https://patchwork.kernel.org/project/linux-nvdimm/patch/2021031901
> > > > 5237
> > > > .99
> > > > [email protected]/
> > > >
> > > > Changes from V2:
> > > > - fix the type of 'major' in patch 2
> > > > - Rebased on v5.12-rc8
> > > >
> > > > Changes from V1:
> > > > - fix Ritesh's email address
> > > > - simplify return logic in dax_fault_cow_page()
> > > >
> > > > (Rebased on v5.12-rc8)
> > > > ==
> > > >
> > > > Shiyang Ruan (3):
> > > > fsdax: Factor helpers to simplify dax fault code
> > > > fsdax: Factor helper: dax_fault_actor()
> > > > fsdax: Output address in dax_iomap_pfn() and rename it
> > > >
> > > > fs/dax.c | 443
> > > > +++++++++++++++++++++++++++++--------------------------
> > > > 1 file changed, 234 insertions(+), 209 deletions(-)
> > > >
> > > > --
> > > > 2.31.1
> > >
> > >
> >