On Thu, Aug 24, 2017 at 10:16:37AM +1000, Dave Chinner wrote: > On Wed, Aug 23, 2017 at 05:26:36PM +0800, Hou Tao wrote: > > Hi Dave, > > > > On 2017/8/22 6:50, Dave Chinner wrote: > > > On Mon, Aug 21, 2017 at 07:54:19PM +0800, Hou Tao wrote: > > >> Hi all, > > >> > > >> XFS has the configurable error handlers for each mounted device, but > > >> the default values of these configuration are static. It will be useful > > >> to make the default values customizable when there are many XFS filesystems > > >> and we need to shutdown the filesystem after getting any error. > > > > > > Nice! I can see the usefulness of this functionality - it's a piece > > > of the puzzle we are missing. I've had a quick look over the code, > > > and have a few high level questions that I can't answer from looking > > > at the code. > > > Was there any reason you decided to put the default policy > > > management into the kernel rather than try to provide a mechanism to > > > allow userspace to manage it (e.g. via a udev event at mount time)? > > No special reason for the kernel-side default policy, just because I didn't > > find an uevent for the mount event. If the uevent is available or added (?), > > writing an udev rule to set the default error configuration seems simpler and > > works for our scenario. > > I think we'd need to add one, but given we already have a kobj for > the filesystem being mounted, it seems to me like we should be able > to add a mount notification without too much trouble via > kobject_uevent(KOBJ_ONLINE) <ENGAGE BIKESHED MODE> It might be useful to think more broadly about modifying XFS to send uevents. In addition to sending KOBJ_ONLINE to broadcast the mount to error configuration tools, we could (in the future say) schedule online fscks with a service manager. Or we could send KOBJ_CHANGE notices when we hit IO errors, or someone remounts the fs with different options? <END BIKESHED MODE> > > > We've still got to manage per filesystem specific settings from > > > userspace (e.g. root volume might be different to data volumes), so > > > I'm interested to know if you have any ideas on how we can handle > > > that side of the problem, too? > > Do you mean providing extra tools or files to let userspace to manage > > fs specific error settings or something else ? For us, the current sysfs > > files of error configuration are enough and there is no need for anything more. > > I was thinking we should provide a generic userspace tool to catch > the XFS uevents. On a mount notification it can grab the error > config from a config file (e.g. /etc/xfs/error.conf) and push the > config specified in that file into the sysfs error config files of > the newly mounted filesystem.... <nod> > > > Assuming we are going to put default policy into the kernel, this > > > notifier structure seems overly complex when compared to a single > > > structure that holds the default values and looking it up when a > > > specific mount error config holds a flag that says "use the > > > default". > > > The "check the default flag and grab values" code could all be > > > hidden inside xfs_error_get_cfg() so propagation happens on demand. > > > This means all the mounts pick up changes to the default config > > > without needing to be chained to a notifier. > > > > > > The "use defaults" flag could be cleared when a new value is set for > > > that error config, and potentionally we could reset it in the store > > > method if it matches the current default value. > > > > > > I might be missing something here (I frequently do, so please tell > > > me if I have!), but my initial thoughts are that we can do this in a > > > much simpler manner. What do you think, Tao? > > > > Yes, I agree that the notifier method is over-weighted, and the proposed method > > is much better. However it has no way to reset the "use default flag" of a specific > > configuration when the modified default value is modified again to a new value when > > the old default value is equal with the current specific value. Maybe the reset is > > unnecessary ? > > In that case I think the reset is unnecessary. The fact the default > was changed to match a specific filesystem config doesn't mean that > filesystem should start using the defaults. i.e. if you want to > reset the filesystem to default config, then you need to rewrite > it's config to match the current defaults. > > It's a bit clunky, but it's a simple rule that's easy to understand > and it keeps the API and both the kernel and userspace code really > simple. Or we just program the knobs to accept 'default', whatever the kernel thinks that is? --D > > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
