xfs: regression fixes
cover xfs: regression fixes
Hi folks,
These are a couple of fixes that have come to light in the past few days. The first prevents bad things happening in memory reclaim when kswapd delegates block allocation to a workqueue, and the second ensures XFS doesn't incorrectly try to check the sector size on non-xfs devices when mount is probing.
Both are regressions, but given how late in the 3.15 cycle we are, I'll probably just push these through the 3.16 merge to give them a bit more testing and let them be handled by stable kernel backports. Anyone have any particular concerns over that plan?
Cheers,
Dave.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 1/2 xfs: block allocation work needs to be kswapd aware
From: Dave Chinner <[email protected]>
Upon memory pressure, kswapd calls xfs_vm_writepage() from shrink_page_list(). This can result in delayed allocation occurring and that gets deferred to the the allocation workqueue.
The allocation then runs outside kswapd context, which means if it needs memory (and it does to demand page metadata from disk) it can block in shrink_inactive_list() waiting for IO congestion. These blocking waits are normally avoiding in kswapd context, so under memory pressure writeback from kswapd can be arbitrarily delayed by memory reclaim.
To avoid this, pass the kswapd context to the allocation being done by the workqueue, so that memory reclaim understands correctly that the work is being done for kswapd and therefore it is not blocked and does not delay memory reclaim.
To avoid issues with int->char conversion of flag fields (as noticed in v1 of this patch) convert the flag fields in the struct xfs_bmalloca to bool types. pahole indicates these variables are still single byte variables, so no extra space is consumed by this change.
--- fs/xfs/xfs_bmap_util.c | 16 +++++++++++++--- fs/xfs/xfs_bmap_util.h | 13 +++++++------ 2 files changed, 20 insertions(+), 9 deletions(-)
fs/xfs/xfs_bmap_util.c +13 -3
@@ -258,14 +258,23 @@ xfs_bmapi_allocate_worker(struct xfs_bmalloca *args =container_of (work, struct xfs_bmalloca, work);unsigned long pflags; +unsigned long new_pflags =PF_FSTRANS ; -/* we are in a transaction context here */ -current_set_flags_nested (&pflags,PF_FSTRANS ); +/* +* we are in a transaction context here, but may also be doing work +* in kswapd context, and hence we may need to inherit that state +* temporarily to ensure that we don't block waiting for memory reclaim +* in any way. +*/ +if (args->kswapd) + new_pflags |=PF_MEMALLOC |PF_SWAPWRITE |PF_KSWAPD ; + +current_set_flags_nested (&pflags, new_pflags); args->result =__xfs_bmapi_allocate (args);complete (args->done); -current_restore_flags_nested (&pflags,PF_FSTRANS ); +current_restore_flags_nested (&pflags, new_pflags); }/*
@@ -284,6 +293,7 @@ xfs_bmapi_allocate( args->done = &done; + args->kswapd =current_is_kswapd ();INIT_WORK_ONSTACK (&args->work, xfs_bmapi_allocate_worker);queue_work (xfs_alloc_wq, &args->work);wait_for_completion (&done);
fs/xfs/xfs_bmap_util.h +7 -6
@@ -50,12 +50,13 @@ struct xfs_bmalloca {xfs_extlen_t total;/* total blocks needed for xaction */ xfs_extlen_t minlen;/* minimum allocation size (blocks) */ xfs_extlen_t minleft;/* amount must be left after alloc */ -char eof;/* set if allocating past last extent */ -char wasdel;/* replacing a delayed allocation */ -char userdata;/* set if is user data */ -char aeof;/* allocated space at eof */ -char conv;/* overwriting unwritten extents */ -char stack_switch; +bool eof;/* set if allocating past last extent */ +bool wasdel;/* replacing a delayed allocation */ +bool userdata;/* set if is user data */ +bool aeof;/* allocated space at eof */ +bool conv;/* overwriting unwritten extents */ +bool stack_switch; +bool kswapd;/* allocation in kswapd context */ int flags;struct completion *done;struct work_struct work;
Signature · 6 lines
1.9.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 1/2] xfs: block allocation work needs to be kswapd aware
Quoted 18 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Upon memory pressure, kswapd calls
xfs_vm_writepage()fromshrink_page_list(). This can result in delayed allocation occurring and that gets deferred to the the allocation workqueue.The allocation then runs outside kswapd context, which means if it needs memory (and it does to demand page metadata from disk) it can block in
shrink_inactive_list()waiting for IO congestion. These blocking waits are normally avoiding in kswapd context, so under memory pressure writeback from kswapd can be arbitrarily delayed by memory reclaim.To avoid this, pass the kswapd context to the allocation being done by the workqueue, so that memory reclaim understands correctly that the work is being done for kswapd and therefore it is not blocked and does not delay memory reclaim.
Quoted 5 lines
To avoid issues with
int->charconversion of flag fields (as noticed in v1 of this patch) convert the flag fields in the structxfs_bmallocato bool types. pahole indicates these variables are still single byte variables, so no extra space is consumed by this change.
Looks good,
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 2/2 xfs: xfs_readsb needs to check for magic numbers
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
Commit daba542 ("xfs: skip verification on initial "guess" superblock read") dropped the use of a verifier for the initial superblock read so we can probe the sector size of the filesystem stored in the superblock. It, however, now fails to validate that what was read initially is actually an XFS superblock and hence will fail the sector size check and return ENOSYS.
This causes probe-based mounts to fail because it expects XFS to return EINVAL when it doesn't recognise the superblock format.
--- fs/xfs/xfs_mount.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
fs/xfs/xfs_mount.c +17 -6
@@ -323,8 +323,19 @@ reread:/* * Initialize the mount structure from the superblock. */ -xfs_sb_from_disk (&mp->m_sb,XFS_BUF_TO_SBP (bp)); -xfs_sb_quota_from_disk (&mp->m_sb); +xfs_sb_from_disk (sbp,XFS_BUF_TO_SBP (bp)); +xfs_sb_quota_from_disk (sbp); + +/* +* If we haven't validated the superblock, do so now before we try +* to check the sector size and reread the superblock appropriately. +*/ +if (sbp->sb_magicnum !=XFS_SB_MAGIC ) { +if (loud) +xfs_warn (mp,"Invalid superblock magic number" ); + error =EINVAL ; + goto release_buf; + }/* * We must be able to do sector-sized and sector-aligned IO.
@@ -337,11 +348,11 @@ reread: goto release_buf; } -/* -* Re-read the superblock so the buffer is correctly sized, -* and properly verified. -*/ if (buf_ops ==NULL ) { +/* +* Re-read the superblock so the buffer is correctly sized, +* and properly verified. +*/ xfs_buf_relse (bp); sector_size = sbp->sb_sectsize; buf_ops = loud ? &xfs_sb_buf_ops : &xfs_sb_quiet_buf_ops;
Signature · 6 lines
1.9.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 2/2] xfs: xfs_readsb needs to check for magic numbers
Quoted 11 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Commit daba542 ("xfs: skip verification on initial "guess" superblock read") dropped the use of a verifier for the initial superblock read so we can probe the sector size of the filesystem stored in the superblock. It, however, now fails to validate that what was read initially is actually an XFS superblock and hence will fail the sector size check and return ENOSYS.
This causes probe-based mounts to fail because it expects XFS to return EINVAL when it doesn't recognise the superblock format.
Looks good,
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs