Assigning NULL to gfn of dax_direct_access if useless

PATCH In review Huaisheng Ye 2 participants 8 messages Jul 4, 2018

Subsystems (5)

VFS s390 libnvdimm: non-volatile memory device Filesystem direct access (DAX) +1 more subsystems

lore

Share & tools

lore

Huaisheng Ye

author

2018-07-04 16:38 UTC #0 lore raw

v2 cover Assigning NULL to gfn of dax_direct_access if useless

From: Huaisheng Ye <[email protected]>

Changes since v1 [1]:
* Collect Jan's reviewed-by.
* According to Dan's suggestion, update the unit test infrastructure tools/testing/nvdimm/pmem-dax.c for checking the validity of gfn.

[1]: https://lkml.org/lkml/2018/7/4/81

---

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 all direct_access need to check the validity of pointer pfn for NULL assignment. If pfn equals to NULL, it doesn't need to calculate its value.

---

Huaisheng Ye (4):
nvdimm/pmem: check the validity of the pointer pfn nvdimm/pmem-dax: check the validity of the pointer pfn s390/block/dcssblk: check the validity of the pointer pfn fs/dax: Assign NULL to gfn of dax_direct_access if useless

 drivers/nvdimm/pmem.c           | 3 ++-
 drivers/s390/block/dcssblk.c    | 3 ++-
 fs/dax.c                        | 9 +++------
 tools/testing/nvdimm/pmem-dax.c | 6 ++++--
 4 files changed, 11 insertions(+), 10 deletions(-)

Signature · 1 line

1.8.3.1

Huaisheng Ye

author

2018-07-04 16:38 UTC #1 lore raw

PATCH v2 1/4 nvdimm/pmem: check the validity of the pointer pfn

From: Huaisheng Ye <[email protected]>

direct_access needs to check the validity of pointer pfn for NULL assignment. If pfn equals to NULL, it doesn't need to calculate the value.

Signed-off-by Huaisheng Ye <[email protected]>

Reviewed-by Jan Kara <[email protected]>

---
 drivers/nvdimm/pmem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/nvdimm/pmem.c +2 -1

index 9d71492..018f990 100644
@@ -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

Signature · 1 line

1.8.3.1

Huaisheng Ye

author

2018-07-04 16:38 UTC #2 lore raw

PATCH v2 2/4 nvdimm/pmem-dax: check the validity of the pointer pfn

In reply to Huaisheng Ye

From: Huaisheng Ye <[email protected]>

direct_access needs to check the validity of pointer pfn for NULL assignment. If pfn equals to NULL, it doesn't need to calculate the value.

Signed-off-by Huaisheng Ye <[email protected]>

Reviewed-by Jan Kara <[email protected]>

---
 tools/testing/nvdimm/pmem-dax.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

tools/testing/nvdimm/pmem-dax.c +4 -2

index b53596a..d4cb528 100644
@@ -33,7 +33,8 @@ long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff,
 
 		*kaddr = pmem->virt_addr + offset;
 		page = vmalloc_to_page(pmem->virt_addr + offset);
-		*pfn = page_to_pfn_t(page);
+		if (pfn)
+			*pfn = page_to_pfn_t(page);
 		pr_debug_ratelimited("%s: pmem: %p pgoff: %#lx pfn: %#lx\n",
 				__func__, pmem, pgoff, page_to_pfn(page));

@@ -41,7 +42,8 @@ long __pmem_direct_access(struct pmem_device *pmem, pgoff_t pgoff,
 	}
 
 	*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

Signature · 1 line

1.8.3.1

Huaisheng Ye

author

2018-07-04 16:38 UTC #3 lore raw

PATCH v2 3/4 s390/block/dcssblk: check the validity of the pointer pfn

In reply to Huaisheng Ye

From: Huaisheng Ye <[email protected]>

direct_access needs to check the validity of pointer pfn for NULL assignment. If pfn equals to NULL, it doesn't need to calculate the value.

Signed-off-by Huaisheng Ye <[email protected]>

Reviewed-by Jan Kara <[email protected]>

---
 drivers/s390/block/dcssblk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/s390/block/dcssblk.c +2 -1

index 0a312e4..5cdfa02 100644
@@ -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;

Signature · 1 line

1.8.3.1

Huaisheng Ye

author

2018-07-04 16:38 UTC #4 lore raw

PATCH v2 4/4 fs/dax: Assign NULL to gfn of dax_direct_access if useless

In reply to Huaisheng Ye

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]>

Reviewed-by Jan Kara <[email protected]>

---
 fs/dax.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

fs/dax.c +3 -6

index aaec72de..aa75dfd 100644
@@ -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;

Signature · 1 line

1.8.3.1

Dan Williams

2018-07-04 16:48 UTC #5 lore raw

In reply to Huaisheng Ye

Quoted 7 lines · Huaisheng Ye

From: Huaisheng Ye <[email protected]>
Changes since v1 [1]:
* Collect Jan's reviewed-by.
* According to Dan's suggestion, update the unit test infrastructure tools/testing/nvdimm/pmem-dax.c for checking the validity of gfn.

Quoted 5 lines

[1]: https://lkml.org/lkml/2018/7/4/81
---
Some functions within fs/dax don't need to get gfn from direct_access.

Any reason you are calling it 'gfn'? I'm assuming that is a typo and it should be 'pfn'.

Huaisheng Ye

author

2018-07-04 16:55 UTC #6 lore raw

---- On Thu, 05 Jul 2018 00:48:40 +0800 Dan Williams <[email protected]> wrote ----

Quoted 8 lines · Huaisheng Ye

Quoted 7 lines
From: Huaisheng Ye <[email protected]>
Changes since v1 [1]:
* Collect Jan's reviewed-by.
* According to Dan's suggestion, update the unit test infrastructure tools/testing/nvdimm/pmem-dax.c for checking the validity of gfn.

Quoted 8 lines

Quoted 5 lines
[1]: https://lkml.org/lkml/2018/7/4/81
---
Some functions within fs/dax don't need to get gfn from direct_access.
Any reason you are calling it 'gfn'? I'm assuming that is a typo and it should be 'pfn'.

Oh, sorry.
I made a mistake, it is a typo. I will correct it right now.

---
Cheers,
Huaisheng Ye

Dan Williams

2018-07-04 17:16 UTC #7 lore raw

Quoted 17 lines · Huaisheng Ye

---- On Thu, 05 Jul 2018 00:48:40 +0800 Dan Williams <[email protected]> wrote ----
Quoted 12 lines · Huaisheng Ye
Quoted 11 lines
From: Huaisheng Ye <[email protected]>
Changes since v1 [1]:
* Collect Jan's reviewed-by.
* According to Dan's suggestion, update the unit test infrastructure tools/testing/nvdimm/pmem-dax.c for checking the validity of gfn.
[1]: https://lkml.org/lkml/2018/7/4/81
---

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Some functions within fs/dax don't need to get gfn from direct_access.
Any reason you are calling it 'gfn'? I'm assuming that is a typo and it should be 'pfn'.
Oh, sorry.
I made a mistake, it is a typo. I will correct it right now.

Don't worry about sending a new version. I'll fix it up when I apply the series.