public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Carlos Maiolino <cmaiolino@redhat.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 0/7] Configurable error behavior [V2]
Date: Wed, 4 May 2016 08:59:48 +1000	[thread overview]
Message-ID: <20160503225948.GU26977@dastard> (raw)
In-Reply-To: <1462298140-12411-1-git-send-email-cmaiolino@redhat.com>

On Tue, May 03, 2016 at 07:55:33PM +0200, Carlos Maiolino wrote:
> Hi folks,
> 
> I spoke with Dave and to offload him a bit, I took over his patchset for enable
> configurable error handlers.
> 
> Since he did most of the design, I kept his signed-off to the patches.
> 
> Here are core the changes I did from his last patchset to this one:
> 
>  - Removed fail_speed configuration
> 	According with what has been discussed, there is no real need to have a
> fail_speed configuration, but instead, use the "max_retries" configuration to
> get for how long the filesystem should retrying, with the only special case for
> 	"-1", which will make the filesystem to retry forever.
> 
>  - Fail at unmount is no longer a config option
> 	Having a filesystem stuck forever during unmount due errors is not a
> 	good thing, so just enforce it if we are trying to unmount a failed
> 	filesystem.

I think this is wrong - the option should still remain to let the
filesystem retry for a long while if desired. We have lazy unmounts
to deal with this situation (i.e. it won't block an unmount command)
and there are cases where leaving the unmount retrying in the
background is useful.

If you are particularly worried about not being able to shut down a
filesystem that has been unmounted lazily and hence terminate the
retry forever loop, then we should be looking at ensuring the fs is
still visible in /sys/fs/xfs/<dev> when it is in this state and
providing a new shutdown hook through that interface....

> I reduced by now, the amount of patches into this patchset, once, our priority
> here IMHO is to enable the possibility to shutdown the filesystem when we have
> metadata errors, and I can work on disabling specific error configs and add
> memory errors later, I hope that with a reduced amount of patches it can be
> easier for people to review the core of the error handlers configuration and
> speed up the inclusion of this patchset.

We'll see - the issue here is that once we settle on a sysfs
interface, we can't change it easily (it's part of the user facing
ABI). Hence if we don't consider all the different types of errors
we want to handle from the start, we may miss something that we
can't easily fix in future. Hence we at least have to consider the
different constraints for the different error types now to determine
if the abstractions and presentation will handle everything we think
we might need...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  parent reply	other threads:[~2016-05-03 22:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-03 17:55 [PATCH 0/7] Configurable error behavior [V2] Carlos Maiolino
2016-05-03 17:55 ` [PATCH 1/7] xfs: configurable error behaviour via sysfs Carlos Maiolino
2016-05-03 17:55 ` [PATCH 2/7] xfs: introduce metadata IO error class Carlos Maiolino
2016-05-03 17:55 ` [PATCH 3/7] xfs: add configurable error support to metadata buffers Carlos Maiolino
2016-05-03 17:55 ` [PATCH 4/7] xfs: introduce table-based init for error behaviours Carlos Maiolino
2016-05-03 17:55 ` [PATCH 5/7] xfs: add configuration of error failure speed Carlos Maiolino
2016-05-03 23:24   ` Dave Chinner
2016-05-04  9:57     ` Carlos Maiolino
2016-05-03 17:55 ` [PATCH 6/7] xfs: add configuration handles for specific errors Carlos Maiolino
2016-05-03 17:55 ` [PATCH 7/7] xfs: shutdown filesystem at unmount in case of errors Carlos Maiolino
2016-05-03 22:59 ` Dave Chinner [this message]
2016-05-04  9:53   ` [PATCH 0/7] Configurable error behavior [V2] Carlos Maiolino

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160503225948.GU26977@dastard \
    --to=david@fromorbit.com \
    --cc=cmaiolino@redhat.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox