[v2,1/2] dax: dax_layout_busy_page() warn on !exceptional

Message ID 20180627212252.31032-2-ross.zwisler@linux.intel.com
State New
Headers show
Series
  • ext4: fix DAX dma vs truncate/hole-punch
Related show

Commit Message

Ross Zwisler June 27, 2018, 9:22 p.m.
Inodes using DAX should only ever have exceptional entries in their page
caches.  Make this clear by warning if the iteration in
dax_layout_busy_page() ever sees a non-exceptional entry, and by adding a
comment for the pagevec_release() call which only deals with struct page
pointers.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/dax.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Theodore Y. Ts'o July 2, 2018, 10:15 p.m. | #1
On Wed, Jun 27, 2018 at 03:22:51PM -0600, Ross Zwisler wrote:
> Inodes using DAX should only ever have exceptional entries in their page
> caches.  Make this clear by warning if the iteration in
> dax_layout_busy_page() ever sees a non-exceptional entry, and by adding a
> comment for the pagevec_release() call which only deals with struct page
> pointers.
> 
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks, applied (to the ext4 tree).  If someone thinks they should go
in via some other tree, holler.

					- Ted
Ross Zwisler July 3, 2018, 3:41 p.m. | #2
On Mon, Jul 02, 2018 at 06:15:03PM -0400, Theodore Y. Ts'o wrote:
> On Wed, Jun 27, 2018 at 03:22:51PM -0600, Ross Zwisler wrote:
> > Inodes using DAX should only ever have exceptional entries in their page
> > caches.  Make this clear by warning if the iteration in
> > dax_layout_busy_page() ever sees a non-exceptional entry, and by adding a
> > comment for the pagevec_release() call which only deals with struct page
> > pointers.
> > 
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > Reviewed-by: Jan Kara <jack@suse.cz>
> 
> Thanks, applied (to the ext4 tree).  If someone thinks they should go
> in via some other tree, holler.
> 
> 					- Ted

Hey Ted,

It looks like you only picked up patch 1/2?  (I'm looking at the 'dev' branch
in your repo.)  Was that intentional?

You can find the final version of the 2nd patch here:

https://lists.01.org/pipermail/linux-nvdimm/2018-July/016602.html

Thanks,
- Ross
Theodore Y. Ts'o July 3, 2018, 5:44 p.m. | #3
On Tue, Jul 03, 2018 at 09:41:37AM -0600, Ross Zwisler wrote:
> 
> It looks like you only picked up patch 1/2?  (I'm looking at the 'dev' branch
> in your repo.)  Was that intentional?

Actually, it was a mistake, in that if you looked at the commit, it's
currently an empty commit.  The patch failed to apply because the ext4
tree is still based on v4.17-rc4.

My current plan is to hold the two patches until I get the current
patch of fixes pushed to Linus (probably in the next day or two; I'll
drop the empty commit before I send a pull request to reduce
confusion).  I'll then reset the ext4 tree to be based on v4.17 (or
possibly v4.18-rcX if that is necessary) and then apply the two
patches in this series.

Apologies for the confusion....

						- Ted

Patch

diff --git a/fs/dax.c b/fs/dax.c
index 641192808bb6..897b51e41d8f 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -566,7 +566,8 @@  struct page *dax_layout_busy_page(struct address_space *mapping)
 			if (index >= end)
 				break;
 
-			if (!radix_tree_exceptional_entry(pvec_ent))
+			if (WARN_ON_ONCE(
+			     !radix_tree_exceptional_entry(pvec_ent)))
 				continue;
 
 			xa_lock_irq(&mapping->i_pages);
@@ -578,6 +579,13 @@  struct page *dax_layout_busy_page(struct address_space *mapping)
 			if (page)
 				break;
 		}
+
+		/*
+		 * We don't expect normal struct page entries to exist in our
+		 * tree, but we keep these pagevec calls so that this code is
+		 * consistent with the common pattern for handling pagevecs
+		 * throughout the kernel.
+		 */
 		pagevec_remove_exceptionals(&pvec);
 		pagevec_release(&pvec);
 		index++;