All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hugh Dickins <hughd@google.com>
To: David Herrmann <dh.herrmann@gmail.com>
Cc: linux-kernel@vger.kernel.org,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	Ryan Lortie <desrt@desrt.ca>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
	linux-api@vger.kernel.org, Greg Kroah-Hartman <greg@kroah.com>,
	john.stultz@linaro.org,
	Lennart Poettering <lennart@poettering.net>,
	Daniel Mack <zonque@gmail.com>, Kay Sievers <kay@vrfy.org>,
	Hugh Dickins <hughd@google.com>,
	Tony Battersby <tonyb@cybernetics.com>,
	Andy Lutomirski <luto@amacapital.net>
Subject: Re: [RFC v3 6/7] shm: wait for pins to be released when sealing
Date: Wed, 16 Jul 2014 03:09:58 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LSU.2.11.1407160308550.1775@eggly.anvils> (raw)
In-Reply-To: <1402655819-14325-7-git-send-email-dh.herrmann@gmail.com>

On Fri, 13 Jun 2014, David Herrmann wrote:

> We currently fail setting SEAL_WRITE in case there're pending page
> references. This patch extends the pin-tests to wait up to 150ms for all
> references to be dropped. This is still not perfect in that it doesn't
> account for harmless read-only pins, but it's much better than a hard
> failure.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Right, I didn't look through the patch itself, just compared the result
with what I sent.  Okay, you prefer to separate out shmem_tag_pins().

Yes, it looks fine.  There's just one change I'd like at this stage,
something I realized shortly after sending the code fragment: please
add a call to lru_add_drain() at the head of shmem_tag_pins().  The
reason being that lru_add_drain() is local to the cpu, so cheap, and
in many cases will bring down all the raised refcounts right then.

Whereas lru_add_drain_all() in the first scan of shmem_wait_for_pins()
is much more expensive, involving inter-processor interrupts to do
that on all cpus: it is appropriate to call it at that point, but we
really ought to try the cheaper lru_add_drain() at the earlier stage.

I would also like never to embark on this scan of the radix_tree
and wait for pins, if the pages were never given out in a VM_SHARED
mapping - or is that unrealistic, because every memfd is read-write,
and typical initialization expected to be by mmap() rather than write()?
But anyway, you're quite right not to get into that at this stage:
it's best left as an optimization once the basics are safely in.

Hugh

WARNING: multiple messages have this Message-ID (diff)
From: Hugh Dickins <hughd@google.com>
To: David Herrmann <dh.herrmann@gmail.com>
Cc: linux-kernel@vger.kernel.org,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	Ryan Lortie <desrt@desrt.ca>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
	linux-api@vger.kernel.org, Greg Kroah-Hartman <greg@kroah.com>,
	john.stultz@linaro.org,
	Lennart Poettering <lennart@poettering.net>,
	Daniel Mack <zonque@gmail.com>, Kay Sievers <kay@vrfy.org>,
	Hugh Dickins <hughd@google.com>,
	Tony Battersby <tonyb@cybernetics.com>,
	Andy Lutomirski <luto@amacapital.net>
Subject: Re: [RFC v3 6/7] shm: wait for pins to be released when sealing
Date: Wed, 16 Jul 2014 03:09:58 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LSU.2.11.1407160308550.1775@eggly.anvils> (raw)
In-Reply-To: <1402655819-14325-7-git-send-email-dh.herrmann@gmail.com>

On Fri, 13 Jun 2014, David Herrmann wrote:

> We currently fail setting SEAL_WRITE in case there're pending page
> references. This patch extends the pin-tests to wait up to 150ms for all
> references to be dropped. This is still not perfect in that it doesn't
> account for harmless read-only pins, but it's much better than a hard
> failure.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Right, I didn't look through the patch itself, just compared the result
with what I sent.  Okay, you prefer to separate out shmem_tag_pins().

Yes, it looks fine.  There's just one change I'd like at this stage,
something I realized shortly after sending the code fragment: please
add a call to lru_add_drain() at the head of shmem_tag_pins().  The
reason being that lru_add_drain() is local to the cpu, so cheap, and
in many cases will bring down all the raised refcounts right then.

Whereas lru_add_drain_all() in the first scan of shmem_wait_for_pins()
is much more expensive, involving inter-processor interrupts to do
that on all cpus: it is appropriate to call it at that point, but we
really ought to try the cheaper lru_add_drain() at the earlier stage.

I would also like never to embark on this scan of the radix_tree
and wait for pins, if the pages were never given out in a VM_SHARED
mapping - or is that unrealistic, because every memfd is read-write,
and typical initialization expected to be by mmap() rather than write()?
But anyway, you're quite right not to get into that at this stage:
it's best left as an optimization once the basics are safely in.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2014-07-16 10:11 UTC|newest]

Thread overview: 107+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-13 10:36 [PATCH v3 0/7] File Sealing & memfd_create() David Herrmann
2014-06-13 10:36 ` David Herrmann
2014-06-13 10:36 ` [PATCH v3 1/7] mm: allow drivers to prevent new writable mappings David Herrmann
2014-06-13 10:36   ` David Herrmann
2014-07-09  8:55   ` Hugh Dickins
2014-07-09  8:55     ` Hugh Dickins
2014-07-19 16:12     ` David Herrmann
2014-07-19 16:12       ` David Herrmann
2014-06-13 10:36 ` [PATCH v3 2/7] shm: add sealing API David Herrmann
2014-06-13 10:36   ` David Herrmann
2014-07-16 10:06   ` Hugh Dickins
2014-07-16 10:06     ` Hugh Dickins
2014-07-19 16:17     ` David Herrmann
2014-07-19 16:17       ` David Herrmann
2014-06-13 10:36 ` [PATCH v3 3/7] shm: add memfd_create() syscall David Herrmann
2014-06-13 10:36   ` David Herrmann
2014-06-13 12:27   ` Michael Kerrisk (man-pages)
2014-06-13 12:27     ` Michael Kerrisk (man-pages)
2014-06-13 12:41     ` David Herrmann
2014-06-13 12:41       ` David Herrmann
2014-06-13 14:20       ` Michael Kerrisk (man-pages)
2014-06-13 14:20         ` Michael Kerrisk (man-pages)
2014-06-13 16:20         ` John Stultz
2014-06-13 16:20           ` John Stultz
2014-06-13 16:20           ` John Stultz
2014-06-16  4:12           ` Michael Kerrisk (man-pages)
2014-06-16  4:12             ` Michael Kerrisk (man-pages)
2014-07-08 18:39         ` David Herrmann
2014-07-08 18:39           ` David Herrmann
2014-06-15 10:50   ` Jann Horn
2014-07-16 10:07   ` Hugh Dickins
2014-07-16 10:07     ` Hugh Dickins
2014-07-19 16:29     ` David Herrmann
2014-07-19 16:29       ` David Herrmann
2014-06-13 10:36 ` [PATCH v3 4/7] selftests: add memfd_create() + sealing tests David Herrmann
2014-06-13 10:36   ` David Herrmann
2014-07-16 10:07   ` Hugh Dickins
2014-07-16 10:07     ` Hugh Dickins
2014-07-19 16:31     ` David Herrmann
2014-07-19 16:31       ` David Herrmann
2014-06-13 10:36 ` [PATCH v3 5/7] selftests: add memfd/sealing page-pinning tests David Herrmann
2014-06-13 10:36   ` David Herrmann
2014-07-16 10:08   ` Hugh Dickins
2014-07-16 10:08     ` Hugh Dickins
2014-07-19 16:32     ` David Herrmann
2014-07-19 16:32       ` David Herrmann
2014-06-13 10:36 ` [RFC v3 6/7] shm: wait for pins to be released when sealing David Herrmann
2014-06-13 10:36   ` David Herrmann
2014-07-16 10:09   ` Hugh Dickins [this message]
2014-07-16 10:09     ` Hugh Dickins
2014-07-19 16:36     ` David Herrmann
2014-07-19 16:36       ` David Herrmann
2014-06-13 10:36 ` [RFC v3 7/7] shm: isolate pinned pages when sealing files David Herrmann
2014-06-13 10:36   ` David Herrmann
2014-06-13 15:06   ` Andy Lutomirski
2014-06-13 15:06     ` Andy Lutomirski
2014-06-13 15:27     ` David Herrmann
2014-06-13 15:27       ` David Herrmann
2014-06-13 17:23       ` Andy Lutomirski
2014-06-13 17:23         ` Andy Lutomirski
2014-07-09  8:57   ` Hugh Dickins
2014-07-09  8:57     ` Hugh Dickins
2014-07-19 16:40     ` David Herrmann
2014-07-19 16:40       ` David Herrmann
2014-06-13 15:10 ` [PATCH v3 0/7] File Sealing & memfd_create() Andy Lutomirski
2014-06-13 15:10   ` Andy Lutomirski
2014-06-13 15:15   ` David Herrmann
2014-06-13 15:15     ` David Herrmann
2014-06-13 15:15     ` David Herrmann
2014-06-13 15:17     ` Andy Lutomirski
2014-06-13 15:17       ` Andy Lutomirski
2014-06-13 15:17       ` Andy Lutomirski
2014-06-13 15:33       ` David Herrmann
2014-06-13 15:33         ` David Herrmann
2014-06-13 15:33         ` David Herrmann
2014-06-17  9:54         ` Florian Weimer
2014-06-17  9:54           ` Florian Weimer
2014-06-17 10:01           ` David Herrmann
2014-06-17 10:01             ` David Herrmann
2014-06-17 10:01             ` David Herrmann
2014-06-17 10:04             ` Florian Weimer
2014-06-17 10:04               ` Florian Weimer
2014-06-17 10:10               ` David Herrmann
2014-06-17 10:10                 ` David Herrmann
2014-06-17 12:13                 ` Florian Weimer
2014-06-17 12:13                   ` Florian Weimer
2014-06-17 13:26                   ` David Herrmann
2014-06-17 13:26                     ` David Herrmann
2014-06-17 13:26                     ` David Herrmann
2014-06-17 16:20             ` Andy Lutomirski
2014-06-17 16:36               ` David Herrmann
2014-06-17 16:36                 ` David Herrmann
2014-06-17 16:41                 ` Andy Lutomirski
2014-06-17 16:41                   ` Andy Lutomirski
2014-06-17 16:51                   ` David Herrmann
2014-06-17 16:51                     ` David Herrmann
2014-06-17 17:01                     ` Andy Lutomirski
2014-06-17 17:01                       ` Andy Lutomirski
2014-06-17 20:31                       ` Hugh Dickins
2014-06-17 20:31                         ` Hugh Dickins
2014-06-17 20:31                         ` Hugh Dickins
2014-06-17 21:25                         ` Andy Lutomirski
2014-06-17 21:25                           ` Andy Lutomirski
2014-07-08 16:54 ` David Herrmann
2014-07-08 16:54   ` David Herrmann
2014-07-09  8:53   ` Hugh Dickins
2014-07-09  8:53     ` Hugh Dickins

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LSU.2.11.1407160308550.1775@eggly.anvils \
    --to=hughd@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=desrt@desrt.ca \
    --cc=dh.herrmann@gmail.com \
    --cc=greg@kroah.com \
    --cc=john.stultz@linaro.org \
    --cc=kay@vrfy.org \
    --cc=lennart@poettering.net \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=mtk.manpages@gmail.com \
    --cc=tonyb@cybernetics.com \
    --cc=torvalds@linux-foundation.org \
    --cc=zonque@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.