2020-05-17 21:51:24

by Guoqing Jiang

[permalink] [raw]
Subject: [PATCH 00/10] Introduce attach/detach_page_private to cleanup code

Hell Andrew and Al,

Since no more feedback from RFC V3, I suppose the series could be ready
for merge. And Song has already acked the md patch, then all the rest
patches are for fs and mm.

So, if no further comments for the patchset, could you consider to queue
them from either of your tree to avoid potential build issue? Thank you!

RFC V3: https://lore.kernel.org/lkml/[email protected]/
RFC V3: https://lore.kernel.org/lkml/[email protected]/
RFC: https://lore.kernel.org/lkml/[email protected]/

Thanks,
Guoqing

No change since RFC V3.

RFC V2 -> RFC V3:
1. rename clear_page_private to detach_page_private.
2. Update the comments for attach/detach_page_private from Mattew.
3. add one patch to call new function in mm/migrate.c as suggested by Mattew, but
use the conservative way to keep the orginal semantics [2].


RFC -> RFC V2:
1. rename the new functions and add comments for them.
2. change the return type of attach_page_private.
3. call attach_page_private(newpage, clear_page_private(page)) to cleanup code further.
4. avoid potential use-after-free in orangefs.

[1]. https://lore.kernel.org/linux-fsdevel/[email protected]/
[2]. https://lore.kernel.org/lkml/[email protected]/

Guoqing Jiang (10):
include/linux/pagemap.h: introduce attach/detach_page_private
md: remove __clear_page_buffers and use attach/detach_page_private
btrfs: use attach/detach_page_private
fs/buffer.c: use attach/detach_page_private
f2fs: use attach/detach_page_private
iomap: use attach/detach_page_private
ntfs: replace attach_page_buffers with attach_page_private
orangefs: use attach/detach_page_private
buffer_head.h: remove attach_page_buffers
mm/migrate.c: call detach_page_private to cleanup code

drivers/md/md-bitmap.c | 12 ++----------
fs/btrfs/disk-io.c | 4 +---
fs/btrfs/extent_io.c | 21 ++++++---------------
fs/btrfs/inode.c | 23 +++++------------------
fs/buffer.c | 16 ++++------------
fs/f2fs/f2fs.h | 11 ++---------
fs/iomap/buffered-io.c | 19 ++++---------------
fs/ntfs/aops.c | 2 +-
fs/ntfs/mft.c | 2 +-
fs/orangefs/inode.c | 32 ++++++--------------------------
include/linux/buffer_head.h | 8 --------
include/linux/pagemap.h | 37 +++++++++++++++++++++++++++++++++++++
mm/migrate.c | 5 +----
13 files changed, 70 insertions(+), 122 deletions(-)

--
2.17.1


2020-05-17 21:52:11

by Guoqing Jiang

[permalink] [raw]
Subject: [PATCH 07/10] ntfs: replace attach_page_buffers with attach_page_private

Call the new function since attach_page_buffers will be removed.

Cc: Anton Altaparmakov <[email protected]>
Cc: [email protected]
Signed-off-by: Guoqing Jiang <[email protected]>
---
No change since RFC V2.

RFC -> RFC V2
1. change the name of new function to attach_page_private.

fs/ntfs/aops.c | 2 +-
fs/ntfs/mft.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs/aops.c b/fs/ntfs/aops.c
index 554b744f41bf..bb0a43860ad2 100644
--- a/fs/ntfs/aops.c
+++ b/fs/ntfs/aops.c
@@ -1732,7 +1732,7 @@ void mark_ntfs_record_dirty(struct page *page, const unsigned int ofs) {
bh = bh->b_this_page;
} while (bh);
tail->b_this_page = head;
- attach_page_buffers(page, head);
+ attach_page_private(page, head);
} else
buffers_to_free = bh;
}
diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 3aac5c917afe..fbb9f1bc623d 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -504,7 +504,7 @@ int ntfs_sync_mft_mirror(ntfs_volume *vol, const unsigned long mft_no,
bh = bh->b_this_page;
} while (bh);
tail->b_this_page = head;
- attach_page_buffers(page, head);
+ attach_page_private(page, head);
}
bh = head = page_buffers(page);
BUG_ON(!bh);
--
2.17.1

2020-05-17 22:09:45

by Guoqing Jiang

[permalink] [raw]
Subject: [PATCH 03/10] btrfs: use attach/detach_page_private

Since the new pair function is introduced, we can call them to clean the
code in btrfs.

Cc: Chris Mason <[email protected]>
Cc: Josef Bacik <[email protected]>
Cc: David Sterba <[email protected]>
Cc: [email protected]
Signed-off-by: Guoqing Jiang <[email protected]>
---
No change since RFC V3.

RFC V2 -> RFC V3
1. rename clear_page_private to detach_page_private.

RFC -> RFC V2
1. change the name of new functions to attach/clear_page_private.
2. call attach_page_private(newpage, clear_page_private(page)) to
cleanup code further as suggested by Dave Chinner.

fs/btrfs/disk-io.c | 4 +---
fs/btrfs/extent_io.c | 21 ++++++---------------
fs/btrfs/inode.c | 23 +++++------------------
3 files changed, 12 insertions(+), 36 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 714b57553ed6..44745d5e05cf 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -976,9 +976,7 @@ static void btree_invalidatepage(struct page *page, unsigned int offset,
btrfs_warn(BTRFS_I(page->mapping->host)->root->fs_info,
"page private not zero on page %llu",
(unsigned long long)page_offset(page));
- ClearPagePrivate(page);
- set_page_private(page, 0);
- put_page(page);
+ detach_page_private(page);
}
}

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 451d17bfafd8..704239546093 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3099,22 +3099,16 @@ static int submit_extent_page(unsigned int opf,
static void attach_extent_buffer_page(struct extent_buffer *eb,
struct page *page)
{
- if (!PagePrivate(page)) {
- SetPagePrivate(page);
- get_page(page);
- set_page_private(page, (unsigned long)eb);
- } else {
+ if (!PagePrivate(page))
+ attach_page_private(page, eb);
+ else
WARN_ON(page->private != (unsigned long)eb);
- }
}

void set_page_extent_mapped(struct page *page)
{
- if (!PagePrivate(page)) {
- SetPagePrivate(page);
- get_page(page);
- set_page_private(page, EXTENT_PAGE_PRIVATE);
- }
+ if (!PagePrivate(page))
+ attach_page_private(page, (void *)EXTENT_PAGE_PRIVATE);
}

static struct extent_map *
@@ -4935,10 +4929,7 @@ static void btrfs_release_extent_buffer_pages(struct extent_buffer *eb)
* We need to make sure we haven't be attached
* to a new eb.
*/
- ClearPagePrivate(page);
- set_page_private(page, 0);
- /* One for the page private */
- put_page(page);
+ detach_page_private(page);
}

if (mapped)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 037efc25d993..6c2d6ecedd6a 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -7927,11 +7927,8 @@ static void btrfs_readahead(struct readahead_control *rac)
static int __btrfs_releasepage(struct page *page, gfp_t gfp_flags)
{
int ret = try_release_extent_mapping(page, gfp_flags);
- if (ret == 1) {
- ClearPagePrivate(page);
- set_page_private(page, 0);
- put_page(page);
- }
+ if (ret == 1)
+ detach_page_private(page);
return ret;
}

@@ -7953,14 +7950,8 @@ static int btrfs_migratepage(struct address_space *mapping,
if (ret != MIGRATEPAGE_SUCCESS)
return ret;

- if (page_has_private(page)) {
- ClearPagePrivate(page);
- get_page(newpage);
- set_page_private(newpage, page_private(page));
- set_page_private(page, 0);
- put_page(page);
- SetPagePrivate(newpage);
- }
+ if (page_has_private(page))
+ attach_page_private(newpage, detach_page_private(page));

if (PagePrivate2(page)) {
ClearPagePrivate2(page);
@@ -8082,11 +8073,7 @@ static void btrfs_invalidatepage(struct page *page, unsigned int offset,
}

ClearPageChecked(page);
- if (PagePrivate(page)) {
- ClearPagePrivate(page);
- set_page_private(page, 0);
- put_page(page);
- }
+ detach_page_private(page);
}

/*
--
2.17.1

2020-05-19 21:43:22

by Guoqing Jiang

[permalink] [raw]
Subject: [UPDATE PATCH 10/10] mm/migrate.c: call detach_page_private to cleanup code

We can cleanup code a little by call detach_page_private here.

Signed-off-by: Guoqing Jiang <[email protected]>
---
Add the cast to fix type mismatch warning, sorry for the mistake.

mm/migrate.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 7160c1556f79..44546d407e40 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -797,10 +797,7 @@ static int __buffer_migrate_page(struct address_space *mapping,
if (rc != MIGRATEPAGE_SUCCESS)
goto unlock_buffers;

- ClearPagePrivate(page);
- set_page_private(newpage, page_private(page));
- set_page_private(page, 0);
- put_page(page);
+ set_page_private(newpage, (unsigned long)detach_page_private(page));
get_page(newpage);

bh = head;
--
2.17.1