On 02/12/15 17:14, Dave Chinner wrote:
From: Dave Chinner<dchinner@xxxxxxxxxx> Test generic/224 is failing with a corruption being detected on one of Michael's test boxes. Debug that Michael added is indicating that the minleft trimming is resulting in an underflow: ..... before fixup: rlen 1 args->len 0 after xfs_alloc_fix_len : rlen 1 args->len 1 before goto out_nominleft: rlen 1 args->len 0 before fixup: rlen 1 args->len 0 after xfs_alloc_fix_len : rlen 1 args->len 1 after fixup: rlen 1 args->len 1 before fixup: rlen 1 args->len 0 after xfs_alloc_fix_len : rlen 1 args->len 1 after fixup: rlen 4294967295 args->len 4294967295 XFS: Assertion failed: fs_is_ok, file: fs/xfs/libxfs/xfs_alloc.c, line: 1424 The "goto out_nominleft:" indicates that we are getting close to ENOSPC in the AG, and a couple of allocations later we underflow and the corruption check fires in xfs_alloc_ag_vextent_size(). The issue is that the extent length fixups comaprisons are done with variables of xfs_extlen_t types. These are unsigned so an underflow looks like a really big value and hence is not detected as being smaller than the minimum length allowed for the extent. Hence the corruption check fires as it is noticing that the returned length is longer than the original extent length passed in. This can be easily fixed by ensuring we do the underflow test on signed values, the same way xfs_alloc_fix_len() prevents underflow. So we realise in future that these casts prevent underflows from going undetected, add comments to the code indicating this. Reported-by: Michael L. Semon<mlsemon35@xxxxxxxxx> Tested-by: Michael L. Semon<mlsemon35@xxxxxxxxx> Signed-off-by: Dave Chinner<dchinner@xxxxxxxxxx> --- fs/xfs/libxfs/xfs_alloc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
int diff = be32_to_cpu(agf->agf_freeblks) - args->len - args->minleft;
@@ -286,7 +287,8 @@ xfs_alloc_fix_minleft( if (diff >= 0) return 1;
If the diff math was done correctly, wouldn't it get caught here?
args->len += diff; /* shrink the allocated space */ - if (args->len >= args->minlen) + /* casts to (int) catch length underflows */ + if ((int)args->len >= (int)args->minlen) return 1; args->agbno = NULLAGBLOCK; return 0;
We can and should fix the wrap in xfs_alloc_fix_minleft() but this also points to the fact that xfs_alloc_fix_freelist() is incorrectly choosing AGs that will later fail the allocation alignment, minlen, and minleft requirements.
You can connect the dots to see how this can lead to a deadlock with extent frees. We have seen them. I hacked the XFS code to lead to this situation.
Also bad is xfs_alloc_vextent() will temporally ignore the minleft for the xfs_alloc_fix_freelist() but makes the ag allocator enforce the minleft. I got this condition to ASSERT in xfs_alloc_ag_vextent_size(), but I never got a deadlock. It would require just the right sequence from xfs_alloc_vextent() and xfs_bmap_btalloc().
--Mark. _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs