xfs: configurable error behaviour
cover xfs: configurable error behaviour
Hi folks,
I need to restart the discussion and review of this patch series. There was some discussion of it last time, but nothing really came from that. I'm posting what I have in my tree right now - treat it as though it's an initial posting of the code because I can't recall what I've changed since the first posting.
What I'd like to have to for the next merge window is all the IO error bits sorted out. The final patch (kmem failure behaviour) needs more infrastructure (passing mp to every allocation) so that's a secondary concern right now and I've only included it to demonstrate how to apply this code ot a different subsystem.
Things that need to be nailed down before I can commit the series:
- sysfs layout
- naming conventions for errors and subsystems in sysfs
- how best to display/change default behaviour
Things that we can change/implement later:
- default behaviour
- additional error classes
- additional error types
- additional subsystems
- subsystem error handling implementation
- communication with other subsystems to dynamically change error behaviour
IOWs, what is important right now is how we present this to userspace, because we can't change that easily once we've decided on a presentation structure.
Modifying the code to classify and handle all the different error types is much less important, as we can change that to fix whatever problems we have without impacting the presentation to userspace.
There is definite need for this (e.g. handling of ENOSPC on thin provisioned devices), so I want to get quickly to a consensus on the userspace facing aspects so that we can get this ball rolling.
The biggest unsolved issue is how to change the default behaviour persistently. There is no infrastructure in this patch series to do that, but it is someting that we have to consider so that we don't require default behaviour to be changed after every mount of every filesystem on a system. My thoughts on this is we store changes to the defaults in xattrs on the root inode, but I'm open to ideas here as there's no code written for it yet. Solving this problem, however, is not necessary before commiting the initial code; it's something we can add later once we've worked out all the details.
Discuss!
-Dave.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 1/9 xfs: configurable error behaviour via sysfs
From: Dave Chinner <[email protected]>
We need to be able to change the way XFS behaviours in error conditions depending on the type of underlying storage. This is necessary for handling non-traditional block devices with extended error cases, such as thin provisioned devices that can return ENOSPC as an IO error.
Introduce the basic sysfs infrastructure needed to define and configure error behaviours. This is done to be generic enough to extend to configuring behaviour in other error conditions, such as ENOMEM, which also has different desired behaviours according to machine configuration.
--- fs/xfs/xfs_mount.c | 10 +++++++++- fs/xfs/xfs_mount.h | 27 ++++++++++++++++++++++++++ fs/xfs/xfs_sysfs.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++- fs/xfs/xfs_sysfs.h | 3 +++ 4 files changed, 94 insertions(+), 2 deletions(-)
fs/xfs/xfs_mount.c +9 -1
@@ -708,10 +708,15 @@ xfs_mountfs(if (error) goto out_remove_sysfs; - error =xfs_uuid_mount (mp); + error =xfs_error_sysfs_init (mp);if (error) goto out_del_stats; + + error =xfs_uuid_mount (mp); +if (error) + goto out_remove_error_sysfs; +/* * Set the minimum read and write sizes */
@@ -986,6 +991,8 @@ xfs_mountfs(xfs_da_unmount (mp); out_remove_uuid:xfs_uuid_unmount (mp); + out_remove_error_sysfs: +xfs_error_sysfs_del (mp); out_del_stats:xfs_sysfs_del (&mp->m_stats.xs_kobj); out_remove_sysfs:
@@ -1074,6 +1081,7 @@ xfs_unmountfs(#endif xfs_free_perag (mp); +xfs_error_sysfs_del (mp);xfs_sysfs_del (&mp->m_stats.xs_kobj);xfs_sysfs_del (&mp->m_kobj); }
fs/xfs/xfs_mount.h +27 -0
@@ -37,6 +37,31 @@ enum { XFS_LOWSP_MAX, }; +/* +* Error Configuration +* +* Error classes define the subsystem the configuration belongs to. +* Error numbers define the errors that are configurable. +* Failure types describe when an error is considered fatal. +*/ +enum { +XFS_ERR_CLASS_MAX , +}; +enum { +XFS_ERR_ERRNO_MAX , +}; +enum { +XFS_ERR_FAIL_DEFAULT , +XFS_ERR_FAIL_NEVER , +XFS_ERR_FAIL_SLOW , +XFS_ERR_FAIL_FAST , +}; + +struct xfs_error_cfg { +struct xfs_kobj kobj; +int fail_speed; +}; + typedef struct xfs_mount { struct super_block *m_super; xfs_tid_t m_tid; /* next unused tid for fs */
@@ -127,6 +152,8 @@ typedef struct xfs_mount {int64_t m_low_space[XFS_LOWSP_MAX ];/* low free space thresholds */ struct xfs_kobj m_kobj; +struct xfs_kobj m_error_kobj; +struct xfs_error_cfg m_error_cfg[XFS_ERR_CLASS_MAX ][XFS_ERR_ERRNO_MAX ];struct xstats m_stats;/* per-fs stats */ struct workqueue_struct *m_buf_workqueue;
fs/xfs/xfs_sysfs.c +55 -1
69 lines not on this page · raw message
fs/xfs/xfs_sysfs.h +3 -0
@@ -58,4 +58,7 @@ xfs_sysfs_del( wait_for_completion(&kobj->complete); } +int xfs_error_sysfs_init (struct xfs_mount *mp); +void xfs_error_sysfs_del (struct xfs_mount *mp); +#endif /* __XFS_SYSFS_H__ */
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH 2/9 xfs: introduce metadata IO error class
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
Now we have the basic infrastructure, add the first error class so we can build up the infrastructure in a meaningful way. Add the metadata async write IO error class and sysfs entry, and introduce a default configuration that matches the existing behaviour of XFS_ERR_FAIL_NEVER for async write metadata buffers.
--- fs/xfs/xfs_mount.h | 3 +++ fs/xfs/xfs_sysfs.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+)
fs/xfs/xfs_mount.h +3 -0
@@ -45,9 +45,11 @@ enum {* Failure types describe when an error is considered fatal. */ enum { +XFS_ERR_METADATA ,XFS_ERR_CLASS_MAX , };enum { +XFS_ERR_DEFAULT ,XFS_ERR_ERRNO_MAX , };enum {
@@ -153,6 +155,7 @@ typedef struct xfs_mount {/* low free space thresholds */ struct xfs_kobj m_kobj;struct xfs_kobj m_error_kobj; +struct xfs_kobj m_error_meta_kobj;struct xfs_error_cfg m_error_cfg[XFS_ERR_CLASS_MAX ][XFS_ERR_ERRNO_MAX ];struct xstats m_stats;/* per-fs stats */
fs/xfs/xfs_sysfs.c +34 -0
@@ -344,11 +344,34 @@ intxfs_error_sysfs_init ( struct xfs_mount *mp) { +struct xfs_error_cfg *cfg;int error;/* .../xfs/<dev>/error/ */ error =xfs_sysfs_init (&mp->m_error_kobj, &xfs_error_ktype, &mp->m_kobj,"error" ); +if (error) +return error; + +/* .../xfs/<dev>/error/metadata/ */ + error =xfs_sysfs_init (&mp->m_error_meta_kobj, &xfs_error_ktype, + &mp->m_error_kobj,"metadata" ); +if (error) + goto out_error; + + cfg = &mp->m_error_cfg[XFS_ERR_METADATA ][XFS_ERR_DEFAULT ]; + error =xfs_sysfs_init (&cfg->kobj, &xfs_error_cfg_ktype, + &mp->m_error_meta_kobj,"Default" ); +if (error) + goto out_error_meta; + cfg->fail_speed =XFS_ERR_FAIL_NEVER ; + +return 0 ; + +out_error_meta: +xfs_sysfs_del (&mp->m_error_meta_kobj); +out_error: +xfs_sysfs_del (&mp->m_error_kobj);return error; }
@@ -356,5 +379,16 @@ voidxfs_error_sysfs_del ( struct xfs_mount *mp) { +struct xfs_error_cfg *cfg; +int i, j; + +for (i =0 ; i <XFS_ERR_CLASS_MAX ; i++) { +for (j =0 ; j <XFS_ERR_ERRNO_MAX ; j++) { + cfg = &mp->m_error_cfg[i][j]; + +xfs_sysfs_del (&cfg->kobj); + } + } +xfs_sysfs_del (&mp->m_error_meta_kobj);xfs_sysfs_del (&mp->m_error_kobj); }
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH 3/9 xfs: add configurable error support to metadata buffers
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
With the error configuration handle for async metadata write errors in place, we can now add initial support to the IO error processing in xfs_buf_iodone_error().
Add an infrastructure function to look up the configuration handle, and rearrange the error handling to prepare the way for different error handling conigurations to be used.
--- fs/xfs/xfs_buf.h | 1 + fs/xfs/xfs_buf_item.c | 112 ++++++++++++++++++++++++++++++-------------------- fs/xfs/xfs_mount.h | 3 ++ fs/xfs/xfs_sysfs.c | 17 ++++++++ 4 files changed, 88 insertions(+), 45 deletions(-)
fs/xfs/xfs_buf.h +1 -0
@@ -183,6 +183,7 @@ typedef struct xfs_buf {unsigned int b_page_count;/* size of page array */ unsigned int b_offset;/* page offset in first page */ int b_error;/* error code on I/O */ +int b_last_error;/* previous async I/O error */ const struct xfs_buf_ops *b_ops;#ifdef XFS_BUF_LOCK_TRACKING
fs/xfs/xfs_buf_item.c +67 -45
147 lines not on this page · raw message
fs/xfs/xfs_mount.h +3 -0
@@ -371,4 +371,7 @@ extern void xfs_set_low_space_thresholds(struct xfs_mount *);int xfs_zero_extent (struct xfs_inode *ip,xfs_fsblock_t start_fsb,xfs_off_t count_fsb); +struct xfs_error_cfg *xfs_error_get_cfg (struct xfs_mount *mp, +int error_class,int error); +#endif /* __XFS_MOUNT_H__ */
fs/xfs/xfs_sysfs.c +17 -0
@@ -392,3 +392,20 @@ xfs_error_sysfs_del(xfs_sysfs_del (&mp->m_error_meta_kobj);xfs_sysfs_del (&mp->m_error_kobj); } + +struct xfs_error_cfg * +xfs_error_get_cfg ( +struct xfs_mount *mp, +int error_class, +int error) +{ +struct xfs_error_cfg *cfg; + +switch (error) { +default : + cfg = &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT ]; +break ; + } + +return cfg; +}
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH 4/9 xfs: introduce table-based init for error behaviours
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
Before we start expanding the number of error classes and errors we can configure behaviour for, we need a simple and clear way to define the default behaviour that we initialised each mount with. Introduce a table based method for keeping the initial configuration in, and apply that to the existing initialisation code.
--- fs/xfs/xfs_sysfs.c | 72 +++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 60 insertions(+), 12 deletions(-)
fs/xfs/xfs_sysfs.c +60 -12
@@ -340,11 +340,67 @@ struct kobj_type xfs_error_ktype = { .release = xfs_sysfs_release, }; +/* +* Error initialisation tables. These need to be ordered in the same +* order as the enums used to index the array. All class init tables need to +* define a "default" behaviour as the first entry, all other entries can be +* empty. +*/ +struct xfs_error_init { +char *name; +int fail_speed; +}; + +static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX ] = { + { .name ="Default" , + .fail_speed =XFS_ERR_FAIL_NEVER , + }, +}; + +static int +xfs_error_sysfs_init_class ( +struct xfs_mount *mp, +int class, +const char *parent_name, +struct xfs_kobj *parent_kobj, +const struct xfs_error_init init[]) +{ +struct xfs_error_cfg *cfg; +int error; +int i; + +ASSERT (class <XFS_ERR_CLASS_MAX ); + + error =xfs_sysfs_init (parent_kobj, &xfs_error_ktype, + &mp->m_error_kobj, parent_name); +if (error) +return error; + +for (i =0 ; i <XFS_ERR_ERRNO_MAX ; i++) { + cfg = &mp->m_error_cfg[class][i]; + error =xfs_sysfs_init (&cfg->kobj, &xfs_error_cfg_ktype, + parent_kobj, init[i].name); +if (error) + goto out_error; + + cfg->fail_speed = init[i].fail_speed; + } +return 0 ; + +out_error: +/* unwind the entries that succeeded */ +for (i--; i >=0 ; i--) { + cfg = &mp->m_error_cfg[class][i]; +xfs_sysfs_del (&cfg->kobj); + } +xfs_sysfs_del (parent_kobj); +return error; +} +int xfs_error_sysfs_init (struct xfs_mount *mp) { -struct xfs_error_cfg *cfg;int error;/* .../xfs/<dev>/error/ */
@@ -354,22 +410,14 @@ xfs_error_sysfs_init(return error;/* .../xfs/<dev>/error/metadata/ */ - error =xfs_sysfs_init (&mp->m_error_meta_kobj, &xfs_error_ktype, - &mp->m_error_kobj,"metadata" ); + error =xfs_error_sysfs_init_class (mp,XFS_ERR_METADATA , +"metadata" , &mp->m_error_meta_kobj, + xfs_error_meta_init);if (error) goto out_error; - cfg = &mp->m_error_cfg[XFS_ERR_METADATA ][XFS_ERR_DEFAULT ]; - error =xfs_sysfs_init (&cfg->kobj, &xfs_error_cfg_ktype, - &mp->m_error_meta_kobj,"Default" ); -if (error) - goto out_error_meta; - cfg->fail_speed =XFS_ERR_FAIL_NEVER ; -return 0 ; -out_error_meta: -xfs_sysfs_del (&mp->m_error_meta_kobj); out_error:xfs_sysfs_del (&mp->m_error_kobj);return error;
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH 5/9 xfs: add configuration of error failure speed
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
On reception of an error, we can fail immediately, perform some bound amount of retries or retry indefinitely. The current behaviour we have is to retry forever.
However, we'd like the ability to choose what behaviour we have, and that requires the ability to configure the behaviour through the new sysfs interfaces. Add configuration options for fail fast, slow or never to reflect the three choices above. Fail fast or fail never don't require any other options, but "fail slow" needs configuration to bound the retry behaviour. Add both a maximum retry count and a retry timeout so that we can bound by time and/or physical IO attempts.
Finally, plumb these into xfs_buf_iodone error processing so that the error behaviour follows the selected configuration.
--- fs/xfs/xfs_buf.h | 23 ++++++++- fs/xfs/xfs_buf_item.c | 22 ++++++++- fs/xfs/xfs_mount.h | 2 + fs/xfs/xfs_sysfs.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 169 insertions(+), 6 deletions(-)
fs/xfs/xfs_buf.h +22 -1
@@ -183,7 +183,28 @@ typedef struct xfs_buf {unsigned int b_page_count;/* size of page array */ unsigned int b_offset;/* page offset in first page */ int b_error;/* error code on I/O */ -int b_last_error;/* previous async I/O error */ + +/* +* async write failure retry count. Initialised to zero on the first +* failure, then when it exceeds the maximum configured without a +* success the write is considered to be failed permanently and the +* iodone handler will take appropriate action. +* +* For retry timeouts, we record the jiffie of the first failure. This +* means that we can change the retry timeout and it on the next error +* it will be checked against the newly configured timeout. This +* prevents buffers getting stuck in retry loops with a really long +* timeout. +* +* last_error is used to ensure that we are getting repeated errors, not +* different errors. e.g. a block device might change ENOSPC to EIO when +* a failure timeout occurs, so we want to re-initialise the error +* retry behaviour appropriately when that happens. +*/ +int b_retries; +unsigned long b_first_retry_time;/* in jiffies */ +int b_last_error; +const struct xfs_buf_ops *b_ops;#ifdef XFS_BUF_LOCK_TRACKING
fs/xfs/xfs_buf_item.c +21 -1
@@ -970,6 +970,9 @@ xfs_buf_iodone_callback_error( bp->b_flags |= (XBF_WRITE |XBF_ASYNC |XBF_DONE |XBF_WRITE_FAIL ); bp->b_last_error = bp->b_error; + bp->b_retries =0 ; + bp->b_first_retry_time = jiffies; +xfs_buf_ioerror (bp,0 );xfs_buf_submit (bp);return true;
@@ -979,9 +982,25 @@ xfs_buf_iodone_callback_error(* Repeated failure on an async write. Take action according to the * error configuration we have been set up to use. */ -if (cfg->fail_speed ==XFS_ERR_FAIL_FAST ) +switch (cfg->fail_speed) { +case XFS_ERR_FAIL_FAST : goto permanent_error; +case XFS_ERR_FAIL_SLOW : +if (++bp->b_retries > cfg->max_retries) + goto permanent_error; +if (!cfg->retry_timeout) +break ; +if (time_after (jiffies, + cfg->retry_timeout + bp->b_first_retry_time)) + goto permanent_error; +break ; + +case XFS_ERR_FAIL_NEVER : +default : +break ; + } +/* still a transient error, higher layers will retry */ xfs_buf_ioerror (bp,0 );xfs_buf_relse (bp);
@@ -1023,6 +1042,7 @@ xfs_buf_iodone_callbacks(* retry state here in preparation for the next error that may occur. */ bp->b_last_error =0 ; + bp->b_retries =0 ;xfs_buf_do_callbacks (bp); bp->b_fspriv =NULL ;
fs/xfs/xfs_mount.h +2 -0
@@ -62,6 +62,8 @@ enum {struct xfs_error_cfg {struct xfs_kobj kobj;int fail_speed; +int max_retries;/* INT_MAX = retry forever */ +unsigned long retry_timeout;/* in jiffies, 0 = no timeout */ }; typedef struct xfs_mount {
fs/xfs/xfs_sysfs.c +124 -4
157 lines not on this page · raw message
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 5/9] xfs: add configuration of error failure speed
Quoted 26 lines · Dave Chinner
From: Dave Chinner <[email protected]>
On reception of an error, we can fail immediately, perform some bound amount of retries or retry indefinitely. The current behaviour we have is to retry forever.
However, we'd like the ability to choose what behaviour we have, and that requires the ability to configure the behaviour through the new sysfs interfaces. Add configuration options for fail fast, slow or never to reflect the three choices above. Fail fast or fail never don't require any other options, but "fail slow" needs configuration to bound the retry behaviour. Add both a maximum retry count and a retry timeout so that we can bound by time and/or physical IO attempts.
Finally, plumb these into
xfs_buf_iodoneerror processing so that the error behaviour follows the selected configuration.Signed-off-by Dave Chinner <[email protected]>--- fs/xfs/xfs_buf.h | 23 ++++++++- fs/xfs/xfs_buf_item.c | 22 ++++++++- fs/xfs/xfs_mount.h | 2 + fs/xfs/xfs_sysfs.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 169 insertions(+), 6 deletions(-)
...
Quoted 4 lines
fs/xfs/xfs_buf_item.c +0
-0
...
Quoted 14 lines
fs/xfs/xfs_buf_item.c +7
-1@@ -979,9 +982,25 @@ xfs_buf_iodone_callback_error(* Repeated failure on an async write. Take action according to the * error configuration we have been set up to use. */ -if (cfg->fail_speed ==XFS_ERR_FAIL_FAST ) +switch (cfg->fail_speed) { +case XFS_ERR_FAIL_FAST : goto permanent_error; +case XFS_ERR_FAIL_SLOW : +if (++bp->b_retries > cfg->max_retries) + goto permanent_error; +if (!cfg->retry_timeout) +break ;
fs/xfs/xfs_buf_item.c xfs_buf_iodone_callback_error( · line 1003
Quoted 10 lines
fs/xfs/xfs_buf_item.c +10
-0+if (time_after (jiffies, + cfg->retry_timeout + bp->b_first_retry_time)) + goto permanent_error; +break ; + +case XFS_ERR_FAIL_NEVER : +default : +break ; + } +
I wonder a bit how granular this system needs to be in terms of user interface, at least right now. For example, fail fast and fail never just seem like variants of fail slow with particular tunables. Fail fast is roughly equivalent to a retry count of one, whereas fail never implies an infinite (e.g., -1) retry count. Do we really need the higher level classification?
Quoted 28 lines
fs/xfs/xfs_buf_item.c +1
-0/* still a transient error, higher layers will retry */ xfs_buf_ioerror (bp,0 );xfs_buf_relse (bp);@@ -1023,6 +1042,7 @@ xfs_buf_iodone_callbacks(* retry state here in preparation for the next error that may occur. */ bp->b_last_error =0 ; + bp->b_retries =0 ;xfs_buf_do_callbacks (bp); bp->b_fspriv =NULL ;fs/xfs/xfs_mount.h +2
-0@@ -62,6 +62,8 @@ enum {struct xfs_error_cfg {struct xfs_kobj kobj;int fail_speed; +int max_retries;/* INT_MAX = retry forever */ +unsigned long retry_timeout;/* in jiffies, 0 = no timeout */ }; typedef struct xfs_mount {fs/xfs/xfs_sysfs.c +0
-0
...
Quoted 1 line
fs/xfs/xfs_sysfs.c +0
-0@@ -330,6 +326,123 @@ to_error_cfg(struct kobject *kobject)
...
fs/xfs/xfs_sysfs.c to_error_cfg(struct kobject *kobject) · line 411
Quoted 8 lines
fs/xfs/xfs_sysfs.c +8
-0+static ssize_t +retry_timeout_seconds_show ( +struct kobject *kobject, +char *buf) +{ +struct xfs_error_cfg *cfg =to_error_cfg (kobject); + + return snprintf(buf, PAGE_SIZE, "%ld\n",
Trailing whitespace here ^
Brian
Quoted 70 lines
fs/xfs/xfs_sysfs.c +41
-0+ jiffies_to_msecs(cfg->retry_timeout) * MSEC_PER_SEC); +} + +static ssize_t +retry_timeout_seconds_store ( +struct kobject *kobject, +const char *buf, +size_t count) +{ +struct xfs_error_cfg *cfg =to_error_cfg (kobject); +int ret; +int val; + + ret =kstrtoint (buf,0 , &val); +if (ret) +return ret; + +/* 1 day timeout maximum */ +if (val <0 || val >86400 ) +return -EINVAL ; + + cfg->retry_timeout =msecs_to_jiffies (val *MSEC_PER_SEC ); +return count; +} +XFS_SYSFS_ATTR_RW (retry_timeout_seconds); + +static struct attribute *xfs_error_attrs[] = { +ATTR_LIST (failure_speed), +ATTR_LIST (max_retries), +ATTR_LIST (retry_timeout_seconds), +NULL , +}; + +struct kobj_type xfs_error_cfg_ktype = { .release = xfs_sysfs_release, .sysfs_ops = &xfs_sysfs_ops,@@ -349,11 +462,15 @@ struct kobj_type xfs_error_ktype = {struct xfs_error_init {char *name;int fail_speed; +int max_retries; +int retry_timeout;/* in seconds */ };static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX ] = { { .name ="Default" , .fail_speed =XFS_ERR_FAIL_NEVER , + .max_retries =INT_MAX , + .retry_timeout =0 , }, };@@ -384,6 +501,9 @@ xfs_error_sysfs_init_class( goto out_error; cfg->fail_speed = init[i].fail_speed; + cfg->max_retries = init[i].max_retries; + cfg->retry_timeout =msecs_to_jiffies ( + init[i].retry_timeout *MSEC_PER_SEC ); }return 0 ;Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 6/9 xfs: add "fail at unmount" error handling configuration
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
If we take "retry forever" literally on metadata IO errors, we can hang an unmount retries those writes forever. This is the default behaviour, unfortunately. Add a error configuration option for this behaviour and default it to "fail" so that an unmount will trigger actual errors, a shutdown and allow the unmount to succeed. It will be noisy, though, as it will log the errors and shutdown that occurs.
To do this, we need to mark the filesystem as being in the process of unmounting. Do this with a mount flag that is added at the appropriate time (i.e. before the blocking AIL sync). We also need to add this flag if mount fails after the initial phase of log recovery has been run.
The config is done by a separate boolean sysfs option rather than a new fail_speed enum, as fail_at_unmount is relevant to both XFS_ERR_FAIL_NEVER and XFS_ERR_FAIL_SLOW options.
--- fs/xfs/xfs_buf_item.c | 4 ++++ fs/xfs/xfs_mount.c | 9 +++++++++ fs/xfs/xfs_mount.h | 2 ++ fs/xfs/xfs_sysfs.c | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+)
fs/xfs/xfs_buf_item.c +4 -0
@@ -1001,6 +1001,10 @@ xfs_buf_iodone_callback_error(break ; } +/* At unmount we may treat errors differently */ +if ((mp->m_flags &XFS_MOUNT_UNMOUNTING ) && cfg->fail_at_unmount) + goto permanent_error; +/* still a transient error, higher layers will retry */ xfs_buf_ioerror (bp,0 );xfs_buf_relse (bp);
fs/xfs/xfs_mount.c +9 -0
@@ -980,6 +980,7 @@ xfs_mountfs(cancel_delayed_work_sync (&mp->m_reclaim_work);xfs_reclaim_inodes (mp,SYNC_WAIT ); out_log_dealloc: + mp->m_flags |=XFS_MOUNT_UNMOUNTING ;xfs_log_mount_cancel (mp); out_fail_wait:if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)
@@ -1031,6 +1032,14 @@ xfs_unmountfs(xfs_log_force (mp,XFS_LOG_SYNC );/* +* We now need to tell the world we are unmounting. This will allow +* us to detect that the filesystem is going away and we should error +* out anything that we have been retrying in the background. This will +* prevent neverending retries iin AIL pushing from hanging the unmount. +*/ + mp->m_flags |=XFS_MOUNT_UNMOUNTING ; + +/* * Flush all pending changes from the AIL. */ xfs_ail_push_all_sync (mp->m_ail);
fs/xfs/xfs_mount.h +2 -0
@@ -64,6 +64,7 @@ struct xfs_error_cfg {int fail_speed;int max_retries;/* INT_MAX = retry forever */ unsigned long retry_timeout;/* in jiffies, 0 = no timeout */ +bool fail_at_unmount; }; typedef struct xfs_mount {
@@ -187,6 +188,7 @@ typedef struct xfs_mount {#define XFS_MOUNT_WSYNC (1ULL << 0)/* for nfs - all metadata ops must be synchronous except for space allocations */ +#define XFS_MOUNT_UNMOUNTING (1ULL << 1)/* filesystem is unmounting */ #define XFS_MOUNT_WAS_CLEAN (1ULL << 3)#define XFS_MOUNT_FS_SHUTDOWN (1ULL << 4)/* atomic stop of all filesystem operations, typically for
fs/xfs/xfs_sysfs.c +36 -0
@@ -435,10 +435,43 @@ retry_timeout_seconds_store( }XFS_SYSFS_ATTR_RW (retry_timeout_seconds); +static ssize_t +fail_at_unmount_show ( +struct kobject *kobject, +char *buf) +{ +struct xfs_error_cfg *cfg =to_error_cfg (kobject); + +return snprintf (buf,PAGE_SIZE ,"%d\n" , cfg->fail_at_unmount); +} + +static ssize_t +fail_at_unmount_store ( +struct kobject *kobject, +const char *buf, +size_t count) +{ +struct xfs_error_cfg *cfg =to_error_cfg (kobject); +int ret; +int val; + + ret =kstrtoint (buf,0 , &val); +if (ret) +return ret; + +if (val <0 || val >1 ) +return -EINVAL ; + + cfg->fail_at_unmount = val; +return count; +} +XFS_SYSFS_ATTR_RW (fail_at_unmount); +static struct attribute *xfs_error_attrs[] = {ATTR_LIST (failure_speed),ATTR_LIST (max_retries),ATTR_LIST (retry_timeout_seconds), +ATTR_LIST (fail_at_unmount),NULL , };
@@ -464,6 +497,7 @@ struct xfs_error_init {int fail_speed;int max_retries;int retry_timeout;/* in seconds */ +bool fail_at_unmount; };static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX ] = {
@@ -471,6 +505,7 @@ static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = { .fail_speed = XFS_ERR_FAIL_NEVER, .max_retries = INT_MAX, .retry_timeout = 0, + .fail_at_unmount = true, }, };
@@ -504,6 +539,7 @@ xfs_error_sysfs_init_class( cfg->max_retries = init[i].max_retries; cfg->retry_timeout =msecs_to_jiffies ( init[i].retry_timeout *MSEC_PER_SEC ); + cfg->fail_at_unmount = init[i].fail_at_unmount; }return 0 ;
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 6/9] xfs: add "fail at unmount" error handling configuration
Quoted 16 lines · Dave Chinner
From: Dave Chinner <[email protected]>
If we take "retry forever" literally on metadata IO errors, we can hang an unmount retries those writes forever. This is the default behaviour, unfortunately. Add a error configuration option for this behaviour and default it to "fail" so that an unmount will trigger actual errors, a shutdown and allow the unmount to succeed. It will be noisy, though, as it will log the errors and shutdown that occurs.
To do this, we need to mark the filesystem as being in the process of unmounting. Do this with a mount flag that is added at the appropriate time (i.e. before the blocking AIL sync). We also need to add this flag if mount fails after the initial phase of log recovery has been run.
Quoted 6 lines
The config is done by a separate boolean sysfs option rather than a new
fail_speedenum, asfail_at_unmountis relevant to bothXFS_ERR_FAIL_NEVERandXFS_ERR_FAIL_SLOWoptions.Signed-off-by Dave Chinner <[email protected]>---
Similar question of scope/granularity here... why would one want to set this option for a particular error and not any others? In other words, it seems more useful as a global (or per-mount) option.
Brian
Quoted 147 lines
fs/xfs/xfs_buf_item.c | 4 ++++ fs/xfs/xfs_mount.c | 9 +++++++++ fs/xfs/xfs_mount.h | 2 ++ fs/xfs/xfs_sysfs.c | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+)fs/xfs/xfs_buf_item.c +4
-0@@ -1001,6 +1001,10 @@ xfs_buf_iodone_callback_error(break ; } +/* At unmount we may treat errors differently */ +if ((mp->m_flags &XFS_MOUNT_UNMOUNTING ) && cfg->fail_at_unmount) + goto permanent_error; +/* still a transient error, higher layers will retry */ xfs_buf_ioerror (bp,0 );xfs_buf_relse (bp);fs/xfs/xfs_mount.c +9
-0@@ -980,6 +980,7 @@ xfs_mountfs(cancel_delayed_work_sync (&mp->m_reclaim_work);xfs_reclaim_inodes (mp,SYNC_WAIT ); out_log_dealloc: + mp->m_flags |=XFS_MOUNT_UNMOUNTING ;xfs_log_mount_cancel (mp); out_fail_wait:if (mp->m_logdev_targp && mp->m_logdev_targp != mp->m_ddev_targp)@@ -1031,6 +1032,14 @@ xfs_unmountfs(xfs_log_force (mp,XFS_LOG_SYNC );/* +* We now need to tell the world we are unmounting. This will allow +* us to detect that the filesystem is going away and we should error +* out anything that we have been retrying in the background. This will +* prevent neverending retries iin AIL pushing from hanging the unmount. +*/ + mp->m_flags |=XFS_MOUNT_UNMOUNTING ; + +/* * Flush all pending changes from the AIL. */ xfs_ail_push_all_sync (mp->m_ail);fs/xfs/xfs_mount.h +2
-0@@ -64,6 +64,7 @@ struct xfs_error_cfg {int fail_speed;int max_retries;/* INT_MAX = retry forever */ unsigned long retry_timeout;/* in jiffies, 0 = no timeout */ +bool fail_at_unmount; }; typedef struct xfs_mount {@@ -187,6 +188,7 @@ typedef struct xfs_mount {#define XFS_MOUNT_WSYNC (1ULL << 0)/* for nfs - all metadata ops must be synchronous except for space allocations */ +#define XFS_MOUNT_UNMOUNTING (1ULL << 1)/* filesystem is unmounting */ #define XFS_MOUNT_WAS_CLEAN (1ULL << 3)#define XFS_MOUNT_FS_SHUTDOWN (1ULL << 4)/* atomic stop of all filesystem operations, typically for fs/xfs/xfs_sysfs.c +36
-0@@ -435,10 +435,43 @@ retry_timeout_seconds_store( }XFS_SYSFS_ATTR_RW (retry_timeout_seconds); +static ssize_t +fail_at_unmount_show ( +struct kobject *kobject, +char *buf) +{ +struct xfs_error_cfg *cfg =to_error_cfg (kobject); + +return snprintf (buf,PAGE_SIZE ,"%d\n" , cfg->fail_at_unmount); +} + +static ssize_t +fail_at_unmount_store ( +struct kobject *kobject, +const char *buf, +size_t count) +{ +struct xfs_error_cfg *cfg =to_error_cfg (kobject); +int ret; +int val; + + ret =kstrtoint (buf,0 , &val); +if (ret) +return ret; + +if (val <0 || val >1 ) +return -EINVAL ; + + cfg->fail_at_unmount = val; +return count; +} +XFS_SYSFS_ATTR_RW (fail_at_unmount); +static struct attribute *xfs_error_attrs[] = {ATTR_LIST (failure_speed),ATTR_LIST (max_retries),ATTR_LIST (retry_timeout_seconds), +ATTR_LIST (fail_at_unmount),NULL , };@@ -464,6 +497,7 @@ struct xfs_error_init {int fail_speed;int max_retries;int retry_timeout;/* in seconds */ +bool fail_at_unmount; };static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX ] = {@@ -471,6 +505,7 @@ static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = { .fail_speed = XFS_ERR_FAIL_NEVER, .max_retries = INT_MAX, .retry_timeout = 0, + .fail_at_unmount = true, }, };@@ -504,6 +539,7 @@ xfs_error_sysfs_init_class( cfg->max_retries = init[i].max_retries; cfg->retry_timeout =msecs_to_jiffies ( init[i].retry_timeout *MSEC_PER_SEC ); + cfg->fail_at_unmount = init[i].fail_at_unmount; }return 0 ;Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 6/9] xfs: add "fail at unmount" error handling configuration
Quoted 21 lines · Brian Foster
Quoted 20 lines · Dave Chinner
Quoted 20 lines
From: Dave Chinner <[email protected]>
If we take "retry forever" literally on metadata IO errors, we can hang an unmount retries those writes forever. This is the default behaviour, unfortunately. Add a error configuration option for this behaviour and default it to "fail" so that an unmount will trigger actual errors, a shutdown and allow the unmount to succeed. It will be noisy, though, as it will log the errors and shutdown that occurs.
To do this, we need to mark the filesystem as being in the process of unmounting. Do this with a mount flag that is added at the appropriate time (i.e. before the blocking AIL sync). We also need to add this flag if mount fails after the initial phase of log recovery has been run.
The config is done by a separate boolean sysfs option rather than a new
fail_speedenum, asfail_at_unmountis relevant to bothXFS_ERR_FAIL_NEVERandXFS_ERR_FAIL_SLOWoptions.
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
Signed-off-by Dave Chinner <[email protected]>---
Similar question of scope/granularity here... why would one want to set this option for a particular error and not any others? In other words, it seems more useful as a global (or per-mount) option.
I guess my question here is higher-level than that. Why make this (fail_at_unmount) configurable at all. When would one *want* unmount blocked by pending failure retries?
I guess I could imagine it as sort of a safety net, "I told it to retry for a day, and the day's not up yet, so we shouldn't stop trying just because I said unmount!" - but that seems a bit contrived to me.
-Eric
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 7/9 xfs: add configuration handles for specific errors
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
now most of the infrastructure is in place, we can start adding support for configuring specific errors such as ENODEV, ENOSPC, EIO, etc. Add these error configurations and configure them all to have appropriate behaviours. That is, all will use the "fail never, fail at unmount" default except for ENODEV, which is an unrecoverable error so it will be configured as a "fail fast" error.
--- fs/xfs/xfs_mount.h | 3 +++ fs/xfs/xfs_sysfs.c | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+)
fs/xfs/xfs_mount.h +3 -0
@@ -50,6 +50,9 @@ enum { };enum {XFS_ERR_DEFAULT , +XFS_ERR_EIO , +XFS_ERR_ENOSPC , +XFS_ERR_ENODEV ,XFS_ERR_ERRNO_MAX , };enum {
fs/xfs/xfs_sysfs.c +24 -0
@@ -507,6 +507,21 @@ static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = { .retry_timeout = 0, .fail_at_unmount = true, }, + { .name = "EIO", + .fail_speed = XFS_ERR_FAIL_NEVER, + .max_retries = INT_MAX, + .retry_timeout = 0, + .fail_at_unmount = true, + }, + { .name = "ENOSPC", + .fail_speed = XFS_ERR_FAIL_NEVER, + .max_retries = INT_MAX, + .retry_timeout = 0, + .fail_at_unmount = true, + }, + { .name = "ENODEV", + .fail_speed = XFS_ERR_FAIL_FAST, + }, }; static int
@@ -606,6 +621,15 @@ xfs_error_get_cfg(struct xfs_error_cfg *cfg;switch (error) { +case EIO : + cfg = &mp->m_error_cfg[error_class][XFS_ERR_EIO ]; +break ; +case ENOSPC : + cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENOSPC ]; +break ; +case ENODEV : + cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENODEV ]; +break ;default : cfg = &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT ];break ;
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
PATCH 8/9 xfs: disable specific error configurations
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
Different error classes are going to need different error to be configured, so we don't want them all to be visible in sysfs. Add a configuration check into the config initialisation an lookup code to determine if the default should be used for a specific error. If so, the sysfs entry is not created, and on lookup the default config is returned.
Add ENOMEM at this point to exercise this code, as it will be used later when adding a kmem error failure class.
--- fs/xfs/xfs_mount.h | 1 + fs/xfs/xfs_sysfs.c | 22 ++++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-)
fs/xfs/xfs_mount.h +1 -0
@@ -53,6 +53,7 @@ enum {XFS_ERR_EIO ,XFS_ERR_ENOSPC ,XFS_ERR_ENODEV , +XFS_ERR_ENOMEM , XFS_ERR_ERRNO_MAX, }; enum {
fs/xfs/xfs_sysfs.c +18 -4
@@ -522,6 +522,9 @@ static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = { { .name = "ENODEV", .fail_speed = XFS_ERR_FAIL_FAST, }, + { .name = "ENOMEM", + .fail_speed = XFS_ERR_FAIL_DEFAULT, + }, }; static int
@@ -545,12 +548,17 @@ xfs_error_sysfs_init_class(for (i =0 ; i <XFS_ERR_ERRNO_MAX ; i++) { cfg = &mp->m_error_cfg[class][i]; + +/* skip errors that are not configurable for this class */ + cfg->fail_speed = init[i].fail_speed; +if (cfg->fail_speed ==XFS_ERR_FAIL_DEFAULT ) +continue ; + error =xfs_sysfs_init (&cfg->kobj, &xfs_error_cfg_ktype, parent_kobj, init[i].name);if (error) goto out_error; - cfg->fail_speed = init[i].fail_speed; cfg->max_retries = init[i].max_retries; cfg->retry_timeout =msecs_to_jiffies ( init[i].retry_timeout *MSEC_PER_SEC );
@@ -605,7 +613,8 @@ xfs_error_sysfs_del(for (j =0 ; j <XFS_ERR_ERRNO_MAX ; j++) { cfg = &mp->m_error_cfg[i][j]; -xfs_sysfs_del (&cfg->kobj); +if (cfg->fail_speed !=XFS_ERR_FAIL_DEFAULT ) +xfs_sysfs_del (&cfg->kobj); } }xfs_sysfs_del (&mp->m_error_meta_kobj);
@@ -630,10 +639,15 @@ xfs_error_get_cfg(case ENODEV : cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENODEV ];break ; -default : - cfg = &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT ]; +case ENOMEM : + cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENOMEM ];break ; +default : +return &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT ]; } +/* The error may not be not configurable, so uses default behaviour */ +if (cfg->fail_speed ==XFS_ERR_FAIL_DEFAULT ) +return &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT ];return cfg; }
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 8/9] xfs: disable specific error configurations
Quoted 45 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Different error classes are going to need different error to be configured, so we don't want them all to be visible in sysfs. Add a configuration check into the config initialisation an lookup code to determine if the default should be used for a specific error. If so, the sysfs entry is not created, and on lookup the default config is returned.
Add ENOMEM at this point to exercise this code, as it will be used later when adding a kmem error failure class.
Signed-off-by Dave Chinner <[email protected]>--- fs/xfs/xfs_mount.h | 1 + fs/xfs/xfs_sysfs.c | 22 ++++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-)fs/xfs/xfs_mount.h +1
-0@@ -53,6 +53,7 @@ enum {XFS_ERR_EIO ,XFS_ERR_ENOSPC ,XFS_ERR_ENODEV , +XFS_ERR_ENOMEM , XFS_ERR_ERRNO_MAX, }; enum {fs/xfs/xfs_sysfs.c +3
-0@@ -522,6 +522,9 @@ static const struct xfs_error_init xfs_error_meta_init[XFS_ERR_ERRNO_MAX] = { { .name = "ENODEV", .fail_speed = XFS_ERR_FAIL_FAST, }, + { .name = "ENOMEM", + .fail_speed = XFS_ERR_FAIL_DEFAULT, + }, }; static int
fs/xfs/xfs_sysfs.c xfs_error_sysfs_init_class( · line 556
Quoted 9 lines
fs/xfs/xfs_sysfs.c +6
-0@@ -545,12 +548,17 @@ xfs_error_sysfs_init_class(for (i =0 ; i <XFS_ERR_ERRNO_MAX ; i++) { cfg = &mp->m_error_cfg[class][i]; + +/* skip errors that are not configurable for this class */ + cfg->fail_speed = init[i].fail_speed; +if (cfg->fail_speed ==XFS_ERR_FAIL_DEFAULT ) +continue ; +
It seems like this confuses a default configuration for an error with a "tunable" or "configurable" flag (or otherwise has confused me ;). E.g., what happens if we want to alter ENOMEM behavior from the "default" behavior, retain the current definition in the "Default" entry, but still not expose the ENOMEM configuration to the user?
Brian
Quoted 44 lines
fs/xfs/xfs_sysfs.c +9
-4error =xfs_sysfs_init (&cfg->kobj, &xfs_error_cfg_ktype, parent_kobj, init[i].name);if (error) goto out_error; - cfg->fail_speed = init[i].fail_speed; cfg->max_retries = init[i].max_retries; cfg->retry_timeout =msecs_to_jiffies ( init[i].retry_timeout *MSEC_PER_SEC );@@ -605,7 +613,8 @@ xfs_error_sysfs_del(for (j =0 ; j <XFS_ERR_ERRNO_MAX ; j++) { cfg = &mp->m_error_cfg[i][j]; -xfs_sysfs_del (&cfg->kobj); +if (cfg->fail_speed !=XFS_ERR_FAIL_DEFAULT ) +xfs_sysfs_del (&cfg->kobj); } }xfs_sysfs_del (&mp->m_error_meta_kobj);@@ -630,10 +639,15 @@ xfs_error_get_cfg(case ENODEV : cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENODEV ];break ; -default : - cfg = &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT ]; +case ENOMEM : + cfg = &mp->m_error_cfg[error_class][XFS_ERR_ENOMEM ];break ; +default : +return &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT ]; } +/* The error may not be not configurable, so uses default behaviour */ +if (cfg->fail_speed ==XFS_ERR_FAIL_DEFAULT ) +return &mp->m_error_cfg[error_class][XFS_ERR_DEFAULT ];return cfg; }Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 9/9 xfs: add kmem error configuration class
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
Add a new error configuration class for defining how memory allocation failures are supposed to be handled, This requires factoring the initialisation/teardown code to be able to handle a generic error class object.
I'm not yet sure how to hook it into the memory allocation calls - that will be done in a later patch; this just demonstrates how multiple classes are configured and initialised. It may be that we don't configure specific errors here - instead configure how we handle specific types of allocation failure e.g. GFP_KERNEL vs GFP_NOFS vs allocations inside transactions. Either way, we are going to need to plumb the error config handler into the memory allocation code in some manner.
--- fs/xfs/xfs_mount.h | 3 ++- fs/xfs/xfs_sysfs.c | 77 +++++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 61 insertions(+), 19 deletions(-)
fs/xfs/xfs_mount.h +2 -1
@@ -46,6 +46,7 @@ enum {*/ enum {XFS_ERR_METADATA , +XFS_ERR_KMEM ,XFS_ERR_CLASS_MAX , };enum {
@@ -162,7 +163,7 @@ typedef struct xfs_mount {/* low free space thresholds */ struct xfs_kobj m_kobj;struct xfs_kobj m_error_kobj; -struct xfs_kobj m_error_meta_kobj; +struct xfs_kobj m_error_class_kobj[XFS_ERR_CLASS_MAX ];struct xfs_error_cfg m_error_cfg[XFS_ERR_CLASS_MAX ][XFS_ERR_ERRNO_MAX ];struct xstats m_stats;/* per-fs stats */
fs/xfs/xfs_sysfs.c +59 -18
135 lines not on this page · raw message
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
ping?
Quoted 61 lines · Dave Chinner
Hi folks,
I need to restart the discussion and review of this patch series. There was some discussion of it last time, but nothing really came from that. I'm posting what I have in my tree right now - treat it as though it's an initial posting of the code because I can't recall what I've changed since the first posting.
What I'd like to have to for the next merge window is all the IO error bits sorted out. The final patch (kmem failure behaviour) needs more infrastructure (passing mp to every allocation) so that's a secondary concern right now and I've only included it to demonstrate how to apply this code ot a different subsystem.
Things that need to be nailed down before I can commit the series:
- sysfs layout
- naming conventions for errors and subsystems in sysfs
- how best to display/change default behaviourThings that we can change/implement later:
- default behaviour
- additional error classes
- additional error types
- additional subsystems
- subsystem error handling implementation
- communication with other subsystems to dynamically change error behaviourIOWs, what is important right now is how we present this to userspace, because we can't change that easily once we've decided on a presentation structure.
Modifying the code to classify and handle all the different error types is much less important, as we can change that to fix whatever problems we have without impacting the presentation to userspace.
There is definite need for this (e.g. handling of ENOSPC on thin provisioned devices), so I want to get quickly to a consensus on the userspace facing aspects so that we can get this ball rolling.
The biggest unsolved issue is how to change the default behaviour persistently. There is no infrastructure in this patch series to do that, but it is someting that we have to consider so that we don't require default behaviour to be changed after every mount of every filesystem on a system. My thoughts on this is we store changes to the defaults in xattrs on the root inode, but I'm open to ideas here as there's no code written for it yet. Solving this problem, however, is not necessary before commiting the initial code; it's something we can add later once we've worked out all the details.
Discuss!
-Dave.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Hi Dave,
Here are a few comments about it, I didn't finish to review all the patches, so, for now, it's an implementation review, other than code review.
First, I tend to agree with Brian and Eric in most of their comments.
I don't see any point in having a fail fast/slow/never, if we can configure the number of retries, so, it just looks redundant to me.
Setting 1, -1 or N for fail fast/never/after N times, looks simpler.
Also, I don't see a point in having a fail_at_unmount option, IMHO it should always fail at unmount, unless, if we expect the sysadmin to extend the underlying storage device (let's say here, a thin pool), so that we can flush all the metadata in AIL and clean unmount.
I'm sorry if I'm wrong here, but, shouldn't xfs_log_force also check for the error configuration here? Otherwise, if, we set the configuration from fail never to fail fast, after an unmount has been issued, unmount will be stuck waiting for xfs_log_worker to finish metadata writeback, which will never happen since we have no space and, it does not check for the error configuration.
I can easily reproduce this behavior trying to unmount a filesystem, then set the configuration to fail fast after the unmount.
Unmount will be stuck at:
[<ffffffff8138ad56>] xfs_ail_push_all_sync+0xb6/0x100 [<ffffffff813750ee>] xfs_unmountfs+0x6e/0x1b0 [<ffffffff81377bf0>] xfs_fs_put_super+0x30/0x90 [<ffffffff812193da>] generic_shutdown_super+0x6a/0xf0 [<ffffffff81219747>] kill_block_super+0x27/0x70 [<ffffffff81219a83>] deactivate_locked_super+0x43/0x70 [<ffffffff81219b0c>] deactivate_super+0x5c/0x60 [<ffffffff81236baf>] cleanup_mnt+0x3f/0x90 [<ffffffff81236c42>] __cleanup_mnt+0x12/0x20 [<ffffffff810a49c3>] task_work_run+0x73/0x90 [<ffffffff81002242>] exit_to_usermode_loop+0xc2/0xd0 [<ffffffff81002d11>] syscall_return_slowpath+0xa1/0xb0 [<ffffffff818129cc>] int_ret_from_sys_call+0x25/0x8f
[<ffffffffffffffff>] 0xffffffffffffffff
and kernel will be spinning into:
kworker/3:1H-478 [003] .... 361.187791: xfs_log_force: dev 253:4 lsn 0x0
caller xfs_log_worker
or from dmesg output:
[ 103.881961] XFS (dm-4): Unmounting Filesystem [ 103.882824] XFS (dm-4): xfs_do_force_shutdown(0x1) called from line 1134 of file fs/xfs/xfs_buf_item.c. Return address = 0xffffffff813818d7 [ 103.884101] XFS (dm-4): I/O Error Detected. Shutting down filesystem [ 103.884713] XFS (dm-4): Please umount the filesystem and rectify the problem(s) [ 121.185786] XFS (dm-4): xfs_log_force: error -5 returned. [ 151.185818] XFS (dm-4): xfs_log_force: error -5 returned.
I'll add more comments as soon as I finish to review the patches.
Btw, while testing it, I found a bug in dm-thin and direct-io code, which return wrong error information to the filesystem, and actually prevented your patchset to work as expected, I got a few patches to dm-thin and direct io to test, and the patches fixed the problem I found, but I'm not sure when they will be posted upstream. Hopefully today? :)
Quoted 60 lines · Dave Chinner
Hi folks,
I need to restart the discussion and review of this patch series. There was some discussion of it last time, but nothing really came from that. I'm posting what I have in my tree right now - treat it as though it's an initial posting of the code because I can't recall what I've changed since the first posting.
What I'd like to have to for the next merge window is all the IO error bits sorted out. The final patch (kmem failure behaviour) needs more infrastructure (passing mp to every allocation) so that's a secondary concern right now and I've only included it to demonstrate how to apply this code ot a different subsystem.
Things that need to be nailed down before I can commit the series:
- sysfs layout
- naming conventions for errors and subsystems in sysfs
- how best to display/change default behaviourThings that we can change/implement later:
- default behaviour
- additional error classes
- additional error types
- additional subsystems
- subsystem error handling implementation
- communication with other subsystems to dynamically change error behaviourIOWs, what is important right now is how we present this to userspace, because we can't change that easily once we've decided on a presentation structure.
Modifying the code to classify and handle all the different error types is much less important, as we can change that to fix whatever problems we have without impacting the presentation to userspace.
There is definite need for this (e.g. handling of ENOSPC on thin provisioned devices), so I want to get quickly to a consensus on the userspace facing aspects so that we can get this ball rolling.
The biggest unsolved issue is how to change the default behaviour persistently. There is no infrastructure in this patch series to do that, but it is someting that we have to consider so that we don't require default behaviour to be changed after every mount of every filesystem on a system. My thoughts on this is we store changes to the defaults in xattrs on the root inode, but I'm open to ideas here as there's no code written for it yet. Solving this problem, however, is not necessary before commiting the initial code; it's something we can add later once we've worked out all the details.
Discuss!
-Dave.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Signature · 6 lines
Carlos _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs