Date:   Thu, 03 Jun 2021 13:01:05 +0000
From:   Simon Ser <contact@...rsion.fr>
To:     Ming Lin <minggr@...il.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Hugh Dickins <hughd@...gle.com>, Peter Xu <peterx@...hat.com>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        Matthew Wilcox <willy@...radead.org>,
        Dan Williams <dan.j.williams@...el.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Will Deacon <will@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        David Herrmann <dh.herrmann@...il.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Greg Kroah-Hartman <greg@...ah.com>,
        "tytso@....edu" <tytso@....edu>
Subject: Re: Sealed memfd & no-fault mmap

On Tuesday, June 1st, 2021 at 9:08 AM, Ming Lin <minggr@...il.com> wrote:

> On 5/31/2021 11:24 PM, Linus Torvalds wrote:
> > On Mon, May 31, 2021 at 11:13 AM Ming Lin <minggr@...il.com> wrote:
> >>
> >> OK, I borrowed code from filemap_xip.c and implemented this behavior.
> >
> > I think that "unmap page" is too complicated and fragile.
> >
> > The only page that could possibly validly be unmapped is a stale zero
> > page, but that code in shmem_unmap_nofault_page() seems to try to
> > handle other cases too (ie that whole page_remove_rmap() - afaik a
> > zero page has no rmap).
> >
> > I get the feeling that the simpler thing to do is to just say "if you
> > use MAP_NOSIGBUS, and you access pages that don't have a backing
> > store, you will get zero pages, and they will NOT BE SYNCHRONIZED with
> > the backing store possibly later being updated".
> >
> > IOW, just document the fact that a MAP_NOSIGBUS mapping isn't coherent
> > wrt shmem contents that are expanded and filled in later.
> >
> > Don't try to "fix" it - because any user that uses MAP_NOSIGBUS had
> > better just accept that it's not compatible with expanding the shmem
> > backing store later.
> >
> > Keep it simple and stupid. Hmm?
>
> Simon,
>
> Is this "simple" solution good enough for Wayland compositor use case?

I've tried your patch "mm: adds MAP_NOSIGBUS extension for shmem read" with a
libwayland hack [1] and a Wayland client that shrinks a shm file after the
compositor has mmap'ed it [2]. It seems to work nicely, thanks!

Regarding the requirements for Wayland:

- The baseline requirement is being able to avoid SIGBUS for read-only mappings
  of shm files.
- Wayland clients can expand their shm files. However the compositor doesn't
  need to immediately access the new expanded region. The client will tell the
  compositor what the new shm file size is, and the compositor will re-map it.
- Ideally, MAP_NOSIGBUS would work on PROT_WRITE + MAP_SHARED mappings (of
  course, the no-SIGBUS behavior would be restricted to that mapping). The
  use-case is writing back to client buffers e.g. for screen capture. From the
  earlier discussions it seems like this would be complicated to implement.
  This means we'll need to come up with a new libwayland API to allow
  compositors to opt-in to the read-only mappings. This is sub-optimal but
  seems doable.
- Ideally, MAP_SIGBUS wouldn't be restricted to shm. There are use-cases for
  using it on ordinary files too, e.g. for sharing ICC profiles. But from the
  earlier replies it seems very unlikely that this will become possible, and
  making it work only on shm files would already be fantastic.

Thanks again for working on this! Let me know if the above is unclear
or if some info is missing.

Simon

[1]: https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/145
[2]: https://github.com/emersion/wleird/blob/master/sigbus.c

Powered by blists - more mailing lists