lkml.org 
[lkml]   [2017]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [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]
    \
     
     \ /
      Last update: 2017-04-12 23:56    [W:4.884 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site