2021-02-07 19:07:50

by Chaitanya Kulkarni

[permalink] [raw]
Subject: [RFC PATCH 0/8] use core page calls instead of kmaps

This RFC is based on the discussion going on the linux-fsdevel [1].

I've tested this on the brd and null_blk. The fio verify job seems to
run without any error on the top of the original series applied [1].

Any feedback is welcome to move this forward.

-ck

[1] https://lore.kernel.org/linux-fsdevel/[email protected]/T/#m53145c155fa3631595594877da96a3a75b71e909

Chaitanya Kulkarni (8):
brd: use memcpy_from_page() in copy_from_brd()
brd: use memcpy_from_page() in copy_from_brd()
null_blk: use memcpy_page() in copy_to_nullb()
null_blk: use memcpy_page() in copy_from_nullb()
ext4: use memcpy_from_page() in pagecache_read()
ext4: use memcpy_to_page() in pagecache_write()
f2fs: use memcpy_from_page() in pagecache_read()
f2fs: use memcpy_to_page() in pagecache_write()

drivers/block/brd.c | 17 ++++++-----------
drivers/block/null_blk/main.c | 23 ++++++-----------------
fs/ext4/verity.c | 10 ++--------
fs/f2fs/verity.c | 10 ++--------
4 files changed, 16 insertions(+), 44 deletions(-)

--
2.22.1


2021-02-07 19:08:04

by Chaitanya Kulkarni

[permalink] [raw]
Subject: [RFC PATCH 5/8] ext4: use memcpy_from_page() in pagecache_read()

Signed-off-by: Chaitanya Kulkarni <[email protected]>
---
fs/ext4/verity.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
index 5b7ba8f71153..c8e07f8a792d 100644
--- a/fs/ext4/verity.c
+++ b/fs/ext4/verity.c
@@ -45,16 +45,13 @@ static int pagecache_read(struct inode *inode, void *buf, size_t count,
size_t n = min_t(size_t, count,
PAGE_SIZE - offset_in_page(pos));
struct page *page;
- void *addr;

page = read_mapping_page(inode->i_mapping, pos >> PAGE_SHIFT,
NULL);
if (IS_ERR(page))
return PTR_ERR(page);

- addr = kmap_atomic(page);
- memcpy(buf, addr + offset_in_page(pos), n);
- kunmap_atomic(addr);
+ memcpy_from_page(buf, page, offset_in_page(pos), n);

put_page(page);

--
2.22.1

2021-02-07 19:08:15

by Chaitanya Kulkarni

[permalink] [raw]
Subject: [RFC PATCH 3/8] null_blk: use memcpy_page() in copy_to_nullb()

Signed-off-by: Chaitanya Kulkarni <[email protected]>
---
drivers/block/null_blk/main.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index d6c821d48090..c9b6db82b07c 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1010,7 +1010,6 @@ static int copy_to_nullb(struct nullb *nullb, struct page *source,
size_t temp, count = 0;
unsigned int offset;
struct nullb_page *t_page;
- void *dst, *src;

while (count < n) {
temp = min_t(size_t, nullb->dev->blocksize, n - count);
@@ -1024,11 +1023,7 @@ static int copy_to_nullb(struct nullb *nullb, struct page *source,
if (!t_page)
return -ENOSPC;

- src = kmap_atomic(source);
- dst = kmap_atomic(t_page->page);
- memcpy(dst + offset, src + off + count, temp);
- kunmap_atomic(dst);
- kunmap_atomic(src);
+ memcpy_page(t_page->page, offset, source, off + count, temp);

__set_bit(sector & SECTOR_MASK, t_page->bitmap);

--
2.22.1

2021-02-07 19:08:18

by Chaitanya Kulkarni

[permalink] [raw]
Subject: [RFC PATCH 1/8] brd: use memcpy_from_page() in copy_from_brd()

Signed-off-by: Chaitanya Kulkarni <[email protected]>
---
drivers/block/brd.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index cb5c872ac9b2..d41b7d489e9f 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -226,11 +226,9 @@ static void copy_from_brd(void *dst, struct brd_device *brd,

copy = min_t(size_t, n, PAGE_SIZE - offset);
page = brd_lookup_page(brd, sector);
- if (page) {
- src = kmap_atomic(page);
- memcpy(dst, src + offset, copy);
- kunmap_atomic(src);
- } else
+ if (page)
+ memcpy_from_page(dst, page, offset, copy);
+ else
memset(dst, 0, copy);

if (copy < n) {
--
2.22.1

2021-02-07 19:08:37

by Chaitanya Kulkarni

[permalink] [raw]
Subject: [RFC PATCH 2/8] brd: use memcpy_from_page() in copy_from_brd()

Signed-off-by: Chaitanya Kulkarni <[email protected]>
---
drivers/block/brd.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index d41b7d489e9f..c1f6d768a1b3 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -220,7 +220,6 @@ static void copy_from_brd(void *dst, struct brd_device *brd,
sector_t sector, size_t n)
{
struct page *page;
- void *src;
unsigned int offset = (sector & (PAGE_SECTORS-1)) << SECTOR_SHIFT;
size_t copy;

@@ -236,11 +235,9 @@ static void copy_from_brd(void *dst, struct brd_device *brd,
sector += copy >> SECTOR_SHIFT;
copy = n - copy;
page = brd_lookup_page(brd, sector);
- if (page) {
- src = kmap_atomic(page);
- memcpy(dst, src, copy);
- kunmap_atomic(src);
- } else
+ if (page)
+ memcpy_from_page(dst, page, offset, copy);
+ else
memset(dst, 0, copy);
}
}
--
2.22.1

2021-02-07 19:08:49

by Chaitanya Kulkarni

[permalink] [raw]
Subject: [RFC PATCH 8/8] f2fs: use memcpy_to_page() in pagecache_write()

Signed-off-by: Chaitanya Kulkarni <[email protected]>
---
fs/f2fs/verity.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
index 44e057bdc416..ca019685a944 100644
--- a/fs/f2fs/verity.c
+++ b/fs/f2fs/verity.c
@@ -79,7 +79,6 @@ static int pagecache_write(struct inode *inode, const void *buf, size_t count,
PAGE_SIZE - offset_in_page(pos));
struct page *page;
void *fsdata;
- void *addr;
int res;

res = pagecache_write_begin(NULL, inode->i_mapping, pos, n, 0,
@@ -87,9 +86,7 @@ static int pagecache_write(struct inode *inode, const void *buf, size_t count,
if (res)
return res;

- addr = kmap_atomic(page);
- memcpy(addr + offset_in_page(pos), buf, n);
- kunmap_atomic(addr);
+ memcpy_to_page(page, offset_in_page(pos) buf, n);

res = pagecache_write_end(NULL, inode->i_mapping, pos, n, n,
page, fsdata);
--
2.22.1

2021-02-07 19:09:28

by Chaitanya Kulkarni

[permalink] [raw]
Subject: [RFC PATCH 4/8] null_blk: use memcpy_page() in copy_from_nullb()

Signed-off-by: Chaitanya Kulkarni <[email protected]>
---
drivers/block/null_blk/main.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index c9b6db82b07c..1c0e1a295e90 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1042,7 +1042,6 @@ static int copy_from_nullb(struct nullb *nullb, struct page *dest,
size_t temp, count = 0;
unsigned int offset;
struct nullb_page *t_page;
- void *dst, *src;

while (count < n) {
temp = min_t(size_t, nullb->dev->blocksize, n - count);
@@ -1051,16 +1050,11 @@ static int copy_from_nullb(struct nullb *nullb, struct page *dest,
t_page = null_lookup_page(nullb, sector, false,
!null_cache_active(nullb));

- dst = kmap_atomic(dest);
- if (!t_page) {
- memset(dst + off + count, 0, temp);
- goto next;
- }
- src = kmap_atomic(t_page->page);
- memcpy(dst + off + count, src + offset, temp);
- kunmap_atomic(src);
-next:
- kunmap_atomic(dst);
+ if (t_page)
+ memcpy_page(dest, off + count, t_page->page, offset,
+ temp);
+ else
+ zero_user(dest, off + count, temp);

count += temp;
sector += temp >> SECTOR_SHIFT;
--
2.22.1

2021-02-07 19:10:34

by Chaitanya Kulkarni

[permalink] [raw]
Subject: [RFC PATCH 6/8] ext4: use memcpy_to_page() in pagecache_write()

Signed-off-by: Chaitanya Kulkarni <[email protected]>
---
fs/ext4/verity.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
index c8e07f8a792d..cc4b046490b0 100644
--- a/fs/ext4/verity.c
+++ b/fs/ext4/verity.c
@@ -77,7 +77,6 @@ static int pagecache_write(struct inode *inode, const void *buf, size_t count,
PAGE_SIZE - offset_in_page(pos));
struct page *page;
void *fsdata;
- void *addr;
int res;

res = pagecache_write_begin(NULL, inode->i_mapping, pos, n, 0,
@@ -85,9 +84,7 @@ static int pagecache_write(struct inode *inode, const void *buf, size_t count,
if (res)
return res;

- addr = kmap_atomic(page);
- memcpy(addr + offset_in_page(pos), buf, n);
- kunmap_atomic(addr);
+ memcpy_to_page(page, offset_in_page(pos), buf, n);

res = pagecache_write_end(NULL, inode->i_mapping, pos, n, n,
page, fsdata);
--
2.22.1

2021-02-07 19:11:00

by Chaitanya Kulkarni

[permalink] [raw]
Subject: [RFC PATCH 7/8] f2fs: use memcpy_from_page() in pagecache_read()

Signed-off-by: Chaitanya Kulkarni <[email protected]>
---
fs/f2fs/verity.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
index 054ec852b5ea..44e057bdc416 100644
--- a/fs/f2fs/verity.c
+++ b/fs/f2fs/verity.c
@@ -47,16 +47,13 @@ static int pagecache_read(struct inode *inode, void *buf, size_t count,
size_t n = min_t(size_t, count,
PAGE_SIZE - offset_in_page(pos));
struct page *page;
- void *addr;

page = read_mapping_page(inode->i_mapping, pos >> PAGE_SHIFT,
NULL);
if (IS_ERR(page))
return PTR_ERR(page);

- addr = kmap_atomic(page);
- memcpy(buf, addr + offset_in_page(pos), n);
- kunmap_atomic(addr);
+ memcpy_from_page(buf, page, offset_in_page(pos), n);

put_page(page);

--
2.22.1

2021-02-07 19:14:14

by Chaitanya Kulkarni

[permalink] [raw]
Subject: Re: [RFC PATCH 0/8] use core page calls instead of kmaps

On 2/7/21 11:04, Chaitanya Kulkarni wrote:
> Chaitanya Kulkarni (8):
> brd: use memcpy_from_page() in copy_from_brd()
> brd: use memcpy_from_page() in copy_from_brd()
I'm aware that couple of places in brd code we can use memcpy_to_page()
and get rid the local variable, once I get some feedback I'll add those
to the V1.

2021-02-08 04:38:18

by Ira Weiny

[permalink] [raw]
Subject: Re: [RFC PATCH 2/8] brd: use memcpy_from_page() in copy_from_brd()

On Sun, Feb 07, 2021 at 11:04:19AM -0800, Chaitanya Kulkarni wrote:
> Signed-off-by: Chaitanya Kulkarni <[email protected]>
> ---
> drivers/block/brd.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/brd.c b/drivers/block/brd.c
> index d41b7d489e9f..c1f6d768a1b3 100644
> --- a/drivers/block/brd.c
> +++ b/drivers/block/brd.c
> @@ -220,7 +220,6 @@ static void copy_from_brd(void *dst, struct brd_device *brd,
> sector_t sector, size_t n)
> {
> struct page *page;
> - void *src;
> unsigned int offset = (sector & (PAGE_SECTORS-1)) << SECTOR_SHIFT;
> size_t copy;
>
> @@ -236,11 +235,9 @@ static void copy_from_brd(void *dst, struct brd_device *brd,
> sector += copy >> SECTOR_SHIFT;
> copy = n - copy;
> page = brd_lookup_page(brd, sector);
> - if (page) {
> - src = kmap_atomic(page);
> - memcpy(dst, src, copy);
> - kunmap_atomic(src);
> - } else
> + if (page)
> + memcpy_from_page(dst, page, offset, copy);

Why 'offset'?

Ira

> + else
> memset(dst, 0, copy);
> }
> }
> --
> 2.22.1
>

2021-02-08 04:43:26

by Ira Weiny

[permalink] [raw]
Subject: Re: [RFC PATCH 0/8] use core page calls instead of kmaps

On Sun, Feb 07, 2021 at 07:10:41PM +0000, Chaitanya Kulkarni wrote:
> On 2/7/21 11:04, Chaitanya Kulkarni wrote:
> > Chaitanya Kulkarni (8):
> > brd: use memcpy_from_page() in copy_from_brd()
> > brd: use memcpy_from_page() in copy_from_brd()
> I'm aware that couple of places in brd code we can use memcpy_to_page()
> and get rid the local variable, once I get some feedback I'll add those
> to the V1.

Except for the one comment I had this series look's good to me.

Reviewed-by: Ira Weiny <[email protected]>

Thanks!
Ira

2021-02-08 05:51:30

by Chaitanya Kulkarni

[permalink] [raw]
Subject: Re: [RFC PATCH 2/8] brd: use memcpy_from_page() in copy_from_brd()

>> @@ -236,11 +235,9 @@ static void copy_from_brd(void *dst, struct brd_device *brd,
>> sector += copy >> SECTOR_SHIFT;
>> copy = n - copy;
>> page = brd_lookup_page(brd, sector);
>> - if (page) {
>> - src = kmap_atomic(page);
>> - memcpy(dst, src, copy);
>> - kunmap_atomic(src);
>> - } else
>> + if (page)
>> + memcpy_from_page(dst, page, offset, copy);
> Why 'offset'?
Will fix it in the V1.
> Ira
>
>> + else
>> memset(dst, 0, copy);
>> }
>> }
>> --
>> 2.22.1
>>

2021-02-08 05:52:03

by Chaitanya Kulkarni

[permalink] [raw]
Subject: Re: [RFC PATCH 0/8] use core page calls instead of kmaps

On 2/7/21 20:42, Ira Weiny wrote:
> On Sun, Feb 07, 2021 at 07:10:41PM +0000, Chaitanya Kulkarni wrote:
>> On 2/7/21 11:04, Chaitanya Kulkarni wrote:
>>> Chaitanya Kulkarni (8):
>>> brd: use memcpy_from_page() in copy_from_brd()
>>> brd: use memcpy_from_page() in copy_from_brd()
>> I'm aware that couple of places in brd code we can use memcpy_to_page()
>> and get rid the local variable, once I get some feedback I'll add those
>> to the V1.
> Except for the one comment I had this series look's good to me.
>
> Reviewed-by: Ira Weiny <[email protected]>
>
> Thanks!
> Ira
>
>
Thanks Ira, I'll add your fix in the V1 and send with your review-by tag,
when this code is upstream.

Thanks a lot for doing this we can get rid of go in null_blk which makes
code smooth.

2021-02-10 18:06:58

by Eric Biggers

[permalink] [raw]
Subject: Re: [RFC PATCH 8/8] f2fs: use memcpy_to_page() in pagecache_write()

On Sun, Feb 07, 2021 at 11:04:25AM -0800, Chaitanya Kulkarni wrote:
> Signed-off-by: Chaitanya Kulkarni <[email protected]>

No explanation in commit message? There isn't much explanation needed for this,
but there should be at least one sentence.

Likewise for the other patches.

> fs/f2fs/verity.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
> index 44e057bdc416..ca019685a944 100644
> --- a/fs/f2fs/verity.c
> +++ b/fs/f2fs/verity.c
> @@ -79,7 +79,6 @@ static int pagecache_write(struct inode *inode, const void *buf, size_t count,
> PAGE_SIZE - offset_in_page(pos));
> struct page *page;
> void *fsdata;
> - void *addr;
> int res;
>
> res = pagecache_write_begin(NULL, inode->i_mapping, pos, n, 0,
> @@ -87,9 +86,7 @@ static int pagecache_write(struct inode *inode, const void *buf, size_t count,
> if (res)
> return res;
>
> - addr = kmap_atomic(page);
> - memcpy(addr + offset_in_page(pos), buf, n);
> - kunmap_atomic(addr);
> + memcpy_to_page(page, offset_in_page(pos) buf, n);

This is missing a comma between 'offset_in_page(pos)' and 'buf'.

Otherwise the patches to fs/{ext4,f2fs}/verity.c look fine; thanks for doing
this!

- Eric

2021-02-10 19:46:21

by Chaitanya Kulkarni

[permalink] [raw]
Subject: Re: [RFC PATCH 8/8] f2fs: use memcpy_to_page() in pagecache_write()

On 2/10/21 10:01, Eric Biggers wrote:
> On Sun, Feb 07, 2021 at 11:04:25AM -0800, Chaitanya Kulkarni wrote:
>> Signed-off-by: Chaitanya Kulkarni <[email protected]>
> No explanation in commit message? There isn't much explanation needed for this,
> but there should be at least one sentence.
>
> Likewise for the other patches.
>
Thanks Eric for your comment.

Sent out minimal RFC, I'll add detail explanation also having original
series
in the tree helps.