Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1513485 > unrolled thread

[PATCH v9 00/16] re-enable DAX PMD support

Back to article view | Back to linux.kernel

Contents

  [PATCH v9 00/16] re-enable DAX PMD support Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-11-01 21:10 +0100
    [PATCH v9 02/16] dax: remove buffer_size_valid() Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-11-01 21:10 +0100
    Re: [PATCH v9 00/16] re-enable DAX PMD support Dave Chinner <david@fromorbit.com> - 2016-11-03 03:00 +0100
      Re: [PATCH v9 00/16] re-enable DAX PMD support Ross Zwisler <ross.zwisler@linux.intel.com> - 2016-11-03 19:00 +0100
        Re: [PATCH v9 00/16] re-enable DAX PMD support Dave Chinner <david@fromorbit.com> - 2016-11-03 22:20 +0100

#1513485 — [PATCH v9 00/16] re-enable DAX PMD support

DAX PMDs have been disabled since Jan Kara introduced DAX radix tree based
locking.  This series allows DAX PMDs to participate in the DAX radix tree
based locking scheme so that they can be re-enabled.

Previously we had talked about this series going through the XFS tree, but
Jan has a patch set that will need to build on this series and it heavily
modifies the MM code.  I think he would prefer that series to go through
Andrew Morton's -MM tree, so it probably makes sense for this series to go
through that same tree.

For reference, here is the series from Jan that I was talking about:
https://marc.info/?l=linux-mm&m=147499252322902&w=2

Andrew, can you please pick this up for the v4.10 merge window?
This series is currently based on v4.9-rc3.  I tried to rebase onto a -mm
branch or tag, but couldn't find one that contained the DAX iomap changes
that were merged as part of the v4.9 merge window.  I'm happy to rebase &
test on a v4.9-rc* based -MM branch or tag whenever they are available.

Changes since v8:
- Rebased onto v4.9-rc3.
- Updated the DAX PMD fault path so that on fallback we always check to see
  if we are dealing with a transparent huge page, and if we are we will
  split it.  This was already happening for one of the fallback cases via a
  patch from Toshi, and Jan hit a deadlock in another fallback case where
  the same splitting was needed.  (Jan & Toshi)

This series has passed all my xfstests testing, including the test that was
hitting the deadlock with v8.

Here is a tree containing my changes:
https://git.kernel.org/cgit/linux/kernel/git/zwisler/linux.git/log/?h=dax_pmd_v9

Ross Zwisler (16):
  ext4: tell DAX the size of allocation holes
  dax: remove buffer_size_valid()
  ext2: remove support for DAX PMD faults
  dax: make 'wait_table' global variable static
  dax: remove the last BUG_ON() from fs/dax.c
  dax: consistent variable naming for DAX entries
  dax: coordinate locking for offsets in PMD range
  dax: remove dax_pmd_fault()
  dax: correct dax iomap code namespace
  dax: add dax_iomap_sector() helper function
  dax: dax_iomap_fault() needs to call iomap_end()
  dax: move RADIX_DAX_* defines to dax.h
  dax: move put_(un)locked_mapping_entry() in dax.c
  dax: add struct iomap based DAX PMD support
  xfs: use struct iomap based DAX PMD fault path
  dax: remove "depends on BROKEN" from FS_DAX_PMD

 fs/Kconfig          |   1 -
 fs/dax.c            | 826 +++++++++++++++++++++++++++++-----------------------
 fs/ext2/file.c      |  35 +--
 fs/ext4/inode.c     |   3 +
 fs/xfs/xfs_aops.c   |  26 +-
 fs/xfs/xfs_aops.h   |   3 -
 fs/xfs/xfs_file.c   |  10 +-
 include/linux/dax.h |  58 +++-
 mm/filemap.c        |   5 +-
 9 files changed, 537 insertions(+), 430 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1513487 — [PATCH v9 02/16] dax: remove buffer_size_valid()

Now that ext4 properly sets bh.b_size when we call get_block() for a hole,
rely on that value and remove the buffer_size_valid() sanity check.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/dax.c | 22 +---------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 014defd..b09817a 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -123,19 +123,6 @@ static bool buffer_written(struct buffer_head *bh)
 	return buffer_mapped(bh) && !buffer_unwritten(bh);
 }
 
-/*
- * When ext4 encounters a hole, it returns without modifying the buffer_head
- * which means that we can't trust b_size.  To cope with this, we set b_state
- * to 0 before calling get_block and, if any bit is set, we know we can trust
- * b_size.  Unfortunate, really, since ext4 knows precisely how long a hole is
- * and would save us time calling get_block repeatedly.
- */
-static bool buffer_size_valid(struct buffer_head *bh)
-{
-	return bh->b_state != 0;
-}
-
-
 static sector_t to_sector(const struct buffer_head *bh,
 		const struct inode *inode)
 {
@@ -177,8 +164,6 @@ static ssize_t dax_io(struct inode *inode, struct iov_iter *iter,
 				rc = get_block(inode, block, bh, rw == WRITE);
 				if (rc)
 					break;
-				if (!buffer_size_valid(bh))
-					bh->b_size = 1 << blkbits;
 				bh_max = pos - first + bh->b_size;
 				bdev = bh->b_bdev;
 				/*
@@ -1012,12 +997,7 @@ int dax_pmd_fault(struct vm_area_struct *vma, unsigned long address,
 
 	bdev = bh.b_bdev;
 
-	/*
-	 * If the filesystem isn't willing to tell us the length of a hole,
-	 * just fall back to PTEs.  Calling get_block 512 times in a loop
-	 * would be silly.
-	 */
-	if (!buffer_size_valid(&bh) || bh.b_size < PMD_SIZE) {
+	if (bh.b_size < PMD_SIZE) {
 		dax_pmd_dbg(&bh, address, "allocated block too small");
 		return VM_FAULT_FALLBACK;
 	}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1514244

On Tue, Nov 01, 2016 at 01:54:02PM -0600, Ross Zwisler wrote:
> DAX PMDs have been disabled since Jan Kara introduced DAX radix tree based
> locking.  This series allows DAX PMDs to participate in the DAX radix tree
> based locking scheme so that they can be re-enabled.

I've seen patch 0/16 - where did you send the other 16? I need to
pick up the bug fix that is in this patch set...

> Previously we had talked about this series going through the XFS tree, but
> Jan has a patch set that will need to build on this series and it heavily
> modifies the MM code.  I think he would prefer that series to go through
> Andrew Morton's -MM tree, so it probably makes sense for this series to go
> through that same tree.

Seriously, I was 10 minutes away from pushing out the previous
version of this patchset as a stable topic branch, just like has
been discussed and several times over the past week.  Indeed, I
mentioned that I was planning on pushing out this topic branch today
not more than 4 hours ago, and you were on the cc list.

The -mm tree is not the place to merge patchsets with dependencies
like this because it's an unstable, rebasing tree. Hence it cannot
be shared and used as the base of common development between
multiple git trees like we have for the fs/ subsystem.

This needs to go out as a stable topic branch so that other
dependent work can reliably build on top of it for the next merge
window. e.g. the ext4 DAX iomap patch series that is likely to be
merged through the ext4 tree, so it needs a stable branch. There's
iomap direct IO patches for XFS pending, and they conflict with this
patchset. i.e. we need a stable git base to work from...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

#1514753

On Thu, Nov 03, 2016 at 12:58:26PM +1100, Dave Chinner wrote:
> On Tue, Nov 01, 2016 at 01:54:02PM -0600, Ross Zwisler wrote:
> > DAX PMDs have been disabled since Jan Kara introduced DAX radix tree based
> > locking.  This series allows DAX PMDs to participate in the DAX radix tree
> > based locking scheme so that they can be re-enabled.
> 
> I've seen patch 0/16 - where did you send the other 16? I need to
> pick up the bug fix that is in this patch set...

I CC'd your "david@fromorbit.com" address on the entire set, as well as all
the usual lists (linux-xfs, linux-fsdevel, linux-nvdimm, etc).

They are also available via the libnvdimm patchwork:

https://patchwork.kernel.org/project/linux-nvdimm/list/

or via my tree:

https://git.kernel.org/cgit/linux/kernel/git/zwisler/linux.git/log/?h=dax_pmd_v9

The only patch that is different between v8 and v9 is:
[PATCH v9 14/16] dax: add struct iomap based DAX PMD support

> > Previously we had talked about this series going through the XFS tree, but
> > Jan has a patch set that will need to build on this series and it heavily
> > modifies the MM code.  I think he would prefer that series to go through
> > Andrew Morton's -MM tree, so it probably makes sense for this series to go
> > through that same tree.
> 
> Seriously, I was 10 minutes away from pushing out the previous
> version of this patchset as a stable topic branch, just like has
> been discussed and several times over the past week.  Indeed, I
> mentioned that I was planning on pushing out this topic branch today
> not more than 4 hours ago, and you were on the cc list.

I'm confused - I sent v9 of this series out 2 days ago, on Tuesday?
I have seen multiple messages from you this week saying you were going to pick
this series up, but I saw them all after I had already sent this series out.

> The -mm tree is not the place to merge patchsets with dependencies
> like this because it's an unstable, rebasing tree. Hence it cannot
> be shared and used as the base of common development between
> multiple git trees like we have for the fs/ subsystem.
> 
> This needs to go out as a stable topic branch so that other
> dependent work can reliably build on top of it for the next merge
> window. e.g. the ext4 DAX iomap patch series that is likely to be
> merged through the ext4 tree, so it needs a stable branch. There's
> iomap direct IO patches for XFS pending, and they conflict with this
> patchset. i.e. we need a stable git base to work from...

Yea, my apologies.  Really this comes down to a lack of understanding on my
part about about which series should be merged via which maintainers, and how
stable topic branches can be shared.  I didn't realize that if you make a
stable branch that could be easily used by other trees, and that for example
Jan's MM or ext4 based patches could be merged by another maintainer but be
based on your topic branch.

Sorry for the confusion, I was just trying to figure out a way that Jan's
changes could also be merged.  Please do pick up v9 of my PMD set. :)

[toc] | [prev] | [next] | [standalone]

#1514844

On Thu, Nov 03, 2016 at 11:51:02AM -0600, Ross Zwisler wrote:
> On Thu, Nov 03, 2016 at 12:58:26PM +1100, Dave Chinner wrote:
> > On Tue, Nov 01, 2016 at 01:54:02PM -0600, Ross Zwisler wrote:
> > > DAX PMDs have been disabled since Jan Kara introduced DAX radix tree based
> > > locking.  This series allows DAX PMDs to participate in the DAX radix tree
> > > based locking scheme so that they can be re-enabled.
> > 
> > I've seen patch 0/16 - where did you send the other 16? I need to
> > pick up the bug fix that is in this patch set...
> 
> I CC'd your "david@fromorbit.com" address on the entire set, as well as all
> the usual lists (linux-xfs, linux-fsdevel, linux-nvdimm, etc).

Ok, now I'm /really/ confused. Procmail logs show:

From ross.zwisler@linux.intel.com Wed Nov 02 06:56:46 2016
Subject: [PATCH v9 00/16] re-enable DAX PMD support
Folder: incoming/xfs-linux/new/1478030206.9177_1.dastard 5348
From ross.zwisler@linux.intel.com Wed Nov 02 06:56:48 2016
Subject: [PATCH v9 01/16] ext4: tell DAX the size of allocation holes
Folder: incoming/xfs-linux/new/1478030208.9182_1.dastard 3725
From ross.zwisler@linux.intel.com Wed Nov 02 06:56:49 2016
Subject: [PATCH v9 02/16] dax: remove buffer_size_valid()
Folder: incoming/xfs-linux/new/1478030209.9187_1.dastard 4692
.....

so procmail has seen them, and put them all in the same bucket like
it has for everything else.

But only patch 0 appeared in my linux-xfs mail box - the rest of the
files logged by procmail don't exist. No errors or indications of
failures anywhere. They've just vanished into thin air...

> They are also available via the libnvdimm patchwork:
> 
> https://patchwork.kernel.org/project/linux-nvdimm/list/
> 
> or via my tree:
> 
> https://git.kernel.org/cgit/linux/kernel/git/zwisler/linux.git/log/?h=dax_pmd_v9
> 
> The only patch that is different between v8 and v9 is:
> [PATCH v9 14/16] dax: add struct iomap based DAX PMD support

OK, thanks, I'll pull it in.

> 
> > > Previously we had talked about this series going through the XFS tree, but
> > > Jan has a patch set that will need to build on this series and it heavily
> > > modifies the MM code.  I think he would prefer that series to go through
> > > Andrew Morton's -MM tree, so it probably makes sense for this series to go
> > > through that same tree.
> > 
> > Seriously, I was 10 minutes away from pushing out the previous
> > version of this patchset as a stable topic branch, just like has
> > been discussed and several times over the past week.  Indeed, I
> > mentioned that I was planning on pushing out this topic branch today
> > not more than 4 hours ago, and you were on the cc list.
> 
> I'm confused - I sent v9 of this series out 2 days ago, on Tuesday?
> I have seen multiple messages from you this week saying you were going to pick
> this series up, but I saw them all after I had already sent this series out.

That's what I'm really confused - I replied immediately after this
email appeared in my in-box - I was working from v8 because I didn't
know this version existed. This v9 patch zero email hit procmail on
"Wed Nov 02 06:56:46 2016" and i replied immediately when i saw it:
"On Thu, Nov 03, 2016 at 12:58:26PM +1100,"

So there's some 30 hours between it passing through procmail and
mutt adding it to my inbox. And mutt hasn't see any of the other
emails in the thread.

/me sighs and wonders how much other email has been going missing
lately....

> Sorry for the confusion,

Clearly not your fault, Ross.

> I was just trying to figure out a way that Jan's
> changes could also be merged.  Please do pick up v9 of my PMD set. :)

Will do, but I've got to find my way out of WTF-Landia first...

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web