Hi On Wed, Jul 16, 2014 at 12:09 PM, Hugh Dickins <hughd@xxxxxxxxxx> wrote: > 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@xxxxxxxxx> > > 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(). The main reason why I split both is to avoid goto-label "restart" and "restart2". > 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 added an lru_add_drain_all() to my shmem_test_pins() function in Patch 2/7. This patch dropped it again as your wait_for_pins() already included it and it's quite expensive. But yes, the local lru_add_drain() makes perfect sense. Fixed! Thanks David > 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. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>