From: Huaisheng Ye <[email protected]>
Some functions within fs/dax don't need to get gfn from direct_access.
Assigning NULL to gfn of dax_direct_access is more intuitive and simple
than offering a useless local variable.
So direct_access needs to check validity of the pointer pfn For NULL
assignment.
Signed-off-by: Huaisheng Ye <[email protected]>
---
drivers/nvdimm/pmem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index 9d71492..018f990 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -233,7 +233,8 @@ __weak long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff,
PFN_PHYS(nr_pages))))
return -EIO;
*kaddr = pmem->virt_addr + offset;
- *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags);
+ if (pfn)
+ *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags);
/*
* If badblocks are present, limit known good range to the
--
1.8.3.1
From: Huaisheng Ye <[email protected]>
Some functions within fs/dax don't need to get gfn from direct_access.
Assigning NULL to gfn of dax_direct_access is more intuitive and simple
than offering a useless local variable.
Signed-off-by: Huaisheng Ye <[email protected]>
---
fs/dax.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index aaec72de..aa75dfd 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -550,7 +550,6 @@ static int copy_user_dax(struct block_device *bdev, struct dax_device *dax_dev,
{
void *vto, *kaddr;
pgoff_t pgoff;
- pfn_t pfn;
long rc;
int id;
@@ -559,7 +558,7 @@ static int copy_user_dax(struct block_device *bdev, struct dax_device *dax_dev,
return rc;
id = dax_read_lock();
- rc = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), &kaddr, &pfn);
+ rc = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), &kaddr, NULL);
if (rc < 0) {
dax_read_unlock(id);
return rc;
@@ -961,7 +960,6 @@ int __dax_zero_page_range(struct block_device *bdev,
pgoff_t pgoff;
long rc, id;
void *kaddr;
- pfn_t pfn;
rc = bdev_dax_pgoff(bdev, sector, PAGE_SIZE, &pgoff);
if (rc)
@@ -969,7 +967,7 @@ int __dax_zero_page_range(struct block_device *bdev,
id = dax_read_lock();
rc = dax_direct_access(dax_dev, pgoff, 1, &kaddr,
- &pfn);
+ NULL);
if (rc < 0) {
dax_read_unlock(id);
return rc;
@@ -1024,7 +1022,6 @@ int __dax_zero_page_range(struct block_device *bdev,
ssize_t map_len;
pgoff_t pgoff;
void *kaddr;
- pfn_t pfn;
if (fatal_signal_pending(current)) {
ret = -EINTR;
@@ -1036,7 +1033,7 @@ int __dax_zero_page_range(struct block_device *bdev,
break;
map_len = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size),
- &kaddr, &pfn);
+ &kaddr, NULL);
if (map_len < 0) {
ret = map_len;
break;
--
1.8.3.1
From: Huaisheng Ye <[email protected]>
Some functions within fs/dax don't need to get gfn from direct_access.
Assigning NULL to gfn of dax_direct_access is more intuitive and simple
than offering a useless local variable.
So direct_access needs to check validity of the pointer pfn For NULL
assignment. If pfn equals to NULL, it doesn't need to calculate its
value.
Signed-off-by: Huaisheng Ye <[email protected]>
---
drivers/s390/block/dcssblk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index 0a312e4..5cdfa02 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -916,7 +916,8 @@ static DEVICE_ATTR(save, S_IWUSR | S_IRUSR, dcssblk_save_show,
dev_sz = dev_info->end - dev_info->start + 1;
*kaddr = (void *) dev_info->start + offset;
- *pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset),
+ if (pfn)
+ *pfn = __pfn_to_pfn_t(PFN_DOWN(dev_info->start + offset),
PFN_DEV|PFN_SPECIAL);
return (dev_sz - offset) / PAGE_SIZE;
--
1.8.3.1
On Wed 04-07-18 14:40:58, Huaisheng Ye wrote:
> From: Huaisheng Ye <[email protected]>
>
> Some functions within fs/dax don't need to get gfn from direct_access.
> Assigning NULL to gfn of dax_direct_access is more intuitive and simple
> than offering a useless local variable.
>
> Signed-off-by: Huaisheng Ye <[email protected]>
I like this. You can add:
Reviewed-by: Jan Kara <[email protected]>
for the series.
Honza
> ---
> fs/dax.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index aaec72de..aa75dfd 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -550,7 +550,6 @@ static int copy_user_dax(struct block_device *bdev, struct dax_device *dax_dev,
> {
> void *vto, *kaddr;
> pgoff_t pgoff;
> - pfn_t pfn;
> long rc;
> int id;
>
> @@ -559,7 +558,7 @@ static int copy_user_dax(struct block_device *bdev, struct dax_device *dax_dev,
> return rc;
>
> id = dax_read_lock();
> - rc = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), &kaddr, &pfn);
> + rc = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size), &kaddr, NULL);
> if (rc < 0) {
> dax_read_unlock(id);
> return rc;
> @@ -961,7 +960,6 @@ int __dax_zero_page_range(struct block_device *bdev,
> pgoff_t pgoff;
> long rc, id;
> void *kaddr;
> - pfn_t pfn;
>
> rc = bdev_dax_pgoff(bdev, sector, PAGE_SIZE, &pgoff);
> if (rc)
> @@ -969,7 +967,7 @@ int __dax_zero_page_range(struct block_device *bdev,
>
> id = dax_read_lock();
> rc = dax_direct_access(dax_dev, pgoff, 1, &kaddr,
> - &pfn);
> + NULL);
> if (rc < 0) {
> dax_read_unlock(id);
> return rc;
> @@ -1024,7 +1022,6 @@ int __dax_zero_page_range(struct block_device *bdev,
> ssize_t map_len;
> pgoff_t pgoff;
> void *kaddr;
> - pfn_t pfn;
>
> if (fatal_signal_pending(current)) {
> ret = -EINTR;
> @@ -1036,7 +1033,7 @@ int __dax_zero_page_range(struct block_device *bdev,
> break;
>
> map_len = dax_direct_access(dax_dev, pgoff, PHYS_PFN(size),
> - &kaddr, &pfn);
> + &kaddr, NULL);
> if (map_len < 0) {
> ret = map_len;
> break;
> --
> 1.8.3.1
>
>
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
---- On Wed, 04 Jul 2018 19:30:12 +0800 Jan Kara <[email protected]> wrote ----
> On Wed 04-07-18 14:40:58, Huaisheng Ye wrote:
> > From: Huaisheng Ye <[email protected]>
> >
> > Some functions within fs/dax don't need to get gfn from direct_access.
> > Assigning NULL to gfn of dax_direct_access is more intuitive and simple
> > than offering a useless local variable.
> >
> > Signed-off-by: Huaisheng Ye <[email protected]>
>
> I like this. You can add:
>
> Reviewed-by: Jan Kara <[email protected]>
>
> for the series.
>
> Honza
>
I am so happy you like them, thank you very much.
---
Cheers,
Huaisheng
On Wed, Jul 4, 2018 at 6:07 AM, Huaisheng Ye <[email protected]> wrote:
> ---- On Wed, 04 Jul 2018 19:30:12 +0800 Jan Kara <[email protected]> wrote ----
> > On Wed 04-07-18 14:40:58, Huaisheng Ye wrote:
> > > From: Huaisheng Ye <[email protected]>
> > >
> > > Some functions within fs/dax don't need to get gfn from direct_access.
> > > Assigning NULL to gfn of dax_direct_access is more intuitive and simple
> > > than offering a useless local variable.
> > >
> > > Signed-off-by: Huaisheng Ye <[email protected]>
> >
> > I like this. You can add:
> >
> > Reviewed-by: Jan Kara <[email protected]>
> >
> > for the series.
> >
> > Honza
> >
> I am so happy you like them, thank you very much.
Yes, I like this too. In fact I have a similar patch in my tree
already that I have been preparing to send out. I am using it to delay
when we need to have the 'struct page' memmap for dax initialized.
Attached is the full patch, but the series is still a work in
progress.
On Tue, Jul 3, 2018 at 11:40 PM, Huaisheng Ye <[email protected]> wrote:
> From: Huaisheng Ye <[email protected]>
>
> Some functions within fs/dax don't need to get gfn from direct_access.
> Assigning NULL to gfn of dax_direct_access is more intuitive and simple
> than offering a useless local variable.
>
> So direct_access needs to check validity of the pointer pfn For NULL
> assignment.
>
> Signed-off-by: Huaisheng Ye <[email protected]>
> ---
> drivers/nvdimm/pmem.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index 9d71492..018f990 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -233,7 +233,8 @@ __weak long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff,
> PFN_PHYS(nr_pages))))
> return -EIO;
> *kaddr = pmem->virt_addr + offset;
> - *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags);
> + if (pfn)
> + *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags);
>
> /*
> * If badblocks are present, limit known good range to the
Looks good. You also need to update the unit test infrastructure
version of this operation in:
tools/testing/nvdimm/pmem-dax.c
On Wed, Jul 4, 2018 at 7:37 AM, Dan Williams <[email protected]> wrote:
> On Wed, Jul 4, 2018 at 6:07 AM, Huaisheng Ye <[email protected]> wrote:
>> ---- On Wed, 04 Jul 2018 19:30:12 +0800 Jan Kara <[email protected]> wrote ----
>> > On Wed 04-07-18 14:40:58, Huaisheng Ye wrote:
>> > > From: Huaisheng Ye <[email protected]>
>> > >
>> > > Some functions within fs/dax don't need to get gfn from direct_access.
>> > > Assigning NULL to gfn of dax_direct_access is more intuitive and simple
>> > > than offering a useless local variable.
>> > >
>> > > Signed-off-by: Huaisheng Ye <[email protected]>
>> >
>> > I like this. You can add:
>> >
>> > Reviewed-by: Jan Kara <[email protected]>
>> >
>> > for the series.
>> >
>> > Honza
>> >
>> I am so happy you like them, thank you very much.
>
> Yes, I like this too. In fact I have a similar patch in my tree
> already that I have been preparing to send out. I am using it to delay
> when we need to have the 'struct page' memmap for dax initialized.
> Attached is the full patch, but the series is still a work in
> progress.
Btw, I'll drop my version and apply your series since you got it
posted first and it can stand alone as its own cleanup.
> From: Dan Williams [mailto:[email protected]]
> Sent: Wednesday, July 04, 2018 10:40 PM
> On Tue, Jul 3, 2018 at 11:40 PM, Huaisheng Ye <[email protected]> wrote:
> > From: Huaisheng Ye <[email protected]>
> >
> > Some functions within fs/dax don't need to get gfn from direct_access.
> > Assigning NULL to gfn of dax_direct_access is more intuitive and simple
> > than offering a useless local variable.
> >
> > So direct_access needs to check validity of the pointer pfn For NULL
> > assignment.
> >
> > Signed-off-by: Huaisheng Ye <[email protected]>
> > ---
> > drivers/nvdimm/pmem.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> > index 9d71492..018f990 100644
> > --- a/drivers/nvdimm/pmem.c
> > +++ b/drivers/nvdimm/pmem.c
> > @@ -233,7 +233,8 @@ __weak long __pmem_direct_access(struct pmem_device *pmem,
> pgoff_t pgoff,
> > PFN_PHYS(nr_pages))))
> > return -EIO;
> > *kaddr = pmem->virt_addr + offset;
> > - *pfn = phys_to_pfn_t(pmem->phys_addr + offset, pmem->pfn_flags);
> > + if (pfn)
> > + *pfn = phys_to_pfn_t(pmem->phys_addr + offset,
> pmem->pfn_flags);
> >
> > /*
> > * If badblocks are present, limit known good range to the
>
> Looks good. You also need to update the unit test infrastructure
> version of this operation in:
>
> tools/testing/nvdimm/pmem-dax.c
Yes, you are right.
Cheers,
Huaisheng Ye
From: Dan Williams [mailto:[email protected]]
Sent: Wednesday, July 04, 2018 10:42 PM
>
> On Wed, Jul 4, 2018 at 7:37 AM, Dan Williams <[email protected]> wrote:
> > On Wed, Jul 4, 2018 at 6:07 AM, Huaisheng Ye <[email protected]> wrote:
> >> ---- On Wed, 04 Jul 2018 19:30:12 +0800 Jan Kara <[email protected]> wrote ----
> >> > On Wed 04-07-18 14:40:58, Huaisheng Ye wrote:
> >> > > From: Huaisheng Ye <[email protected]>
> >> > >
> >> > > Some functions within fs/dax don't need to get gfn from direct_access.
> >> > > Assigning NULL to gfn of dax_direct_access is more intuitive and simple
> >> > > than offering a useless local variable.
> >> > >
> >> > > Signed-off-by: Huaisheng Ye <[email protected]>
> >> >
> >> > I like this. You can add:
> >> >
> >> > Reviewed-by: Jan Kara <[email protected]>
> >> >
> >> > for the series.
> >> >
> >> > Honza
> >> >
> >> I am so happy you like them, thank you very much.
> >
> > Yes, I like this too. In fact I have a similar patch in my tree
> > already that I have been preparing to send out. I am using it to delay
> > when we need to have the 'struct page' memmap for dax initialized.
> > Attached is the full patch, but the series is still a work in
> > progress.
>
> Btw, I'll drop my version and apply your series since you got it
> posted first and it can stand alone as its own cleanup.
Hi Dan,
I will resend the series later, and thanks for your help.
Cheers,
Huaisheng Ye