linux-kernel - Re: [PATCH 1/2] mm: introduce put_user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Dec 2018 11:28:46 +0100
From:   Jan Kara <jack@...e.cz>
To:     Jerome Glisse <jglisse@...hat.com>
Cc:     John Hubbard <jhubbard@...dia.com>,
        Matthew Wilcox <willy@...radead.org>,
        Dan Williams <dan.j.williams@...el.com>,
        John Hubbard <john.hubbard@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux MM <linux-mm@...ck.org>, Jan Kara <jack@...e.cz>,
        tom@...pey.com, Al Viro <viro@...iv.linux.org.uk>, benve@...co.com,
        Christoph Hellwig <hch@...radead.org>,
        Christopher Lameter <cl@...ux.com>,
        "Dalessandro, Dennis" <dennis.dalessandro@...el.com>,
        Doug Ledford <dledford@...hat.com>,
        Jason Gunthorpe <jgg@...pe.ca>,
        Michal Hocko <mhocko@...nel.org>, mike.marciniszyn@...el.com,
        rcampbell@...dia.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH 1/2] mm: introduce put_user_page*(), placeholder versions

On Fri 07-12-18 21:24:46, Jerome Glisse wrote:
> Another crazy idea, why not treating GUP as another mapping of the page
> and caller of GUP would have to provide either a fake anon_vma struct or
> a fake vma struct (or both for PRIVATE mapping of a file where you can
> have a mix of both private and file page thus only if it is a read only
> GUP) that would get added to the list of existing mapping.
>
> So the flow would be:
>     somefunction_thatuse_gup()
>     {
>         ...
>         GUP(_fast)(vma, ..., fake_anon, fake_vma);
>         ...
>     }
> 
>     GUP(vma, ..., fake_anon, fake_vma)
>     {
>         if (vma->flags == ANON) {
>             // Add the fake anon vma to the anon vma chain as a child
>             // of current vma
>         } else {
>             // Add the fake vma to the mapping tree
>         }
> 
>         // The existing GUP except that now it inc mapcount and not
>         // refcount
>         GUP_old(..., &nanonymous, &nfiles);
> 
>         atomic_add(&fake_anon->refcount, nanonymous);
>         atomic_add(&fake_vma->refcount, nfiles);
> 
>         return nanonymous + nfiles;
>     }

Thanks for your idea! This is actually something like I was suggesting back
at LSF/MM in Deer Valley. There were two downsides to this I remember
people pointing out:

1) This cannot really work with __get_user_pages_fast(). You're not allowed
to get necessary locks to insert new entry into the VMA tree in that
context. So essentially we'd loose get_user_pages_fast() functionality.

2) The overhead e.g. for direct IO may be noticeable. You need to allocate
the fake tracking VMA, get VMA interval tree lock, insert into the tree.
Then on IO completion you need to queue work to unpin the pages again as you
cannot remove the fake VMA directly from interrupt context where the IO is
completed.

You are right that the cost could be amortized if gup() is called for
multiple consecutive pages however for small IOs there's no help...

So this approach doesn't look like a win to me over using counter in struct
page and I'd rather try looking into squeezing HMM public page usage of
struct page so that we can fit that gup counter there as well. I know that
it may be easier said than done...

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR