linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Matthew Wilcox <willy@linux.intel.com>
Cc: Matthew Wilcox <matthew.r.wilcox@intel.com>,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, msharbiani@twopensource.com
Subject: Re: Should implementations of ->direct_access be allowed to sleep?
Date: Fri, 27 Mar 2015 06:32:24 +1100	[thread overview]
Message-ID: <20150326193224.GA28129@dastard> (raw)
In-Reply-To: <20150326170918.GO4003@linux.intel.com>

On Thu, Mar 26, 2015 at 01:09:18PM -0400, Matthew Wilcox wrote:
> On Tue, Mar 24, 2015 at 11:50:47AM -0700, Matt Mullins wrote:
> > We're also developing a user of direct_access, and we ended up with some
> > questions about the sleeping guarantees of the direct_access API.
> 
> That's a great question.  Since DAX can always sleep when it's calling
> into bdev_direct_access(), I hadn't thought about it (DAX is basically
> called to handle page faults and do I/O; both of which are expected
> to sleep).
> 
> > Since brd is currently the only (x86) implementation of DAX in Linus's tree,
> > I've been testing against that.  We noticed that the brd implementation of DAX
> > can call into alloc_page() with __GFP_WAIT if we call direct_access() on a page
> > that has not yet been allocated.  This is compounded by the fact that brd does
> > not support size > PAGE_SIZE (and thus I call bdev_direct_access() on each use),
> > though the limitation makes sense -- I shouldn't expect the brd driver to be
> > able to allocate a gigabyte of contiguous memory.
> > 
> > The potential sleeping behavior was somewhat surprising to me, as I would expect
> > the NV-DIMM device implementation to simply offset the pfn at which the device
> > is located rather than perform a memory allocation.  What are the guaranteed
> > and/or expected contexts from which direct_access() can be safely called?
> 
> Yes, for 'real' NV-DIMM devices, as you can see by the ones in tree,
> as well as the pmem driver that Ross has been posting, it's a simple
> piece of arithmetic.  The question is whether we should make all users
> of ->direct_access accommodate brd, or whether we should change brd so
> that it doesn't sleep.
> 
> I'm leaning towards the latter.  But I'm not sure what GFP flags to
> recommend that brd use ... GFP_NOWAIT | __GFP_ZERO, perhaps?

What, so we get random IO failures under memory pressure?

I really think we should allow .direct_access to sleep. It means we
can use existing drivers and it also allows future implementations
that might require, say, RDMA to be performed to update a page
before access is granted. i.e. .direct_access is the first hook into
the persistent device at page fault time....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2015-03-26 19:32 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-25 20:33 [PATCH v11 00/21] Add support for NV-DIMMs to ext4 Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 01/21] axonram: Fix bug in direct_access Matthew Wilcox
2014-10-16  7:52   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 02/21] block: Change direct_access calling convention Matthew Wilcox
2014-10-16  8:45   ` Mathieu Desnoyers
2014-10-16 19:39     ` Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 03/21] mm: Fix XIP fault vs truncate race Matthew Wilcox
2014-10-16  8:56   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 04/21] mm: Allow page fault handlers to perform the COW Matthew Wilcox
2014-10-16  9:12   ` Mathieu Desnoyers
2014-10-16 19:48     ` Matthew Wilcox
2014-10-17 15:35       ` Mathieu Desnoyers
2014-10-18 17:22         ` Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 05/21] vfs,ext2: Introduce IS_DAX(inode) Matthew Wilcox
2014-10-16  9:35   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 06/21] vfs: Add copy_to_iter(), copy_from_iter() and iov_iter_zero() Matthew Wilcox
2014-10-16 13:33   ` Mathieu Desnoyers
2014-10-16 13:59     ` Matthew Wilcox
2014-10-16 14:12       ` Mathieu Desnoyers
2014-10-16 22:21         ` Matthew Wilcox
2014-10-17 15:39           ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 07/21] dax,ext2: Replace XIP read and write with DAX I/O Matthew Wilcox
2014-10-16  9:50   ` Mathieu Desnoyers
2014-10-16 19:51     ` Matthew Wilcox
2014-10-16 22:33       ` Matthew Wilcox
2014-10-17 15:52         ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 08/21] dax,ext2: Replace ext2_clear_xip_target with dax_clear_blocks Matthew Wilcox
2014-10-16 10:05   ` Mathieu Desnoyers
2014-10-16 21:22     ` Matthew Wilcox
2014-10-17 15:45       ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 09/21] dax,ext2: Replace the XIP page fault handler with the DAX page fault handler Matthew Wilcox
2014-10-16 10:20   ` Mathieu Desnoyers
2014-10-16 21:29     ` Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 10/21] dax,ext2: Replace xip_truncate_page with dax_truncate_page Matthew Wilcox
2014-10-16 10:28   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 11/21] dax: Replace XIP documentation with DAX documentation Matthew Wilcox
2014-10-16 12:08   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 12/21] vfs: Remove get_xip_mem Matthew Wilcox
2014-10-16 12:14   ` Mathieu Desnoyers
2014-10-16 21:44     ` Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 13/21] ext2: Remove ext2_xip_verify_sb() Matthew Wilcox
2014-10-16 12:18   ` Mathieu Desnoyers
2014-10-16 21:45     ` Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 14/21] ext2: Remove ext2_use_xip Matthew Wilcox
2014-10-16 12:20   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 15/21] ext2: Remove xip.c and xip.h Matthew Wilcox
2014-10-16 12:21   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 16/21] vfs,ext2: Remove CONFIG_EXT2_FS_XIP and rename CONFIG_FS_XIP to CONFIG_FS_DAX Matthew Wilcox
2014-10-16 12:26   ` Mathieu Desnoyers
2014-10-16 21:52     ` Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 17/21] ext2: Remove ext2_aops_xip Matthew Wilcox
2014-10-16 12:29   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 18/21] ext2: Get rid of most mentions of XIP in ext2 Matthew Wilcox
2014-10-16 12:32   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 19/21] dax: Add dax_zero_page_range Matthew Wilcox
2014-10-16 12:38   ` Mathieu Desnoyers
2014-10-16 22:01     ` Matthew Wilcox
2014-10-17 15:49       ` Mathieu Desnoyers
2014-10-18 17:41         ` Matthew Wilcox
2014-10-18 21:16           ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 20/21] ext4: Add DAX functionality Matthew Wilcox
2014-10-16 12:56   ` Mathieu Desnoyers
2014-10-16 22:16     ` Matthew Wilcox
2014-10-17 15:42       ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 21/21] brd: Rename XIP to DAX Matthew Wilcox
2014-10-16 13:00   ` Mathieu Desnoyers
2015-03-24 18:50   ` Matt Mullins
2015-03-25  3:25     ` Dave Chinner
2015-03-26 17:09     ` Should implementations of ->direct_access be allowed to sleep? Matthew Wilcox
2015-03-26 19:32       ` Dave Chinner [this message]
2015-03-29  8:02         ` Boaz Harrosh
2015-03-29  9:13           ` Boaz Harrosh
2014-09-25 20:47 ` [PATCH v11 00/21] Add support for NV-DIMMs to ext4 Matthew Wilcox
2014-09-30  9:45 ` Valdis.Kletnieks
2014-09-30 14:48   ` Matthew Wilcox
2014-09-30 14:53     ` Valdis.Kletnieks
2014-09-30 16:08       ` Matthew Wilcox
2014-09-30 17:10         ` Zuckerman, Boris
2014-09-30 19:24           ` Matthew Wilcox
2014-09-30 19:31             ` Zuckerman, Boris
2014-09-30 20:37         ` Valdis.Kletnieks
2014-09-30 21:25           ` Andreas Dilger
2014-09-30 21:52             ` Valdis.Kletnieks
2014-10-01 15:45               ` Jeff Moyer
2014-10-01 17:10                 ` Valdis.Kletnieks
2014-10-01 17:17                 ` Valdis.Kletnieks
2014-10-16  7:39 ` Mathieu Desnoyers
2014-10-16 14:11   ` Matthew Wilcox

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=20150326193224.GA28129@dastard \
    --to=david@fromorbit.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=matthew.r.wilcox@intel.com \
    --cc=msharbiani@twopensource.com \
    --cc=willy@linux.intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).