linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Jens Axboe <axboe@fb.com>, Jan Kara <jack@suse.cz>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@ml01.01.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Ross Zwisler <ross.zwisler@linux.intel.com>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v3 14/15] dax: dirty extent notification
Date: Tue, 3 Nov 2015 16:40:39 +1100	[thread overview]
Message-ID: <20151103054039.GQ10656@dastard> (raw)
In-Reply-To: <CAPcyv4hof4rVN0EZHhV9Q7VBE0WMw6hcSrLK-HvB5FOrOwY+tg@mail.gmail.com>

On Mon, Nov 02, 2015 at 08:56:24PM -0800, Dan Williams wrote:
> On Mon, Nov 2, 2015 at 5:16 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Sun, Nov 01, 2015 at 11:30:58PM -0500, Dan Williams wrote:
> >> DAX-enabled block device drivers can use hints from fs/dax.c to
> >> optimize their internal tracking of potentially dirty cpu cache lines.
> >> If a DAX mapping is being used for synchronous operations, dax_do_io(),
> >> a dax-enabled block-driver knows that fs/dax.c will handle immediate
> >> flushing.  For asynchronous mappings, i.e.  returned to userspace via
> >> mmap, the driver can track active extents of the media for flushing.
> >
> > So, essentially, you are marking the calls into the mapping calls
> > with BLKDAX_F_DIRTY when the mapping is requested for a write page
> > fault?  Hence allowing the block device to track "dirty pages"
> > exactly?
> 
> Not pages, but larger extents (1 extent = 1/NUM_DAX_EXTENTS of the
> total storage capacity), because tracking dirty mappings should be
> temporary compatibility hack and not a first class citizen.
> 
> > But, really, if we're going to use Ross's mapping tree patches that
> > use exceptional entries to track dirty pfns, why do we need to this
> > special interface from DAX to the block device? Ross's changes will
> > track mmap'd ranges that are dirtied at the filesytem inode level,
> > and the fsync/writeback will trigger CPU cache writeback of those
> > dirty ranges. This will work for block devices that are mapped by
> > DAX, too, because they have a inode+mapping tree, too.
> >
> > And if we are going to use Ross's infrastructure (which, when we
> > work the kinks out of, I think we will), we really should change
> > dax_do_io() to track pfns that are dirtied this way, too. That will
> > allow us to get rid of all the cache flushing from the DAX layer
> > (they'll get pushed into fsync/writeback) and so we only take the
> > CPU cache flushing penalties when synchronous operations are
> > requested by userspace...
> 
> No, we definitely can't do that.   I think your mental model of the
> cache flushing is similar to the disk model where a small buffer is
> flushed after a large streaming write.  Both Ross' patches and my
> approach suffer from the same horror that the cache flushing is O(N)
> currently, so we don't want to make it responsible for more data
> ranges areas than is strictly necessary.

I didn't see anything that was O(N) in Ross's patches. What part of
the fsync algorithm that Ross proposed are you refering to here?

> >> We can later extend the DAX paths to indicate when an async mapping is
> >> "closed" allowing the active extents to be marked clean.
> >
> > Yes, that's a basic feature of Ross's patches. Hence I think this
> > special case DAX<->bdev interface is the wrong direction to be
> > taking.
> 
> So here's my problem with the "track dirty mappings" in the core
> mm/vfs approach, it's harder to unwind and delete when it turns out no
> application actually needs it, or the platform gives us an O(1) flush
> method that is independent of dirty pte tracking.
> 
> We have the NVML [1] library as the recommended method for
> applications to interact with persistent memory and it is not using
> fsync/msync for its synchronization primitives, it's managing the
> cache directly.  The *only* user for tracking dirty DAX mappings is
> unmodified legacy applications that do mmap I/O and call fsync/msync.

I'm pretty sure there are going to be many people still writing new
applications that use POSIX APIs they expect to work correctly on
pmem because, well, it's going to take 10 years before persistent
memory is common enough for most application developers to only
target storage via NVML.

The whole world is not crazy HFT applications that need to bypass
the kernel for *everything* because even a few nanoseconds of extra
latency matters.

> DAX in my opinion is not a transparent accelerator of all existing
> apps, it's a targeted mechanism for applications ready to take
> advantage of byte addressable persistent memory. 

And this is where we disagree. DAX is a method of allowing POSIX
compliant applications get the best of both worlds - portability
with existing storage and filesystems, yet with the speed and byte
addressiblity of persistent storage through the use of mmap.

Applications designed specifically for persistent memory don't want
a general purpose, POSIX compatible filesystem underneath them. The
should be interacting directly with, and only with, your NVML
library. If the NVML library is implemented by using DAX on a POSIX
compatible, general purpose filesystem, then you're just going to
have to live with everything we need to do to make DAX work with
general purpose POSIX compatible applications.

DAX has always been intended as a *stopgap measure* designed to
bridge the gap between existing POSIX based storage APIs and PMEM
native filesystem implementations. You're advocating that DAX should
only be used by PMEM native applications using NVML and then saying
anything that might be needed for POSIX compatible behaviour is
unacceptible overhead...

> This is why I'm a
> big supporter of your per-inode DAX control proposal.  The fact that
> fsync is painful for large amounts of dirty data is a feature.  It
> detects inodes that should have had DAX-disabled in the first
> instance.

fsync is painful for any storage when there is large amounts of
dirty data. DAX is no different, and it's not a reason for saying
"don't use DAX". DAX + fsync should be faster than "buffered IO
through the page cache on pmem + fsync" because there is only one
memory copy being done in the DAX case.

The buffered IO case has all that per-page radix tree tracking in it,
writeback, etc. Yet:

# mount -o dax /dev/ram0 /mnt/scratch
# time xfs_io -fc "truncate 0" -c "pwrite -b 8m 0 3g" -c fsync /mnt/scratch/file
wrote 3221225472/3221225472 bytes at offset 0
3.000 GiB, 384 ops; 0:00:10.00 (305.746 MiB/sec and 38.2182 ops/sec)
0.00user 10.05system 0:10.05elapsed 100%CPU (0avgtext+0avgdata 10512maxresident)k
0inputs+0outputs (0major+2156minor)pagefaults 0swaps
# umount /mnt/scratch
# mount /dev/ram0 /mnt/scratch
# time xfs_io -fc "truncate 0" -c "pwrite -b 8m 0 3g" -c fsync /mnt/scratch/file
wrote 3221225472/3221225472 bytes at offset 0
3.000 GiB, 384 ops; 0:00:02.00 (1.218 GiB/sec and 155.9046 ops/sec)
0.00user 2.83system 0:02.86elapsed 99%CPU (0avgtext+0avgdata 10468maxresident)k
0inputs+0outputs (0major+2154minor)pagefaults 0swaps
#

So don't tell me that tracking dirty pages in the radix tree too
slow for DAX and that DAX should not be used for POSIX IO based
applications - it should be as fast as buffered IO, if not faster,
and if it isn't then we've screwed up real bad. And right now, we're
screwing up real bad.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2015-11-03  5:41 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-02  4:29 [PATCH v3 00/15] block, dax updates for 4.4 Dan Williams
2015-11-02  4:29 ` [PATCH v3 01/15] pmem, dax: clean up clear_pmem() Dan Williams
2015-11-02  4:29 ` [PATCH v3 02/15] dax: increase granularity of dax_clear_blocks() operations Dan Williams
2015-11-03  0:51   ` Dave Chinner
2015-11-03  3:27     ` Dan Williams
2015-11-03  4:48       ` Dave Chinner
2015-11-03  5:31         ` Dan Williams
2015-11-03  5:52           ` Dave Chinner
2015-11-03  7:24             ` Dan Williams
2015-11-03 16:21           ` Jan Kara
2015-11-03 17:57           ` Ross Zwisler
2015-11-03 20:59             ` Dave Chinner
2015-11-02  4:29 ` [PATCH v3 03/15] block, dax: fix lifetime of in-kernel dax mappings with dax_map_atomic() Dan Williams
2015-11-03 19:01   ` Ross Zwisler
2015-11-03 19:09     ` Jeff Moyer
2015-11-03 22:50     ` Dan Williams
2016-01-18 10:42   ` Geert Uytterhoeven
2015-11-02  4:30 ` [PATCH v3 04/15] libnvdimm, pmem: move request_queue allocation earlier in probe Dan Williams
2015-11-03 19:15   ` Ross Zwisler
2015-11-02  4:30 ` [PATCH v3 05/15] libnvdimm, pmem: fix size trim in pmem_direct_access() Dan Williams
2015-11-03 19:32   ` Ross Zwisler
2015-11-03 21:39     ` Dan Williams
2015-11-02  4:30 ` [PATCH v3 06/15] um: kill pfn_t Dan Williams
2015-11-02  4:30 ` [PATCH v3 07/15] kvm: rename pfn_t to kvm_pfn_t Dan Williams
2015-11-02  4:30 ` [PATCH v3 08/15] mm, dax, pmem: introduce pfn_t Dan Williams
2015-11-02 16:30   ` Joe Perches
2015-11-02  4:30 ` [PATCH v3 09/15] block: notify queue death confirmation Dan Williams
2015-11-02  4:30 ` [PATCH v3 10/15] dax, pmem: introduce zone_device_revoke() and devm_memunmap_pages() Dan Williams
2015-11-02  4:30 ` [PATCH v3 11/15] block: introduce bdev_file_inode() Dan Williams
2015-11-02  4:30 ` [PATCH v3 12/15] block: enable dax for raw block devices Dan Williams
2015-11-02  4:30 ` [PATCH v3 13/15] block, dax: make dax mappings opt-in by default Dan Williams
2015-11-03  0:32   ` Dave Chinner
2015-11-03  7:35     ` Dan Williams
2015-11-03 20:20       ` Dave Chinner
2015-11-03 23:04         ` Dan Williams
2015-11-04 19:23           ` Dan Williams
2015-11-02  4:30 ` [PATCH v3 14/15] dax: dirty extent notification Dan Williams
2015-11-03  1:16   ` Dave Chinner
2015-11-03  4:56     ` Dan Williams
2015-11-03  5:40       ` Dave Chinner [this message]
2015-11-03  7:20         ` Dan Williams
2015-11-03 20:51           ` Dave Chinner
2015-11-03 21:19             ` Dan Williams
2015-11-03 21:37             ` Ross Zwisler
2015-11-03 21:43               ` Dan Williams
2015-11-03 21:18       ` Ross Zwisler
2015-11-03 21:34         ` Dan Williams
2015-11-02  4:31 ` [PATCH v3 15/15] pmem: blkdev_issue_flush support Dan Williams

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=20151103054039.GQ10656@dastard \
    --to=david@fromorbit.com \
    --cc=axboe@fb.com \
    --cc=dan.j.williams@intel.com \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@ml01.01.org \
    --cc=ross.zwisler@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).