Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1624896

Re: [PATCH v2 07/17] fs: new infrastructure for writeback error handling and reporting

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On Wed, Apr 12 2017, Jeff Layton wrote:

> On Thu, 2017-04-13 at 07:55 +1000, NeilBrown wrote:
>> 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).
>> 
>> 
>
> I think it'd be much simpler to adapt NFS over to use the new
> infrastructure (I have a draft patch for that already). You'd lose the
> ability to track a different error for each nfs_open_context, but I'm
> not sure how valuable that is anyway. I'll need to think about that
> one...

From a technical perspective, it might be "simpler" but I contest "much
simpler".   I think it would be easy to put one wb_err_t per
nfs_open_context, if the former were designed well (which itself would
be easy).

From a political perspective, I doubt it would be simple.  NFS is the
way it is for a reason, and convincing an author that their reason is
not valid tends to be harder than most technical issues.
(looking to history...
the 'error' field was added to the nfs_open_context in
Commit: 6caf69feb23a ("NFSv2/v3/v4: Place NFS nfs_page shared data into a single structure    that hangs off filp->private_data. As a side effect, this also    cleans up the NFSv4 private file state info.")

in 2.6.12.  Prior to that file->f_error was used.
Prior to commit 9ffb8c3a1955 ("Import 2.2.3pre1") (which has no comment)
errors were ... interesting.  Look for nfs_check_error in
commit d9c0ffee4db7 ("Import 2.1.128") and notice the use of current->pid!!
All commits from the history.git tree.
)

It is quite possible for an NFS server to return different errors to
different users.  It might be odd, but it is possible.  Should an error
that affects one user pollute all other users?

Thanks,
NeilBrown

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH v2 00/17] fs: introduce new writeback error reporting and convert existing API as a wrapper around it Jeff Layton <jlayton@redhat.com> - 2017-04-12 14:20 +0200
  [PATCH v2 03/17] buffer: use mapping_set_error instead of setting the flag Jeff Layton <jlayton@redhat.com> - 2017-04-12 14:20 +0200
    Re: [PATCH v2 03/17] buffer: use mapping_set_error instead of  setting the flag Matthew Wilcox <willy@infradead.org> - 2017-04-12 16:30 +0200
  [PATCH v2 07/17] fs: new infrastructure for writeback error handling and reporting Jeff Layton <jlayton@redhat.com> - 2017-04-12 14:20 +0200
    Re: [PATCH v2 07/17] fs: new infrastructure for writeback error  handling and reporting Jeff Layton <jlayton@redhat.com> - 2017-04-12 20:50 +0200
      Re: [PATCH v2 07/17] fs: new infrastructure for writeback error handling and reporting NeilBrown <neilb@suse.com> - 2017-04-13 00:00 +0200
        Re: [PATCH v2 07/17] fs: new infrastructure for writeback error  handling and reporting Jeff Layton <jlayton@redhat.com> - 2017-04-13 01:10 +0200
          Re: [PATCH v2 07/17] fs: new infrastructure for writeback error handling and reporting NeilBrown <neilb@suse.com> - 2017-04-18 01:00 +0200

csiph-web