On Sun, Mar 05 2017, Jeff Layton wrote: > I recently did some work to wire up -ENOSPC handling in ceph, and found > I could get back -EIO errors in some cases when I should have instead > gotten -ENOSPC. The problem was that the ceph writeback code would set > PG_error on a writeback error, and that error would clobber the mapping > error. > > While I fixed that problem by simply not setting that bit on errors, > that led me down a rabbit hole of looking at how PG_error is being > handled in the kernel.
Speaking of rabbit holes... I thought to wonder how IO error propagate up from NFS. It doesn't use SetPageError or mapping_set_error() for files (except in one case that looks a bit odd). It has an "nfs_open_context" and store the latest error in ctx->error. So when you get around to documenting how this is supposed to work, it would be worth while describing the required observable behaviour, and note that while filesystems can use mapping_set_error() to achieve this, they don't have to. I notice that drivers/staging/lustre/lustre/llite/rw.c fs/afs/write.c fs/btrfs/extent_io.c fs/cifs/file.c fs/jffs2/file.c fs/jfs/jfs_metapage.c fs/ntfs/aops.c (and possible others) all have SetPageError() calls that seem to be in response to a write error to a file, but don't appear to have matching mapping_set_error() calls. Did you look at these? Did I miss something? Thanks, NeilBrown > > This patch series is a few fixes for things that I 100% noticed by > inspection. I don't have a great way to test these since they involve > error handling. I can certainly doctor up a kernel to inject errors > in this code and test by hand however if these look plausible up front. > > Jeff Layton (3): > nilfs2: set the mapping error when calling SetPageError on writeback > mm: don't TestClearPageError in __filemap_fdatawait_range > mm: set mapping error when launder_pages fails > > fs/nilfs2/segment.c | 1 + > mm/filemap.c | 19 ++++--------------- > mm/truncate.c | 6 +++++- > 3 files changed, 10 insertions(+), 16 deletions(-) > > -- > 2.9.3
signature.asc
Description: PGP signature
