Messages in this thread | | | Subject | Re: [RFC PATCH V2 0/9] Introduce attach/clear_page_private to cleanup code | From | Guoqing Jiang <> | Date | Sat, 2 May 2020 10:56:51 +0200 |
| |
On 5/2/20 2:41 AM, Matthew Wilcox wrote: > On Sat, May 02, 2020 at 12:42:15AM +0200, Guoqing Jiang wrote: >> On 5/2/20 12:16 AM, Matthew Wilcox wrote: >>> On Thu, Apr 30, 2020 at 11:44:41PM +0200, Guoqing Jiang wrote: >>>> include/linux/pagemap.h: introduce attach/clear_page_private >>>> md: remove __clear_page_buffers and use attach/clear_page_private >>>> btrfs: use attach/clear_page_private >>>> fs/buffer.c: use attach/clear_page_private >>>> f2fs: use attach/clear_page_private >>>> iomap: use attach/clear_page_private >>>> ntfs: replace attach_page_buffers with attach_page_private >>>> orangefs: use attach/clear_page_private >>>> buffer_head.h: remove attach_page_buffers >>> I think mm/migrate.c could also use this: >>> >>> ClearPagePrivate(page); >>> set_page_private(newpage, page_private(page)); >>> set_page_private(page, 0); >>> put_page(page); >>> get_page(newpage); >>> >> Thanks for checking! Assume the below change is appropriate. >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index 7160c1556f79..f214adfb3fa4 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, detach_page_private(page)); >> get_page(newpage); > I think you can do: > > @@ -797,11 +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); > - get_page(newpage); > + attach_page_private(newpage, detach_page_private(page)); > > bh = head; > do { > @@ -810,8 +806,6 @@ static int __buffer_migrate_page(struct address_space *mapping, > > } while (bh != head); > > - SetPagePrivate(newpage); > - > if (mode != MIGRATE_SYNC_NO_COPY) > > ... but maybe there's a subtlety to the ordering of the setup of the bh > and setting PagePrivate that means what you have there is a better patch.
Yes, it is better but not sure if the order can be changed here. And seems the original commit is this one.
commit e965f9630c651fa4249039fd4b80c9392d07a856 Author: Christoph Lameter <clameter@sgi.com> Date: Wed Feb 1 03:05:41 2006 -0800
[PATCH] Direct Migration V9: Avoid writeback / page_migrate() method
Migrate a page with buffers without requiring writeback
This introduces a new address space operation migratepage() that may be used by a filesystem to implement its own version of page migration.
A version is provided that migrates buffers attached to pages. Some filesystems (ext2, ext3, xfs) are modified to utilize this feature.
The swapper address space operation are modified so that a regular migrate_page() will occur for anonymous pages without writeback (migrate_pages forces every anonymous page to have a swap entry).
Hope mm experts could take a look, so CC more people and mm list. And the question is that if we can setting PagePrivate before setup bh in the __buffer_migrate_page, thanks for your any further input.
Thanks, Guoqing
| |