Re: [PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

On Sun, Feb 11, 2018 at 05:29:37PM +0300, Konstantin Khlebnikov wrote: > + /* > + * Finally unfreeze refcount. Additional pin to radix tree. > + */ > + page_ref_unfreeze(page_tail, 1 + (!PageAnon(head) || > + PageSwapCache(head))); Please

Re: [PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

On Sun, Feb 11, 2018 at 05:29:37PM +0300, Konstantin Khlebnikov wrote: > + /* > + * Finally unfreeze refcount. Additional pin to radix tree. > + */ > + page_ref_unfreeze(page_tail, 1 + (!PageAnon(head) || > + PageSwapCache(head))); Please

Re: [PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

On Sun, Feb 11, 2018 at 6:47 PM, Kirill A. Shutemov wrote: > On Sun, Feb 11, 2018 at 06:32:10PM +0300, Konstantin Khlebnikov wrote: >> On Sun, Feb 11, 2018 at 6:14 PM, Kirill A. Shutemov >> wrote: >> > On Sun, Feb 11, 2018 at 05:29:37PM +0300,

Re: [PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

On Sun, Feb 11, 2018 at 6:47 PM, Kirill A. Shutemov wrote: > On Sun, Feb 11, 2018 at 06:32:10PM +0300, Konstantin Khlebnikov wrote: >> On Sun, Feb 11, 2018 at 6:14 PM, Kirill A. Shutemov >> wrote: >> > On Sun, Feb 11, 2018 at 05:29:37PM +0300, Konstantin Khlebnikov wrote: >> >> And replace

Re: [PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

On Sun, Feb 11, 2018 at 06:32:10PM +0300, Konstantin Khlebnikov wrote: > On Sun, Feb 11, 2018 at 6:14 PM, Kirill A. Shutemov > wrote: > > On Sun, Feb 11, 2018 at 05:29:37PM +0300, Konstantin Khlebnikov wrote: > >> And replace page_ref_inc()/page_ref_add() with

Re: [PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

On Sun, Feb 11, 2018 at 06:32:10PM +0300, Konstantin Khlebnikov wrote: > On Sun, Feb 11, 2018 at 6:14 PM, Kirill A. Shutemov > wrote: > > On Sun, Feb 11, 2018 at 05:29:37PM +0300, Konstantin Khlebnikov wrote: > >> And replace page_ref_inc()/page_ref_add() with page_ref_unfreeze() which > >> is

Re: [PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

On Sun, Feb 11, 2018 at 6:14 PM, Kirill A. Shutemov wrote: > On Sun, Feb 11, 2018 at 05:29:37PM +0300, Konstantin Khlebnikov wrote: >> And replace page_ref_inc()/page_ref_add() with page_ref_unfreeze() which >> is made especially for that and has semantic of

Re: [PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

On Sun, Feb 11, 2018 at 6:14 PM, Kirill A. Shutemov wrote: > On Sun, Feb 11, 2018 at 05:29:37PM +0300, Konstantin Khlebnikov wrote: >> And replace page_ref_inc()/page_ref_add() with page_ref_unfreeze() which >> is made especially for that and has semantic of smp_store_release(). > > Nak on this

Re: [PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

On Sun, Feb 11, 2018 at 05:29:37PM +0300, Konstantin Khlebnikov wrote: > And replace page_ref_inc()/page_ref_add() with page_ref_unfreeze() which > is made especially for that and has semantic of smp_store_release(). Nak on this part. page_ref_unfreeze() uses atomic_set() which neglects the

Re: [PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

On Sun, Feb 11, 2018 at 05:29:37PM +0300, Konstantin Khlebnikov wrote: > And replace page_ref_inc()/page_ref_add() with page_ref_unfreeze() which > is made especially for that and has semantic of smp_store_release(). Nak on this part. page_ref_unfreeze() uses atomic_set() which neglects the

[PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

THP split makes non-atomic change of tail page flags. This is almost ok because tail pages are locked and isolated but this breaks recent changes in page locking: non-atomic operation could clear bit PG_waiters. As a result concurrent sequence get_page_unless_zero() -> lock_page() might block

[PATCH v2] mm/huge_memory.c: reorder operations in __split_huge_page_tail()

THP split makes non-atomic change of tail page flags. This is almost ok because tail pages are locked and isolated but this breaks recent changes in page locking: non-atomic operation could clear bit PG_waiters. As a result concurrent sequence get_page_unless_zero() -> lock_page() might block