2024-01-15 08:34:05

by Gao Xiang

[permalink] [raw]
Subject: [PATCH v2 3/4] erofs: Don't use certain internal folio_*() functions

From: David Howells <[email protected]>

Filesystems should use folio->index and folio->mapping, instead of
folio_index(folio), folio_mapping() and folio_file_mapping() since
they know that it's in the pagecache.

Change this automagically with:

perl -p -i -e 's/folio_mapping[(]([^)]*)[)]/\1->mapping/g' fs/erofs/*.c
perl -p -i -e 's/folio_file_mapping[(]([^)]*)[)]/\1->mapping/g' fs/erofs/*.c
perl -p -i -e 's/folio_index[(]([^)]*)[)]/\1->index/g' fs/erofs/*.c

Reported-by: Matthew Wilcox <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
Cc: Chao Yu <[email protected]>
Cc: Yue Hu <[email protected]>
Cc: Jeffle Xu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Gao Xiang <[email protected]>
---
Hi folks,

I tend to apply this patch upstream since compressed data fscache
adaption touches this part too. If there is no objection, I'm
going to take this patch separately for -next shortly..

Thanks,
Gao Xiang

Change since v1:
- a better commit message pointed out by Jeff Layton.

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

diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
index 87ff35bff8d5..bc12030393b2 100644
--- a/fs/erofs/fscache.c
+++ b/fs/erofs/fscache.c
@@ -165,10 +165,10 @@ static int erofs_fscache_read_folios_async(struct fscache_cookie *cookie,
static int erofs_fscache_meta_read_folio(struct file *data, struct folio *folio)
{
int ret;
- struct erofs_fscache *ctx = folio_mapping(folio)->host->i_private;
+ struct erofs_fscache *ctx = folio->mapping->host->i_private;
struct erofs_fscache_request *req;

- req = erofs_fscache_req_alloc(folio_mapping(folio),
+ req = erofs_fscache_req_alloc(folio->mapping,
folio_pos(folio), folio_size(folio));
if (IS_ERR(req)) {
folio_unlock(folio);
@@ -276,7 +276,7 @@ static int erofs_fscache_read_folio(struct file *file, struct folio *folio)
struct erofs_fscache_request *req;
int ret;

- req = erofs_fscache_req_alloc(folio_mapping(folio),
+ req = erofs_fscache_req_alloc(folio->mapping,
folio_pos(folio), folio_size(folio));
if (IS_ERR(req)) {
folio_unlock(folio);
--
2.39.3



2024-01-15 14:07:01

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] erofs: Don't use certain internal folio_*() functions

On Mon, Jan 15, 2024 at 04:33:37PM +0800, Gao Xiang wrote:
> From: David Howells <[email protected]>
>
> Filesystems should use folio->index and folio->mapping, instead of
> folio_index(folio), folio_mapping() and folio_file_mapping() since
> they know that it's in the pagecache.
>
> Change this automagically with:
>
> perl -p -i -e 's/folio_mapping[(]([^)]*)[)]/\1->mapping/g' fs/erofs/*.c
> perl -p -i -e 's/folio_file_mapping[(]([^)]*)[)]/\1->mapping/g' fs/erofs/*.c
> perl -p -i -e 's/folio_index[(]([^)]*)[)]/\1->index/g' fs/erofs/*.c
>
> Reported-by: Matthew Wilcox <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Reviewed-by: Jeff Layton <[email protected]>
> Cc: Chao Yu <[email protected]>
> Cc: Yue Hu <[email protected]>
> Cc: Jeffle Xu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Gao Xiang <[email protected]>
> ---
> Hi folks,
>
> I tend to apply this patch upstream since compressed data fscache
> adaption touches this part too. If there is no objection, I'm
> going to take this patch separately for -next shortly..

Could you change the subject? It's not that the functions are
"internal", it's that filesystems don't need to use them because they're
guaranteed to not see swap pages. Maybe just s/internal/unnecessary/

> Thanks,
> Gao Xiang
>
> Change since v1:
> - a better commit message pointed out by Jeff Layton.
>
> fs/erofs/fscache.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
> index 87ff35bff8d5..bc12030393b2 100644
> --- a/fs/erofs/fscache.c
> +++ b/fs/erofs/fscache.c
> @@ -165,10 +165,10 @@ static int erofs_fscache_read_folios_async(struct fscache_cookie *cookie,
> static int erofs_fscache_meta_read_folio(struct file *data, struct folio *folio)
> {
> int ret;
> - struct erofs_fscache *ctx = folio_mapping(folio)->host->i_private;
> + struct erofs_fscache *ctx = folio->mapping->host->i_private;
> struct erofs_fscache_request *req;
>
> - req = erofs_fscache_req_alloc(folio_mapping(folio),
> + req = erofs_fscache_req_alloc(folio->mapping,
> folio_pos(folio), folio_size(folio));
> if (IS_ERR(req)) {
> folio_unlock(folio);
> @@ -276,7 +276,7 @@ static int erofs_fscache_read_folio(struct file *file, struct folio *folio)
> struct erofs_fscache_request *req;
> int ret;
>
> - req = erofs_fscache_req_alloc(folio_mapping(folio),
> + req = erofs_fscache_req_alloc(folio->mapping,
> folio_pos(folio), folio_size(folio));
> if (IS_ERR(req)) {
> folio_unlock(folio);
> --
> 2.39.3
>

2024-01-15 14:47:04

by Gao Xiang

[permalink] [raw]
Subject: [PATCH v3 3/4] erofs: Don't use certain unnecessary folio_*() functions

From: David Howells <[email protected]>

Filesystems should use folio->index and folio->mapping, instead of
folio_index(folio), folio_mapping() and folio_file_mapping() since
they know that it's in the pagecache.

Change this automagically with:

perl -p -i -e 's/folio_mapping[(]([^)]*)[)]/\1->mapping/g' fs/erofs/*.c
perl -p -i -e 's/folio_file_mapping[(]([^)]*)[)]/\1->mapping/g' fs/erofs/*.c
perl -p -i -e 's/folio_index[(]([^)]*)[)]/\1->index/g' fs/erofs/*.c

Reported-by: Matthew Wilcox <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
Cc: Chao Yu <[email protected]>
Cc: Yue Hu <[email protected]>
Cc: Jeffle Xu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Gao Xiang <[email protected]>
---
change since v2:
- update the words s/internal/unnecessary/ in the subject line
pointed out by Matthew.

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

diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c
index 87ff35bff8d5..bc12030393b2 100644
--- a/fs/erofs/fscache.c
+++ b/fs/erofs/fscache.c
@@ -165,10 +165,10 @@ static int erofs_fscache_read_folios_async(struct fscache_cookie *cookie,
static int erofs_fscache_meta_read_folio(struct file *data, struct folio *folio)
{
int ret;
- struct erofs_fscache *ctx = folio_mapping(folio)->host->i_private;
+ struct erofs_fscache *ctx = folio->mapping->host->i_private;
struct erofs_fscache_request *req;

- req = erofs_fscache_req_alloc(folio_mapping(folio),
+ req = erofs_fscache_req_alloc(folio->mapping,
folio_pos(folio), folio_size(folio));
if (IS_ERR(req)) {
folio_unlock(folio);
@@ -276,7 +276,7 @@ static int erofs_fscache_read_folio(struct file *file, struct folio *folio)
struct erofs_fscache_request *req;
int ret;

- req = erofs_fscache_req_alloc(folio_mapping(folio),
+ req = erofs_fscache_req_alloc(folio->mapping,
folio_pos(folio), folio_size(folio));
if (IS_ERR(req)) {
folio_unlock(folio);
--
2.39.3


2024-01-15 14:51:06

by Gao Xiang

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] erofs: Don't use certain internal folio_*() functions

Hi Matthew,

On 2024/1/15 22:06, Matthew Wilcox wrote:
> On Mon, Jan 15, 2024 at 04:33:37PM +0800, Gao Xiang wrote:
>> From: David Howells <[email protected]>
>>
>> Filesystems should use folio->index and folio->mapping, instead of
>> folio_index(folio), folio_mapping() and folio_file_mapping() since
>> they know that it's in the pagecache.
>>
>> Change this automagically with:
>>
>> perl -p -i -e 's/folio_mapping[(]([^)]*)[)]/\1->mapping/g' fs/erofs/*.c
>> perl -p -i -e 's/folio_file_mapping[(]([^)]*)[)]/\1->mapping/g' fs/erofs/*.c
>> perl -p -i -e 's/folio_index[(]([^)]*)[)]/\1->index/g' fs/erofs/*.c
>>
>> Reported-by: Matthew Wilcox <[email protected]>
>> Signed-off-by: David Howells <[email protected]>
>> Reviewed-by: Jeff Layton <[email protected]>
>> Cc: Chao Yu <[email protected]>
>> Cc: Yue Hu <[email protected]>
>> Cc: Jeffle Xu <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Gao Xiang <[email protected]>
>> ---
>> Hi folks,
>>
>> I tend to apply this patch upstream since compressed data fscache
>> adaption touches this part too. If there is no objection, I'm
>> going to take this patch separately for -next shortly..
>
> Could you change the subject? It's not that the functions are
> "internal", it's that filesystems don't need to use them because they're
> guaranteed to not see swap pages. Maybe just s/internal/unnecessary/

Yes, the subject line was inherited from the original one.

Such helpers are useful if the type of a folio is unknown,
let me revise it.

Thanks,
Gao Xiang