Messages in this thread | | | From | Linus Torvalds <> | Date | Thu, 2 Dec 2021 10:54:48 -0800 | Subject | Re: [5.4 PATCH] mm/gup: Do not force a COW break on file-backed memory |
| |
On Wed, Dec 1, 2021 at 8:11 PM Matthew Wilcox <willy@infradead.org> wrote: > > The other patch we've been kicking around (and works) is: > > static inline bool should_force_cow_break(struct vm_area_struct *vma, unsigned > int flags) > { > - return is_cow_mapping(vma->vm_flags) && (flags & FOLL_GET); > + return is_cow_mapping(vma->vm_flags) && > + (!(vma->vm_flags & VM_DENYWRITE)) && (flags & FOLL_GET); > }
That patch makes no sense to me.
It may "work", but it doesn't actually do anything sensible or really fix the problem that I can tell.
I suspect a real fix would be bigger and more invasive.
If the answer is not to backport all the other changes (and they were _really_ invasive), I think one answer may be to simply move the "should_force_cow_break()" down to below the point where you've looked up the page.
Then you can actually look at "is this a file mapped page", and say "if so, that's ok, we can return it as-is".
Otherwise, you do something like
foll_flags |= FOLL_WRITE; free_page(page); goto repeat;
to repeat the loop (now with FOLL_WRITE).
So the patch is bigger and more involved, because you would have done the page lookup (for reading) and now notice "Oh, I need it for writing instead" so you need to undo and re-do).
But at least - unlike backporting everything else - it would be limited to that one __get_user_pages() function.
Hmm?
(And you'd need to handle that follow_hugetlb_page() case too), not just the follow_page_mask() one)
Linus
| |