* [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC @ 2015-02-12 23:14 Dave Chinner 2015-02-13 23:40 ` Mark Tinguely 2015-02-16 13:41 ` Brian Foster 0 siblings, 2 replies; 10+ messages in thread From: Dave Chinner @ 2015-02-12 23:14 UTC (permalink / raw) To: xfs From: Dave Chinner <dchinner@redhat.com> 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@gmail.com> Tested-by: Michael L. Semon <mlsemon35@gmail.com> Signed-off-by: Dave Chinner <dchinner@redhat.com> --- fs/xfs/libxfs/xfs_alloc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c index 710554c..41e3630 100644 --- a/fs/xfs/libxfs/xfs_alloc.c +++ b/fs/xfs/libxfs/xfs_alloc.c @@ -260,6 +260,7 @@ xfs_alloc_fix_len( rlen = rlen - (k - args->mod); else rlen = rlen - args->prod + (args->mod - k); + /* casts to (int) catch length underflows */ if ((int)rlen < (int)args->minlen) return; ASSERT(rlen >= args->minlen && rlen <= args->maxlen); @@ -286,7 +287,8 @@ xfs_alloc_fix_minleft( if (diff >= 0) return 1; 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; -- 2.0.0 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC 2015-02-12 23:14 [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC Dave Chinner @ 2015-02-13 23:40 ` Mark Tinguely 2015-02-14 23:29 ` Dave Chinner 2015-02-16 13:41 ` Brian Foster 1 sibling, 1 reply; 10+ messages in thread From: Mark Tinguely @ 2015-02-13 23:40 UTC (permalink / raw) To: Dave Chinner; +Cc: xfs On 02/12/15 17:14, Dave Chinner wrote: > From: Dave Chinner<dchinner@redhat.com> > > 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@gmail.com> > Tested-by: Michael L. Semon<mlsemon35@gmail.com> > Signed-off-by: Dave Chinner<dchinner@redhat.com> > --- > 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@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC 2015-02-13 23:40 ` Mark Tinguely @ 2015-02-14 23:29 ` Dave Chinner 2015-02-16 3:39 ` Michael L. Semon 0 siblings, 1 reply; 10+ messages in thread From: Dave Chinner @ 2015-02-14 23:29 UTC (permalink / raw) To: Mark Tinguely; +Cc: xfs On Fri, Feb 13, 2015 at 05:40:29PM -0600, Mark Tinguely wrote: > On 02/12/15 17:14, Dave Chinner wrote: > >From: Dave Chinner<dchinner@redhat.com> > > > >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@gmail.com> > >Tested-by: Michael L. Semon<mlsemon35@gmail.com> > >Signed-off-by: Dave Chinner<dchinner@redhat.com> > >--- > > 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; > Preconditions: agf->agf_freeblks = 1 args->len = 1 args->minleft = 2 Therefore, diff = -2 > >@@ -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? No, diff < 0. > > args->len += diff; /* shrink the allocated space */ 1 += -2 = -1 > >- if (args->len >= args->minlen) if (0xffffffff >= 1) broken. > >+ /* casts to (int) catch length underflows */ > >+ if ((int)args->len >= (int)args->minlen) if (-1 >= 1) correct. > > 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. I don't think there's a problem there. At least, not the problem I think you trying to describe. > 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. You should post the test cases and the patch that exposes the issues you are concerned about. Otherwise we have no real idea of what problems you are talking about, and certainly can't reproduce them. > 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. Hmmm - I suspect you haven't understood the underlying reason for setting minleft to zero for the call to xfs_alloc_fix_freelist(). There's are two places we do this: single AG constrainted allocation, and the "any AG, but near ENOSPC" allocation. For single AG constrained allocation, minleft is something we can ignore because we must select that AG for allocation. minleft is applied against the total allocation requirement, not the minimum length of allocation that can be done. Hence we might be able to do a minlen allocation, but we'd reject it because we can't do a maxlen allocation due to minleft requirements. Hence we switch off minleft in that case when doing freelist checks so that we don't reject allocations that could do a minlen allocation successfully. This requires the low level allocator to be able to trim back the selected extent from whatever length it finds to repesect minleft, and if it can't then fail the allocation. That is the function of xfs_alloc_fix_minleft() - constrain the allocated extent to the required minleft or fail the allocation. And for the lowspace algorithm, the reasoning is similar. After two attempts to allocate in AGs that have enough space for a maxlen allocation, we switch off minleft so that we only constrain the AG search to AGs that can definitely satisfy a minlen allocation, hence improving the chance we can do an allocation successfully near ENOSPC. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC 2015-02-14 23:29 ` Dave Chinner @ 2015-02-16 3:39 ` Michael L. Semon 2015-02-16 17:35 ` Mark Tinguely 0 siblings, 1 reply; 10+ messages in thread From: Michael L. Semon @ 2015-02-16 3:39 UTC (permalink / raw) To: Dave Chinner, Mark Tinguely; +Cc: xfs On 02/14/15 18:29, Dave Chinner wrote: > On Fri, Feb 13, 2015 at 05:40:29PM -0600, Mark Tinguely wrote: >> On 02/12/15 17:14, Dave Chinner wrote: >>> From: Dave Chinner<dchinner@redhat.com> >>> >>> 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@gmail.com> >>> Tested-by: Michael L. Semon<mlsemon35@gmail.com> >>> Signed-off-by: Dave Chinner<dchinner@redhat.com> >>> --- >>> 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; >> > > Preconditions: > > agf->agf_freeblks = 1 > args->len = 1 > args->minleft = 2 > > Therefore, diff = -2 > >>> @@ -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? > > No, diff < 0. > > >>> args->len += diff; /* shrink the allocated space */ > > 1 += -2 > = -1 > >>> - if (args->len >= args->minlen) > > if (0xffffffff >= 1) > > broken. > >>> + /* casts to (int) catch length underflows */ >>> + if ((int)args->len >= (int)args->minlen) > > if (-1 >= 1) > > correct. > >>> 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. > > I don't think there's a problem there. At least, not the problem I > think you trying to describe. > >> 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. > > You should post the test cases and the patch that exposes the issues > you are concerned about. Otherwise we have no real idea of what > problems you are talking about, and certainly can't reproduce them. > >> 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. > > Hmmm - I suspect you haven't understood the underlying reason for > setting minleft to zero for the call to xfs_alloc_fix_freelist(). > There's are two places we do this: single AG constrainted > allocation, and the "any AG, but near ENOSPC" allocation. > > For single AG constrained allocation, minleft is something we can > ignore because we must select that AG for allocation. minleft is > applied against the total allocation requirement, not the minimum > length of allocation that can be done. Hence we might be able to do > a minlen allocation, but we'd reject it because we can't do a maxlen > allocation due to minleft requirements. > > Hence we switch off minleft in that case when doing freelist checks > so that we don't reject allocations that could do a minlen > allocation successfully. This requires the low level allocator to be > able to trim back the selected extent from whatever length it finds > to repesect minleft, and if it can't then fail the allocation. > That is the function of xfs_alloc_fix_minleft() - constrain the > allocated extent to the required minleft or fail the allocation. > > And for the lowspace algorithm, the reasoning is similar. After two > attempts to allocate in AGs that have enough space for a maxlen > allocation, we switch off minleft so that we only constrain the AG > search to AGs that can definitely satisfy a minlen allocation, hence > improving the chance we can do an allocation successfully near > ENOSPC. > > Cheers, > > Dave. I took something out of Mark's argument. The original condition was hit using fstests generic/224 on RAID-0, while sizing up an old Core 2 PC for a new purpose. With Mark mentioning AGs, I looked around a little bit. The case can be hit more easily with this test on 16-GB $TEST_DEV and $SCRATCH_DEV: while true; do MKFS_OPTIONS='-d agcount=16' ./check generic/224; done That may not mean anything--a 50 MB/s Caviar SE on a SATA-1 controller may just make the original test too slow for my patience--but the extra AGs do make things seem rather repeatable. I'll be happy to leave both the Core 2 PC and the i686 Pentium 4 PC in an unpatched state, should you or Mark want me to fetch more values via printk debugging. Everything else XFS is in pretty good shape for the "small or few" purposes here. Thanks! Michael _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC 2015-02-16 3:39 ` Michael L. Semon @ 2015-02-16 17:35 ` Mark Tinguely 2015-02-16 23:17 ` Dave Chinner 0 siblings, 1 reply; 10+ messages in thread From: Mark Tinguely @ 2015-02-16 17:35 UTC (permalink / raw) To: Michael L. Semon; +Cc: xfs On 02/15/15 21:39, Michael L. Semon wrote: > On 02/14/15 18:29, Dave Chinner wrote: >> On Fri, Feb 13, 2015 at 05:40:29PM -0600, Mark Tinguely wrote: >>> On 02/12/15 17:14, Dave Chinner wrote: >>>> From: Dave Chinner<dchinner@redhat.com> >>>> >>>> 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@gmail.com> >>>> Tested-by: Michael L. Semon<mlsemon35@gmail.com> >>>> Signed-off-by: Dave Chinner<dchinner@redhat.com> >>>> --- >>>> 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; >>> >> >> Preconditions: >> >> agf->agf_freeblks = 1 >> args->len = 1 >> args->minleft = 2 >> >> Therefore, diff = -2 >> >>>> @@ -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? >> >> No, diff< 0. Hee, what was I thinking? sdrawkcab ylsouivbO >> >> >>>> args->len += diff; /* shrink the allocated space */ >> >> 1 += -2 >> = -1 >> >>>> - if (args->len>= args->minlen) >> >> if (0xffffffff>= 1) >> >> broken. >> >>>> + /* casts to (int) catch length underflows */ >>>> + if ((int)args->len >= (int)args->minlen) >> >> if (-1>= 1) >> >> correct. >> >>>> 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. >> >> I don't think there's a problem there. At least, not the problem I >> think you trying to describe. >> >>> 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. >> >> You should post the test cases and the patch that exposes the issues >> you are concerned about. Otherwise we have no real idea of what >> problems you are talking about, and certainly can't reproduce them. >> >>> 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. >> >> Hmmm - I suspect you haven't understood the underlying reason for >> setting minleft to zero for the call to xfs_alloc_fix_freelist(). >> There's are two places we do this: single AG constrainted >> allocation, and the "any AG, but near ENOSPC" allocation. >> >> For single AG constrained allocation, minleft is something we can >> ignore because we must select that AG for allocation. minleft is >> applied against the total allocation requirement, not the minimum >> length of allocation that can be done. Hence we might be able to do >> a minlen allocation, but we'd reject it because we can't do a maxlen >> allocation due to minleft requirements. >> >> Hence we switch off minleft in that case when doing freelist checks >> so that we don't reject allocations that could do a minlen >> allocation successfully. This requires the low level allocator to be >> able to trim back the selected extent from whatever length it finds >> to repesect minleft, and if it can't then fail the allocation. >> That is the function of xfs_alloc_fix_minleft() - constrain the >> allocated extent to the required minleft or fail the allocation. >> >> And for the lowspace algorithm, the reasoning is similar. After two >> attempts to allocate in AGs that have enough space for a maxlen >> allocation, we switch off minleft so that we only constrain the AG >> search to AGs that can definitely satisfy a minlen allocation, hence >> improving the chance we can do an allocation successfully near >> ENOSPC. >> >> Cheers, >> >> Dave. > > I took something out of Mark's argument. The original condition was > hit using fstests generic/224 on RAID-0, while sizing up an old Core > 2 PC for a new purpose. With Mark mentioning AGs, I looked around a > little bit. The case can be hit more easily with this test on 16-GB > $TEST_DEV and $SCRATCH_DEV: > > while true; do MKFS_OPTIONS='-d agcount=16' ./check generic/224; done > > That may not mean anything--a 50 MB/s Caviar SE on a SATA-1 controller > may just make the original test too slow for my patience--but the > extra AGs do make things seem rather repeatable. > > I'll be happy to leave both the Core 2 PC and the i686 Pentium 4 PC > in an unpatched state, should you or Mark want me to fetch more > values via printk debugging. Everything else XFS is in pretty good > shape for the "small or few" purposes here. > > Thanks! > > Michael Thanks Michael, you don't need to hold your test box for me. I do have a way to recreate these ABBA AGF buffer allocation deadlocks and understand the whys and hows very well. I don't have a community way to make a xfstest for it but I think your test is getting close. I am certain that with Dave's patch alone, and pushed long enough you will hit the ABBA AGF buffer deadlock. It needs to do that allocation to the last AG while freeing an extent to that AG. --Mark. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC 2015-02-16 17:35 ` Mark Tinguely @ 2015-02-16 23:17 ` Dave Chinner 2015-02-17 15:36 ` Michael L. Semon 0 siblings, 1 reply; 10+ messages in thread From: Dave Chinner @ 2015-02-16 23:17 UTC (permalink / raw) To: Mark Tinguely; +Cc: Michael L. Semon, xfs On Mon, Feb 16, 2015 at 11:35:50AM -0600, Mark Tinguely wrote: > Thanks Michael, you don't need to hold your test box for me. I do > have a way to recreate these ABBA AGF buffer allocation deadlocks > and understand the whys and hows very well. I don't have a community > way to make a xfstest for it but I think your test is getting close. If you know what is causing them, then please explain how it occurs and how you think it needs to be fixed. Just telling us that you know something that we don't doesn't help us solve the problem. :( In general, the use of the args->firstblock is supposed to avoid the ABBA locking order issues with multiple allocations in the one transaction by preventing AG selection loops from looping back into AGs with a lower index than the first allocation that was made. So if you are seeing deadlocks, then it may be that we aren't following this constraint correctly in all locations.... Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC 2015-02-16 23:17 ` Dave Chinner @ 2015-02-17 15:36 ` Michael L. Semon 2015-02-18 0:48 ` Dave Chinner 0 siblings, 1 reply; 10+ messages in thread From: Michael L. Semon @ 2015-02-17 15:36 UTC (permalink / raw) To: Dave Chinner, Mark Tinguely; +Cc: xfs On 02/16/15 18:17, Dave Chinner wrote: > On Mon, Feb 16, 2015 at 11:35:50AM -0600, Mark Tinguely wrote: >> Thanks Michael, you don't need to hold your test box for me. I do >> have a way to recreate these ABBA AGF buffer allocation deadlocks >> and understand the whys and hows very well. I don't have a community >> way to make a xfstest for it but I think your test is getting close. > > If you know what is causing them, then please explain how it occurs > and how you think it needs to be fixed. Just telling us that you know > something that we don't doesn't help us solve the problem. :( > > In general, the use of the args->firstblock is supposed to avoid the > ABBA locking order issues with multiple allocations in the one > transaction by preventing AG selection loops from looping back into > AGs with a lower index than the first allocation that was made. > > So if you are seeing deadlocks, then it may be that we aren't > following this constraint correctly in all locations.... > > Cheers, > > Dave. Will this be a classic deadlock that will cause problems when trying to kill processes and unmount filesystems? If so, then I was unable to use generic/224 to trigger a deadlock. If not, then I'll need a better way of looking at the problem. The longest generic/224 loop lasted only 3-1/2 hours, though. The fstests enospc group was given some consideration as well. If this issue does not require a lot of files, I might see if fio can be helpful here. Hints on whether to us a fast kernel or a miserably slow kernel would be rather helpful. My test setup is torn because most of the recent warning messages are coming from the CONFIG_XFS_WARN kernels. The i686 Pentium 4 box will be left that way. However, the Core 2 box was configured per Documentation/SubmitChecklist from the kernel source, adding debug XFS and locktorture. The locktorture settings are in flux, exercising spinlocks at present. There was a mild halt in I/O for generic/017, but that was XFS waiting on kmem-something waiting on a kmemleak function. kmemleak was removed, and I'll continue from there. Thanks! Michael _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC 2015-02-17 15:36 ` Michael L. Semon @ 2015-02-18 0:48 ` Dave Chinner 2015-02-18 15:32 ` Michael L. Semon 0 siblings, 1 reply; 10+ messages in thread From: Dave Chinner @ 2015-02-18 0:48 UTC (permalink / raw) To: Michael L. Semon; +Cc: Mark Tinguely, xfs On Tue, Feb 17, 2015 at 10:36:54AM -0500, Michael L. Semon wrote: > On 02/16/15 18:17, Dave Chinner wrote: > > On Mon, Feb 16, 2015 at 11:35:50AM -0600, Mark Tinguely wrote: > >> Thanks Michael, you don't need to hold your test box for me. I do > >> have a way to recreate these ABBA AGF buffer allocation deadlocks > >> and understand the whys and hows very well. I don't have a community > >> way to make a xfstest for it but I think your test is getting close. > > > > If you know what is causing them, then please explain how it occurs > > and how you think it needs to be fixed. Just telling us that you know > > something that we don't doesn't help us solve the problem. :( > > > > In general, the use of the args->firstblock is supposed to avoid the > > ABBA locking order issues with multiple allocations in the one > > transaction by preventing AG selection loops from looping back into > > AGs with a lower index than the first allocation that was made. > > > > So if you are seeing deadlocks, then it may be that we aren't > > following this constraint correctly in all locations.... > > Will this be a classic deadlock that will cause problems when trying to > kill processes and unmount filesystems? If so, then I was unable to use > generic/224 to trigger a deadlock. If not, then I'll need a better way > of looking at the problem. Yes, it will hang the filesystem. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC 2015-02-18 0:48 ` Dave Chinner @ 2015-02-18 15:32 ` Michael L. Semon 0 siblings, 0 replies; 10+ messages in thread From: Michael L. Semon @ 2015-02-18 15:32 UTC (permalink / raw) To: Dave Chinner; +Cc: Mark Tinguely, xfs On 02/17/15 19:48, Dave Chinner wrote: > On Tue, Feb 17, 2015 at 10:36:54AM -0500, Michael L. Semon wrote: >> On 02/16/15 18:17, Dave Chinner wrote: >>> On Mon, Feb 16, 2015 at 11:35:50AM -0600, Mark Tinguely wrote: >>>> Thanks Michael, you don't need to hold your test box for me. I do >>>> have a way to recreate these ABBA AGF buffer allocation deadlocks >>>> and understand the whys and hows very well. I don't have a community >>>> way to make a xfstest for it but I think your test is getting close. >>> >>> If you know what is causing them, then please explain how it occurs >>> and how you think it needs to be fixed. Just telling us that you know >>> something that we don't doesn't help us solve the problem. :( >>> >>> In general, the use of the args->firstblock is supposed to avoid the >>> ABBA locking order issues with multiple allocations in the one >>> transaction by preventing AG selection loops from looping back into >>> AGs with a lower index than the first allocation that was made. >>> >>> So if you are seeing deadlocks, then it may be that we aren't >>> following this constraint correctly in all locations.... >> >> Will this be a classic deadlock that will cause problems when trying to >> kill processes and unmount filesystems? If so, then I was unable to use >> generic/224 to trigger a deadlock. If not, then I'll need a better way >> of looking at the problem. > > Yes, it will hang the filesystem. > > Cheers, > > Dave. Thanks. I'll try again tonight. Last night's attempt was a combination of fio, fsstress, and a shell loop of xfs_io's fcollapse command, all at once on an SSD. At the end of the night, XFS was laughing at me. Therefore, I added the same test on the 3-partition RAID-0 side. This morning, XFS is still laughing at me, but the RAID-0 test is still running. Thanks! Michael _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC 2015-02-12 23:14 [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC Dave Chinner 2015-02-13 23:40 ` Mark Tinguely @ 2015-02-16 13:41 ` Brian Foster 1 sibling, 0 replies; 10+ messages in thread From: Brian Foster @ 2015-02-16 13:41 UTC (permalink / raw) To: Dave Chinner; +Cc: xfs On Fri, Feb 13, 2015 at 10:14:17AM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > 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@gmail.com> > Tested-by: Michael L. Semon <mlsemon35@gmail.com> > Signed-off-by: Dave Chinner <dchinner@redhat.com> > --- Looks fine to me: Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/libxfs/xfs_alloc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c > index 710554c..41e3630 100644 > --- a/fs/xfs/libxfs/xfs_alloc.c > +++ b/fs/xfs/libxfs/xfs_alloc.c > @@ -260,6 +260,7 @@ xfs_alloc_fix_len( > rlen = rlen - (k - args->mod); > else > rlen = rlen - args->prod + (args->mod - k); > + /* casts to (int) catch length underflows */ > if ((int)rlen < (int)args->minlen) > return; > ASSERT(rlen >= args->minlen && rlen <= args->maxlen); > @@ -286,7 +287,8 @@ xfs_alloc_fix_minleft( > if (diff >= 0) > return 1; > 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; > -- > 2.0.0 > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-02-18 15:32 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-02-12 23:14 [PATCH] xfs: xfs_alloc_fix_minleft can underflow near ENOSPC Dave Chinner 2015-02-13 23:40 ` Mark Tinguely 2015-02-14 23:29 ` Dave Chinner 2015-02-16 3:39 ` Michael L. Semon 2015-02-16 17:35 ` Mark Tinguely 2015-02-16 23:17 ` Dave Chinner 2015-02-17 15:36 ` Michael L. Semon 2015-02-18 0:48 ` Dave Chinner 2015-02-18 15:32 ` Michael L. Semon 2015-02-16 13:41 ` Brian Foster
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.