Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-11 Thread Linus Torvalds
On Fri, Sep 11, 2020 at 5:20 AM Alexander Gordeev wrote: > > What if the entry is still pud_present, but got remapped after > READ_ONCE(*pudp)? IOW, it is still valid, but points elsewhere? That can't happen. The GUP walk doesn't hold any locks, but it *is* done with interrupts disabled, and

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-11 Thread Alexander Gordeev
On Thu, Sep 10, 2020 at 07:11:16PM -0300, Jason Gunthorpe wrote: > On Thu, Sep 10, 2020 at 02:22:37PM -0700, John Hubbard wrote: > > > Or am I way off here, and it really is possible (aside from the current > > s390 situation) to observe something that "is no longer a page table"? > > Yes, that

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-11 Thread Jason Gunthorpe
On Fri, Sep 11, 2020 at 09:09:39AM +0200, pet...@infradead.org wrote: > On Thu, Sep 10, 2020 at 06:59:21PM -0300, Jason Gunthorpe wrote: > > So, I suggest pXX_offset_unlocked() > > Urgh, no. Elsewhere in gup _unlocked() means it will take the lock > itself (get_user_pages_unlocked()) -- although

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-11 Thread peterz
On Thu, Sep 10, 2020 at 06:59:21PM -0300, Jason Gunthorpe wrote: > So, I suggest pXX_offset_unlocked() Urgh, no. Elsewhere in gup _unlocked() means it will take the lock itself (get_user_pages_unlocked()) -- although often it seems to mean the lock is already held (git grep _unlocked and marvel).

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Jason Gunthorpe
On Thu, Sep 10, 2020 at 07:57:49PM +0200, Gerald Schaefer wrote: > On Thu, 10 Sep 2020 10:02:33 -0300 > Jason Gunthorpe wrote: > > > On Thu, Sep 10, 2020 at 11:39:25AM +0200, Alexander Gordeev wrote: > > > > > As Gerald mentioned, it is very difficult to explain in a clear way. > > > Hopefully,

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread John Hubbard
On 9/10/20 3:11 PM, Jason Gunthorpe wrote: On Thu, Sep 10, 2020 at 02:22:37PM -0700, John Hubbard wrote: Or am I way off here, and it really is possible (aside from the current s390 situation) to observe something that "is no longer a page table"? Yes, that is the issue. Remember there is no

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Jason Gunthorpe
On Thu, Sep 10, 2020 at 02:22:37PM -0700, John Hubbard wrote: > Or am I way off here, and it really is possible (aside from the current > s390 situation) to observe something that "is no longer a page table"? Yes, that is the issue. Remember there is no locking for GUP fast. While a page table

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Jason Gunthorpe
On Thu, Sep 10, 2020 at 12:32:05PM -0700, Linus Torvalds wrote: > Yeah, I get hung up on naming sometimes. I don't tend to care much > about private local variables ("i" is a perfectly fine variable name), > but these kinds of somewhat subtle cross-architecture definitions I > feel matter. One of

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Jason Gunthorpe
On Thu, Sep 10, 2020 at 11:39:25AM +0200, Alexander Gordeev wrote: > As Gerald mentioned, it is very difficult to explain in a clear way. > Hopefully, one could make sense ot of it. I would say the page table API requires this invariant: pud = pud_offset(p4d, addr); do {

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread John Hubbard
On 9/10/20 11:13 AM, Jason Gunthorpe wrote: On Thu, Sep 10, 2020 at 10:35:38AM -0700, Linus Torvalds wrote: On Thu, Sep 10, 2020 at 2:40 AM Alexander Gordeev wrote: It is only gup_fast case that exposes the issue. It hits because pointers to stack copies are passed to gup_pXd_range

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Linus Torvalds
On Thu, Sep 10, 2020 at 12:11 PM Gerald Schaefer wrote: > > That sounds a lot like the pXd_offset_orig() from Martins first approach > in this thread: > https://lore.kernel.org/linuxppc-dev/20190418100218.0a4afd51@mschwideX1/ I have to admit to finding that name horrible, but aside from that,

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Gerald Schaefer
On Thu, 10 Sep 2020 11:33:17 -0700 Linus Torvalds wrote: > On Thu, Sep 10, 2020 at 11:13 AM Jason Gunthorpe wrote: > > > > So.. To change away from the stack option I think we'd have to pass > > the READ_ONCE value to pXX_offset() as an extra argument instead of it > > derefing the pointer

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Linus Torvalds
On Thu, Sep 10, 2020 at 11:13 AM Jason Gunthorpe wrote: > > So.. To change away from the stack option I think we'd have to pass > the READ_ONCE value to pXX_offset() as an extra argument instead of it > derefing the pointer internally. Yeah, but I think that would actually be the better model

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Jason Gunthorpe
On Thu, Sep 10, 2020 at 10:35:38AM -0700, Linus Torvalds wrote: > On Thu, Sep 10, 2020 at 2:40 AM Alexander Gordeev > wrote: > > > > It is only gup_fast case that exposes the issue. It hits because > > pointers to stack copies are passed to gup_pXd_range iterators, not > > pointers to real page

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Gerald Schaefer
On Thu, 10 Sep 2020 10:02:33 -0300 Jason Gunthorpe wrote: > On Thu, Sep 10, 2020 at 11:39:25AM +0200, Alexander Gordeev wrote: > > > As Gerald mentioned, it is very difficult to explain in a clear way. > > Hopefully, one could make sense ot of it. > > I would say the page table API requires

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Linus Torvalds
On Thu, Sep 10, 2020 at 2:40 AM Alexander Gordeev wrote: > > It is only gup_fast case that exposes the issue. It hits because > pointers to stack copies are passed to gup_pXd_range iterators, not > pointers to real page tables itself. Can we possibly change fast-gup to not do the stack copies?

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Jason Gunthorpe
On Thu, Sep 10, 2020 at 07:07:57PM +0200, Gerald Schaefer wrote: > I might have lost track a bit. Are we still talking about possible > functional impacts of either our current pagetable walking with s390 > (apart from gup_fast), or the proposed generic change (for s390, or > others?)? I'm

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Gerald Schaefer
On Thu, 10 Sep 2020 12:10:26 -0300 Jason Gunthorpe wrote: > On Thu, Sep 10, 2020 at 03:28:03PM +0200, Gerald Schaefer wrote: > > On Thu, 10 Sep 2020 10:02:33 -0300 > > Jason Gunthorpe wrote: > > > > > On Thu, Sep 10, 2020 at 11:39:25AM +0200, Alexander Gordeev wrote: > > > > > > > As

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Jason Gunthorpe
On Thu, Sep 10, 2020 at 03:28:03PM +0200, Gerald Schaefer wrote: > On Thu, 10 Sep 2020 10:02:33 -0300 > Jason Gunthorpe wrote: > > > On Thu, Sep 10, 2020 at 11:39:25AM +0200, Alexander Gordeev wrote: > > > > > As Gerald mentioned, it is very difficult to explain in a clear way. > > > Hopefully,

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Gerald Schaefer
On Thu, 10 Sep 2020 10:02:33 -0300 Jason Gunthorpe wrote: > On Thu, Sep 10, 2020 at 11:39:25AM +0200, Alexander Gordeev wrote: > > > As Gerald mentioned, it is very difficult to explain in a clear way. > > Hopefully, one could make sense ot of it. > > I would say the page table API requires

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Gerald Schaefer
On Wed, 9 Sep 2020 15:03:24 -0300 Jason Gunthorpe wrote: > On Wed, Sep 09, 2020 at 07:25:34PM +0200, Gerald Schaefer wrote: > > I actually had to draw myself a picture to get some hold of > > this, or rather a walk-through with a certain pud-crossing > > range in a folded 3-level scenario. Not

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-10 Thread Alexander Gordeev
On Wed, Sep 09, 2020 at 03:03:24PM -0300, Jason Gunthorpe wrote: > On Wed, Sep 09, 2020 at 07:25:34PM +0200, Gerald Schaefer wrote: > > I actually had to draw myself a picture to get some hold of > > this, or rather a walk-through with a certain pud-crossing > > range in a folded 3-level scenario.

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-09 Thread Jason Gunthorpe
On Wed, Sep 09, 2020 at 07:25:34PM +0200, Gerald Schaefer wrote: > I actually had to draw myself a picture to get some hold of > this, or rather a walk-through with a certain pud-crossing > range in a folded 3-level scenario. Not sure if I would have > understood my explanation above w/o that, but

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-09 Thread Gerald Schaefer
On Wed, 9 Sep 2020 09:18:46 -0700 Dave Hansen wrote: > On 9/9/20 5:29 AM, Gerald Schaefer wrote: > > This only works well as long there are real pagetable pointers involved, > > that can also be used for iteration. For gup_fast, or any other future > > pagetable walkers using the READ_ONCE logic

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-09 Thread Dave Hansen
On 9/9/20 5:29 AM, Gerald Schaefer wrote: > This only works well as long there are real pagetable pointers involved, > that can also be used for iteration. For gup_fast, or any other future > pagetable walkers using the READ_ONCE logic w/o lock, that is not true. > There are pointers involved to

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-09 Thread Gerald Schaefer
On Tue, 8 Sep 2020 07:30:50 -0700 Dave Hansen wrote: > On 9/7/20 11:00 AM, Gerald Schaefer wrote: > > Commit 1a42010cdc26 ("s390/mm: convert to the generic get_user_pages_fast > > code") introduced a subtle but severe bug on s390 with gup_fast, due to > > dynamic page table folding. > > Would

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-08 Thread Dave Hansen
On 9/7/20 11:00 AM, Gerald Schaefer wrote: > Commit 1a42010cdc26 ("s390/mm: convert to the generic get_user_pages_fast > code") introduced a subtle but severe bug on s390 with gup_fast, due to > dynamic page table folding. Would it be fair to say that the "fake" page table entries s390 allocates

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-08 Thread Gerald Schaefer
On Tue, 8 Sep 2020 14:40:10 +0200 Christophe Leroy wrote: > > > Le 08/09/2020 à 14:09, Christian Borntraeger a écrit : > > > > > > On 08.09.20 07:06, Christophe Leroy wrote: > >> > >> > >> Le 07/09/2020 à 20:00, Gerald Schaefer a écrit : > >>> From: Alexander Gordeev > >>> > >>> Commit

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-08 Thread Christian Borntraeger
On 08.09.20 07:06, Christophe Leroy wrote: > > > Le 07/09/2020 à 20:00, Gerald Schaefer a écrit : >> From: Alexander Gordeev >> >> Commit 1a42010cdc26 ("s390/mm: convert to the generic get_user_pages_fast >> code") introduced a subtle but severe bug on s390 with gup_fast, due to >> dynamic

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-08 Thread Gerald Schaefer
On Tue, 8 Sep 2020 07:30:50 -0700 Dave Hansen wrote: > On 9/7/20 11:00 AM, Gerald Schaefer wrote: > > Commit 1a42010cdc26 ("s390/mm: convert to the generic get_user_pages_fast > > code") introduced a subtle but severe bug on s390 with gup_fast, due to > > dynamic page table folding. > > Would

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-08 Thread Christophe Leroy
Le 08/09/2020 à 14:09, Christian Borntraeger a écrit : On 08.09.20 07:06, Christophe Leroy wrote: Le 07/09/2020 à 20:00, Gerald Schaefer a écrit : From: Alexander Gordeev Commit 1a42010cdc26 ("s390/mm: convert to the generic get_user_pages_fast code") introduced a subtle but severe

Re: [RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-07 Thread Christophe Leroy
Le 07/09/2020 à 20:00, Gerald Schaefer a écrit : From: Alexander Gordeev Commit 1a42010cdc26 ("s390/mm: convert to the generic get_user_pages_fast code") introduced a subtle but severe bug on s390 with gup_fast, due to dynamic page table folding. The question "What would it require for the

[RFC PATCH v2 1/3] mm/gup: fix gup_fast with dynamic page table folding

2020-09-07 Thread Gerald Schaefer
From: Alexander Gordeev Commit 1a42010cdc26 ("s390/mm: convert to the generic get_user_pages_fast code") introduced a subtle but severe bug on s390 with gup_fast, due to dynamic page table folding. The question "What would it require for the generic code to work for s390" has already been