lkml.org 
[lkml]   [2020]   [May]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH V2 0/9] Introduce attach/clear_page_private to cleanup code
From
Date
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

\
 
 \ /
  Last update: 2020-05-02 10:58    [W:0.058 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site