lkml.org 
[lkml]   [2019]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 1/1] mm: introduce put_user_page*(), placeholder versions
    On Tue, Mar 19, 2019 at 10:14:16AM -0400, Jerome Glisse wrote:
    > On Tue, Mar 19, 2019 at 09:47:24AM -0400, Jerome Glisse wrote:
    > > On Tue, Mar 19, 2019 at 03:04:17PM +0300, Kirill A. Shutemov wrote:
    > > > On Fri, Mar 08, 2019 at 01:36:33PM -0800, john.hubbard@gmail.com wrote:
    > > > > From: John Hubbard <jhubbard@nvidia.com>
    > >
    > > [...]
    > >
    > > > > diff --git a/mm/gup.c b/mm/gup.c
    > > > > index f84e22685aaa..37085b8163b1 100644
    > > > > --- a/mm/gup.c
    > > > > +++ b/mm/gup.c
    > > > > @@ -28,6 +28,88 @@ struct follow_page_context {
    > > > > unsigned int page_mask;
    > > > > };
    > > > >
    > > > > +typedef int (*set_dirty_func_t)(struct page *page);
    > > > > +
    > > > > +static void __put_user_pages_dirty(struct page **pages,
    > > > > + unsigned long npages,
    > > > > + set_dirty_func_t sdf)
    > > > > +{
    > > > > + unsigned long index;
    > > > > +
    > > > > + for (index = 0; index < npages; index++) {
    > > > > + struct page *page = compound_head(pages[index]);
    > > > > +
    > > > > + if (!PageDirty(page))
    > > > > + sdf(page);
    > > >
    > > > How is this safe? What prevents the page to be cleared under you?
    > > >
    > > > If it's safe to race clear_page_dirty*() it has to be stated explicitly
    > > > with a reason why. It's not very clear to me as it is.
    > >
    > > The PageDirty() optimization above is fine to race with clear the
    > > page flag as it means it is racing after a page_mkclean() and the
    > > GUP user is done with the page so page is about to be write back
    > > ie if (!PageDirty(page)) see the page as dirty and skip the sdf()
    > > call while a split second after TestClearPageDirty() happens then
    > > it means the racing clear is about to write back the page so all
    > > is fine (the page was dirty and it is being clear for write back).
    > >
    > > If it does call the sdf() while racing with write back then we
    > > just redirtied the page just like clear_page_dirty_for_io() would
    > > do if page_mkclean() failed so nothing harmful will come of that
    > > neither. Page stays dirty despite write back it just means that
    > > the page might be write back twice in a row.
    >
    > Forgot to mention one thing, we had a discussion with Andrea and Jan
    > about set_page_dirty() and Andrea had the good idea of maybe doing
    > the set_page_dirty() at GUP time (when GUP with write) not when the
    > GUP user calls put_page(). We can do that by setting the dirty bit
    > in the pte for instance. They are few bonus of doing things that way:
    > - amortize the cost of calling set_page_dirty() (ie one call for
    > GUP and page_mkclean()
    > - it is always safe to do so at GUP time (ie the pte has write
    > permission and thus the page is in correct state)
    > - safe from truncate race
    > - no need to ever lock the page

    I seem to have missed this conversation, so please excuse me for
    asking a stupid question: if it's a file backed page, what prevents
    background writeback from cleaning the dirty page ~30s into a long
    term pin? i.e. I don't see anything in this proposal that prevents
    the page from being cleaned by writeback and putting us straight
    back into the situation where a long term RDMA is writing to a clean
    page....

    Cheers,

    Dave.
    --
    Dave Chinner
    david@fromorbit.com

    \
     
     \ /
      Last update: 2019-03-19 22:24    [W:3.097 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site