Messages in this thread | | | From | NeilBrown <> | Date | Thu, 13 Apr 2017 07:55:32 +1000 | Subject | Re: [PATCH v2 07/17] fs: new infrastructure for writeback error handling and reporting |
| |
On Wed, Apr 12 2017, Jeff Layton wrote:
> +void __filemap_set_wb_error(struct address_space *mapping, int err)
I was really hoping that this would be
void __set_wb_error(wb_err_t *wb_err, int err)
so
Then nfs_context_set_write_error could become
static void nfs_context_set_write_error(struct nfs_open_context *ctx, int error) { __set_wb_error(&ctx->wb_err, error); }
and filemap_set_sb_error() would be:
static inline void filemap_set_wb_error(struct address_space *mapping, int err) { /* Optimize for the common case of no error */ if (unlikely(err)) __set_wb_error(&mapping->f_wb_err, err); }
Similarly we would have wb_err_t sample_wb_error(wb_err_t *wb_err) { ... }
and
wb_err_t filemap_sample_wb_error(struct address_space *mapping) { return sample_wb_error(&mapping->f_wb_err); }
so nfs_file_fsync_commit() could have ret = sample_wb_error(&ctx->wb_err); in place of ret = xchg(&ctx->error, 0);
int filemap_report_wb_error(struct file *file)
would become
int filemap_report_wb_error(struct file *file, wb_err_t *err)
or something.
The address space is just one (obvious) place where the wb error can be stored. The filesystem might have a different place with finer granularity (nfs already does).
> +wb_err_t filemap_sample_wb_error(struct address_space *mapping) > +{ > + wb_err_t old = READ_ONCE(mapping->wb_err); > + wb_err_t new = old; > + > + /* > + * For the common case of no errors ever having been set, we can skip > + * marking the SEEN bit. Once an error has been set, the value will > + * never go back to zero. > + */ > + if (old != 0) { > + new |= WB_ERR_SEEN; > + if (old != new) > + cmpxchg(&mapping->wb_err, old, new); > + } > + return new; > +}
I do like how the use of cmpxchg work out here - no looping!
Thanks NeilBrown [unhandled content-type:application/pgp-signature] | |