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]>
---
fs/btrfs/disk-io.c | 4 +---
fs/btrfs/extent_io.c | 21 ++++++---------------
fs/btrfs/inode.c | 17 ++++-------------
3 files changed, 11 insertions(+), 31 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a6cb5cbbdb9f..1230863e80f9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -980,9 +980,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);
+ clear_fs_page_private(page);
}
}
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 39e45b8a5031..de8c2d5a99db 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3076,22 +3076,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))
+ set_fs_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))
+ set_fs_page_private(page, (void *)EXTENT_PAGE_PRIVATE);
}
static struct extent_map *
@@ -4929,10 +4923,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);
+ clear_fs_page_private(page);
}
if (mapped)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 320d1062068d..07871c57ba96 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8303,11 +8303,8 @@ btrfs_readpages(struct file *file, struct address_space *mapping,
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)
+ clear_fs_page_private(page);
return ret;
}
@@ -8331,11 +8328,9 @@ static int btrfs_migratepage(struct address_space *mapping,
if (page_has_private(page)) {
ClearPagePrivate(page);
- get_page(newpage);
- set_page_private(newpage, page_private(page));
+ set_fs_page_private(newpage, (void *)page_private(page));
set_page_private(page, 0);
put_page(page);
- SetPagePrivate(newpage);
}
if (PagePrivate2(page)) {
@@ -8458,11 +8453,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);
- }
+ clear_fs_page_private(page);
}
/*
--
2.17.1
On Sun, Apr 26, 2020 at 11:49:19PM +0200, Guoqing Jiang wrote:
> 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]>
....
> 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))
> + set_fs_page_private(page, (void *)EXTENT_PAGE_PRIVATE);
Change the definition of EXTENT_PAGE_PRIVATE so the cast is not
needed? Nothing ever reads EXTENT_PAGE_PRIVATE; it's only there to
set the private flag for other code to check and release the extent
mapping reference to the page...
> @@ -8331,11 +8328,9 @@ static int btrfs_migratepage(struct address_space *mapping,
>
> if (page_has_private(page)) {
> ClearPagePrivate(page);
> - get_page(newpage);
> - set_page_private(newpage, page_private(page));
> + set_fs_page_private(newpage, (void *)page_private(page));
> set_page_private(page, 0);
> put_page(page);
> - SetPagePrivate(newpage);
> }
This is just:
set_fs_page_private(newpage, clear_fs_page_private(page));
Cheers,
Dave.
--
Dave Chinner
[email protected]
On Mon, Apr 27, 2020 at 08:20:54AM +1000, Dave Chinner wrote:
> > 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))
> > + set_fs_page_private(page, (void *)EXTENT_PAGE_PRIVATE);
>
> Change the definition of EXTENT_PAGE_PRIVATE so the cast is not
> needed? Nothing ever reads EXTENT_PAGE_PRIVATE; it's only there to
> set the private flag for other code to check and release the extent
> mapping reference to the page...
IIRC there as a patch on the btrfs list to remove EXTENT_PAGE_PRIVATE,
it might be better to not bother changing it. Maybe the btrfs
maintainers remember this better.
On 4/27/20 12:20 AM, Dave Chinner wrote:
> On Sun, Apr 26, 2020 at 11:49:19PM +0200, Guoqing Jiang wrote:
>> 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]>
> ....
>
>> 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))
>> + set_fs_page_private(page, (void *)EXTENT_PAGE_PRIVATE);
> Change the definition of EXTENT_PAGE_PRIVATE so the cast is not
> needed? Nothing ever reads EXTENT_PAGE_PRIVATE; it's only there to
> set the private flag for other code to check and release the extent
> mapping reference to the page...
Not know the code well, so I just make the cast ...
>> @@ -8331,11 +8328,9 @@ static int btrfs_migratepage(struct address_space *mapping,
>>
>> if (page_has_private(page)) {
>> ClearPagePrivate(page);
>> - get_page(newpage);
>> - set_page_private(newpage, page_private(page));
>> + set_fs_page_private(newpage, (void *)page_private(page));
>> set_page_private(page, 0);
>> put_page(page);
>> - SetPagePrivate(newpage);
>> }
> This is just:
> set_fs_page_private(newpage, clear_fs_page_private(page));
>
Thanks a lot! It is more better.
Thanks,
Guoqing
On Sun, Apr 26, 2020 at 10:54:28PM -0700, Christoph Hellwig wrote:
> On Mon, Apr 27, 2020 at 08:20:54AM +1000, Dave Chinner wrote:
> > > 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))
> > > + set_fs_page_private(page, (void *)EXTENT_PAGE_PRIVATE);
> >
> > Change the definition of EXTENT_PAGE_PRIVATE so the cast is not
> > needed? Nothing ever reads EXTENT_PAGE_PRIVATE; it's only there to
> > set the private flag for other code to check and release the extent
> > mapping reference to the page...
>
> IIRC there as a patch on the btrfs list to remove EXTENT_PAGE_PRIVATE,
> it might be better to not bother changing it. Maybe the btrfs
> maintainers remember this better.
The patch removing it is part of patchset adding full iomap support to
btrfs,
(https://lore.kernel.org/linux-btrfs/[email protected]/)
but it'll still take some time so I'm OK with using the
set_fs_page_private helper and adding the cast to EXTENT_PAGE_PRIVATE
definition.