[PATCH 0/2] record errors in mapping when writeback fails on DAX

[PATCH 0/2] record errors in mapping when writeback fails on DAX

[Jeff Layton]

This is part of the preparatory set of patches to pave the way for
improved writeback error reporting. In order to do this correctly, we
need to ensure that DAX marks the mapping with an error when writeback
fails.

I sent the second patch in this series to Ross last week, but he pointed
out that it makes fsync error out more than it should, since we don't
currently clear errors in filemap_write_and_wait and
filemap_write_and_wait_range.

In order to fix that, I think we need the first patch in this set. There
is a some danger that this could end up causing error flags to be
cleared earlier than they were before when write initiation fails in
other filesystems.

Given how racy all of the AS_* flag handling is though, I'm inclined to
just go ahead and merge both of these into linux-next and deal with any
fallout as it arises.

Does that seem like a reasonable plan? If so, Andrew, would you be
willing to take both of these in for linux-next, with an eye toward
merging into v4.13?

Thanks in advance,

Jeff Layton (2):
  mm: clear any AS_* errors when returning from
    filemap_write_and_wait{_range}
  dax: set errors in mapping when writeback fails

 fs/dax.c     | 4 +++-
 mm/filemap.c | 8 ++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

-- 
2.9.4

--
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>

[PATCH 1/2] mm: clear any AS_* errors when returning from filemap_write_and_wait{_range}

[Jeff Layton]

Currently we don't clear the address space error when there is a -EIO
error on fsync due to writeback initiation failure. If initiating writes
fails with -EIO and the mapping is already flagged with an AS_EIO or
AS_ENOSPC error, then we can end up returning errors on two fsync calls,
even when a write between them succeeded (or there was no write).

Ensure that we also clear out any mapping errors when initiating
writeback fails with -EIO in filemap_write_and_wait and
filemap_write_and_wait_range.

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 mm/filemap.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 6f1be573a5e6..39ff92d7ecdd 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -488,7 +488,7 @@ EXPORT_SYMBOL(filemap_fdatawait);
 
 int filemap_write_and_wait(struct address_space *mapping)
 {
-	int err = 0;
+	int err;
 
 	if ((!dax_mapping(mapping) && mapping->nrpages) ||
 	    (dax_mapping(mapping) && mapping->nrexceptional)) {
@@ -503,6 +503,8 @@ int filemap_write_and_wait(struct address_space *mapping)
 			int err2 = filemap_fdatawait(mapping);
 			if (!err)
 				err = err2;
+		} else {
+			filemap_check_errors(mapping);
 		}
 	} else {
 		err = filemap_check_errors(mapping);
@@ -525,7 +527,7 @@ EXPORT_SYMBOL(filemap_write_and_wait);
 int filemap_write_and_wait_range(struct address_space *mapping,
 				 loff_t lstart, loff_t lend)
 {
-	int err = 0;
+	int err;
 
 	if ((!dax_mapping(mapping) && mapping->nrpages) ||
 	    (dax_mapping(mapping) && mapping->nrexceptional)) {
@@ -537,6 +539,8 @@ int filemap_write_and_wait_range(struct address_space *mapping,
 						lstart, lend);
 			if (!err)
 				err = err2;
+		} else {
+			filemap_check_errors(mapping);
 		}
 	} else {
 		err = filemap_check_errors(mapping);
-- 
2.9.4

--
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>

[PATCH 2/2] dax: set errors in mapping when writeback fails

[Jeff Layton]

Jan's description for this patch is much better than mine, so I'm
quoting it verbatim here:

DAX currently doesn't set errors in the mapping when cache flushing
fails in dax_writeback_mapping_range(). Since this function can get
called only from fsync(2) or sync(2), this is actually as good as it can
currently get since we correctly propagate the error up from
dax_writeback_mapping_range() to filemap_fdatawrite(). However in the
future better writeback error handling will enable us to properly report
these errors on fsync(2) even if there are multiple file descriptors
open against the file or if sync(2) gets called before fsync(2). So
convert DAX to using standard error reporting through the mapping.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-and-Tested-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 fs/dax.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/dax.c b/fs/dax.c
index c22eaf162f95..441280e15d5b 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -856,8 +856,10 @@ int dax_writeback_mapping_range(struct address_space *mapping,
 
 			ret = dax_writeback_one(bdev, dax_dev, mapping,
 					indices[i], pvec.pages[i]);
-			if (ret < 0)
+			if (ret < 0) {
+				mapping_set_error(mapping, ret);
 				goto out;
+			}
 		}
 	}
 out:
-- 
2.9.4

--
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>

Re: [PATCH 0/2] record errors in mapping when writeback fails on DAX

[Jeff Layton]

On Tue, 2017-05-30 at 07:10 -0400, Jeff Layton wrote:
> This is part of the preparatory set of patches to pave the way for
> improved writeback error reporting. In order to do this correctly, we
> need to ensure that DAX marks the mapping with an error when writeback
> fails.
> 
> I sent the second patch in this series to Ross last week, but he pointed
> out that it makes fsync error out more than it should, since we don't
> currently clear errors in filemap_write_and_wait and
> filemap_write_and_wait_range.
> 
> In order to fix that, I think we need the first patch in this set. There
> is a some danger that this could end up causing error flags to be
> cleared earlier than they were before when write initiation fails in
> other filesystems.
> 
> Given how racy all of the AS_* flag handling is though, I'm inclined to
> just go ahead and merge both of these into linux-next and deal with any
> fallout as it arises.
> 
> Does that seem like a reasonable plan? If so, Andrew, would you be
> willing to take both of these in for linux-next, with an eye toward
> merging into v4.13?
> 
> Thanks in advance,
> 

There is an alternative here though...

In the series that I have that adds in the new writeback error reporting
infrastructure, I've added a fstype flag that indicates what flavor of
error reporting the filesystem does.

We could just have DAX check that flag and only mark the mapping for
error if it's set. That should make things work with the new scheme
while preserving the old as much as possible.

Now that I think about it, that's probably the safest avenue. Let's just
drop both of these patches, and I'll just roll a patch like that into
the later series.

Sorry for the noise,
-- 
Jeff Layton <jlayton@redhat.com>

--
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>