linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>,
	linux-kernel@vger.kernel.org, Theodore Ts'o <tytso@mit.edu>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@lst.de>,
	Dan Williams <dan.j.williams@intel.com>,
	Dave Chinner <david@fromorbit.com>, Jan Kara <jack@suse.cz>,
	Matthew Wilcox <mawilcox@microsoft.com>,
	linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, linux-nvdimm@lists.01.org,
	linux-xfs@vger.kernel.org
Subject: Re: [PATCH v7 15/17] dax: add struct iomap based DAX PMD support
Date: Mon, 17 Oct 2016 08:55:36 -0600	[thread overview]
Message-ID: <20161017145536.GB7645@linux.intel.com> (raw)
In-Reply-To: <87a8e3tsow.fsf@linux.vnet.ibm.com>

On Mon, Oct 17, 2016 at 11:36:55AM +0530, Aneesh Kumar K.V wrote:
> Ross Zwisler <ross.zwisler@linux.intel.com> writes:
> 
> > DAX PMDs have been disabled since Jan Kara introduced DAX radix tree based
> > locking.  This patch allows DAX PMDs to participate in the DAX radix tree
> > based locking scheme so that they can be re-enabled using the new struct
> > iomap based fault handlers.
> >
> > There are currently three types of DAX 4k entries: 4k zero pages, 4k DAX
> > mappings that have an associated block allocation, and 4k DAX empty
> > entries.  The empty entries exist to provide locking for the duration of a
> > given page fault.
> >
> > This patch adds three equivalent 2MiB DAX entries: Huge Zero Page (HZP)
> > entries, PMD DAX entries that have associated block allocations, and 2 MiB
> > DAX empty entries.
> >
> > Unlike the 4k case where we insert a struct page* into the radix tree for
> > 4k zero pages, for HZP we insert a DAX exceptional entry with the new
> > RADIX_DAX_HZP flag set.  This is because we use a single 2 MiB zero page in
> > every 2MiB hole mapping, and it doesn't make sense to have that same struct
> > page* with multiple entries in multiple trees.  This would cause contention
> > on the single page lock for the one Huge Zero Page, and it would break the
> > page->index and page->mapping associations that are assumed to be valid in
> > many other places in the kernel.
> >
> > One difficult use case is when one thread is trying to use 4k entries in
> > radix tree for a given offset, and another thread is using 2 MiB entries
> > for that same offset.  The current code handles this by making the 2 MiB
> > user fall back to 4k entries for most cases.  This was done because it is
> > the simplest solution, and because the use of 2MiB pages is already
> > opportunistic.
> >
> > If we were to try to upgrade from 4k pages to 2MiB pages for a given range,
> > we run into the problem of how we lock out 4k page faults for the entire
> > 2MiB range while we clean out the radix tree so we can insert the 2MiB
> > entry.  We can solve this problem if we need to, but I think that the cases
> > where both 2MiB entries and 4K entries are being used for the same range
> > will be rare enough and the gain small enough that it probably won't be
> > worth the complexity.
> >
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > Reviewed-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/dax.c            | 378 +++++++++++++++++++++++++++++++++++++++++++++++-----
> >  include/linux/dax.h |  55 ++++++--
> >  mm/filemap.c        |   3 +-
> >  3 files changed, 386 insertions(+), 50 deletions(-)
> >
> > diff --git a/fs/dax.c b/fs/dax.c
> > index 0582c7c..153cfd5 100644
> > --- a/fs/dax.c
> > +++ b/fs/dax.c
> > @@ -76,6 +76,26 @@ static void dax_unmap_atomic(struct block_device *bdev,
> >  	blk_queue_exit(bdev->bd_queue);
> >  }
> >  
> > +static int dax_is_pmd_entry(void *entry)
> > +{
> > +	return (unsigned long)entry & RADIX_DAX_PMD;
> > +}
> > +
> > +static int dax_is_pte_entry(void *entry)
> > +{
> > +	return !((unsigned long)entry & RADIX_DAX_PMD);
> > +}
> > +
> > +static int dax_is_zero_entry(void *entry)
> > +{
> > +	return (unsigned long)entry & RADIX_DAX_HZP;
> > +}
> 
> How about dax_is_pmd_zero_entry() ?

It's on my to-do list to convert the 4k DAX zero page case to use a singleton
page as well, in which case it's my plan to reuse this helper for both the 4k
and the PMD case.  Having it called dax_is_zero_entry() instead of
dax_is_pmd_zero_entry() allows for this - we'll just have to rename the
underling flag.

--
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:[~2016-10-17 14:55 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-12 22:50 [PATCH v6 00/17] re-enable DAX PMD support Ross Zwisler
     [not found] ` <20161012225022.15507-1-ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2016-10-12 22:50   ` [PATCH v6 01/17] ext4: allow DAX writeback for hole punch Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 02/17] ext4: tell DAX the size of allocation holes Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 03/17] dax: remove buffer_size_valid() Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 04/17] ext2: remove support for DAX PMD faults Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 07/17] dax: consistent variable naming for DAX entries Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 08/17] dax: coordinate locking for offsets in PMD range Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 09/17] dax: remove dax_pmd_fault() Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 10/17] dax: correct dax iomap code namespace Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 11/17] dax: add dax_iomap_sector() helper function Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 12/17] dax: dax_iomap_fault() needs to call iomap_end() Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 13/17] dax: move RADIX_DAX_* defines to dax.h Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 14/17] dax: move put_(un)locked_mapping_entry() in dax.c Ross Zwisler
2016-10-13 15:11     ` Jan Kara
2016-10-12 22:50   ` [PATCH v6 15/17] dax: add struct iomap based DAX PMD support Ross Zwisler
     [not found]     ` <20161012225022.15507-16-ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2016-10-13 15:42       ` Jan Kara
2016-10-13 19:23         ` [PATCH v7 " Ross Zwisler
2016-10-17  6:06           ` Aneesh Kumar K.V
2016-10-17 14:55             ` Ross Zwisler [this message]
2016-10-13 19:25         ` [PATCH v6 " Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 16/17] xfs: use struct iomap based DAX PMD fault path Ross Zwisler
2016-10-12 22:50   ` [PATCH v6 17/17] dax: remove "depends on BROKEN" from FS_DAX_PMD Ross Zwisler
     [not found]     ` <20161012225022.15507-18-ross.zwisler-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2016-10-13 15:42       ` Jan Kara
2016-10-17  5:57       ` Aneesh Kumar K.V
     [not found]         ` <87eg3ftt4r.fsf-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2016-10-17  9:09           ` Jan Kara
2016-10-12 22:50 ` [PATCH v6 05/17] dax: make 'wait_table' global variable static Ross Zwisler
2016-10-12 22:50 ` [PATCH v6 06/17] dax: remove the last BUG_ON() from fs/dax.c Ross Zwisler

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=20161017145536.GB7645@linux.intel.com \
    --to=ross.zwisler@linux.intel.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=akpm@linux-foundation.org \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=dan.j.williams@intel.com \
    --cc=david@fromorbit.com \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=mawilcox@microsoft.com \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /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).