Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-23 Thread Linus Torvalds
On Wed, Sep 23, 2020 at 10:16 AM Linus Torvalds wrote: > > But these two patches are very intentionally meant to be just "this > clearly changes NO semantics at all". The more I look at these, the more I go "this is a cleanup regardless", so I'll just keep thes in my tree as-is.

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-23 Thread Linus Torvalds
On Mon, Sep 21, 2020 at 2:18 PM Peter Xu wrote: > > There's one special path for copy_one_pte() with swap entries, in which > add_swap_count_continuation(GFP_ATOMIC) might fail. In that case we'll return > the swp_entry_t so that the caller will release the locks and redo the same > thing with

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-23 Thread Oleg Nesterov
On 09/22, Peter Xu wrote: > > On Tue, Sep 22, 2020 at 08:23:18PM +0200, Oleg Nesterov wrote: > > > > But I still think that !pte_none() -> pte_none() transition is not possible > > under mmap_write_lock()... > > > > OK, let me repeat I don't understans these code paths enough, let me reword: > > I

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-22 Thread Peter Xu
On Tue, Sep 22, 2020 at 08:23:18PM +0200, Oleg Nesterov wrote: > On 09/22, Peter Xu wrote: > > > > On Tue, Sep 22, 2020 at 06:53:55PM +0200, Oleg Nesterov wrote: > > > On 09/22, Peter Xu wrote: > > > > > > > > On Tue, Sep 22, 2020 at 05:48:46PM +0200, Oleg Nesterov wrote: > > > > > > However since

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-22 Thread Oleg Nesterov
On 09/22, Peter Xu wrote: > > On Tue, Sep 22, 2020 at 06:53:55PM +0200, Oleg Nesterov wrote: > > On 09/22, Peter Xu wrote: > > > > > > On Tue, Sep 22, 2020 at 05:48:46PM +0200, Oleg Nesterov wrote: > > > > > However since I didn't change this logic in this patch, it probably > > > > > means this

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-22 Thread Peter Xu
On Tue, Sep 22, 2020 at 06:53:55PM +0200, Oleg Nesterov wrote: > On 09/22, Peter Xu wrote: > > > > On Tue, Sep 22, 2020 at 05:48:46PM +0200, Oleg Nesterov wrote: > > > > However since I didn't change this logic in this patch, it probably > > > > means this > > > > bug is also in the original code

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-22 Thread Oleg Nesterov
On 09/22, Peter Xu wrote: > > On Tue, Sep 22, 2020 at 05:48:46PM +0200, Oleg Nesterov wrote: > > > However since I didn't change this logic in this patch, it probably means > > > this > > > bug is also in the original code before this series... I'm thinking > > > maybe I > > > should prepare a

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-22 Thread Peter Xu
On Tue, Sep 22, 2020 at 05:48:46PM +0200, Oleg Nesterov wrote: > > However since I didn't change this logic in this patch, it probably means > > this > > bug is also in the original code before this series... I'm thinking maybe I > > should prepare a standalone patch to clear the swp_entry_t and

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-22 Thread Oleg Nesterov
On 09/22, Peter Xu wrote: > > On Tue, Sep 22, 2020 at 12:18:16PM +0200, Oleg Nesterov wrote: > > On 09/22, Oleg Nesterov wrote: > > > > > > On 09/21, Peter Xu wrote: > > > > > > > > @@ -866,13 +877,18 @@ static int copy_pte_range(struct mm_struct > > > > *dst_mm, struct mm_struct *src_mm, > > > >

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-22 Thread Peter Xu
On Tue, Sep 22, 2020 at 12:18:16PM +0200, Oleg Nesterov wrote: > On 09/22, Oleg Nesterov wrote: > > > > On 09/21, Peter Xu wrote: > > > > > > @@ -866,13 +877,18 @@ static int copy_pte_range(struct mm_struct *dst_mm, > > > struct mm_struct *src_mm, > > > pte_unmap_unlock(orig_dst_pte, dst_ptl);

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-22 Thread Peter Xu
On Tue, Sep 22, 2020 at 12:11:29AM -0700, John Hubbard wrote: > On 9/21/20 2:17 PM, Peter Xu wrote: > > There's one special path for copy_one_pte() with swap entries, in which > > add_swap_count_continuation(GFP_ATOMIC) might fail. In that case we'll > > return > > I might be looking at the

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-22 Thread Oleg Nesterov
On 09/22, Oleg Nesterov wrote: > > On 09/21, Peter Xu wrote: > > > > @@ -866,13 +877,18 @@ static int copy_pte_range(struct mm_struct *dst_mm, > > struct mm_struct *src_mm, > > pte_unmap_unlock(orig_dst_pte, dst_ptl); > > cond_resched(); > > > > - if (entry.val) { > > - if

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-22 Thread Oleg Nesterov
On 09/21, Peter Xu wrote: > > @@ -866,13 +877,18 @@ static int copy_pte_range(struct mm_struct *dst_mm, > struct mm_struct *src_mm, > pte_unmap_unlock(orig_dst_pte, dst_ptl); > cond_resched(); > > - if (entry.val) { > - if (add_swap_count_continuation(entry,

Re: [PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-22 Thread John Hubbard
On 9/21/20 2:17 PM, Peter Xu wrote: There's one special path for copy_one_pte() with swap entries, in which add_swap_count_continuation(GFP_ATOMIC) might fail. In that case we'll return I might be looking at the wrong place, but the existing code seems to call

[PATCH 3/5] mm: Rework return value for copy_one_pte()

2020-09-21 Thread Peter Xu
There's one special path for copy_one_pte() with swap entries, in which add_swap_count_continuation(GFP_ATOMIC) might fail. In that case we'll return the swp_entry_t so that the caller will release the locks and redo the same thing with GFP_KERNEL. It's confusing when copy_one_pte() must return