On 3/16/25 11:40 AM, Darrick J. Wong wrote: > On Sun, Mar 16, 2025 at 10:48:50AM -0500, Eric Sandeen wrote: >> On 3/16/25 9:54 AM, Zorro Lang wrote: >>> On Mon, Mar 10, 2025 at 01:29:09PM -0500, Eric Sandeen wrote: >>>> sparse points out that lots of things in random.c could be static, >>>> and upon doing so we realize that nothing in this file is used. >>>> Which is unsurprising since these are all part of the standard >>>> C library ... so just remove the file. >>>> >>>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> >>>> Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx> >>>> Reviewed-by: Christoph Hellwig <hch@xxxxxx> >>>> --- >>> >>> Hi Eric, >>> >>> When I did the fstests regression test this weekend, I found a regression >>> failure on generic/007 (diff output): >>> >>> --- /dev/fd/63 2025-03-15 13:31:35.044534292 -0400 >>> +++ generic/007.out.bad 2025-03-15 13:31:35.002455111 -0400 >>> @@ -14,9 +14,9 @@ >>> ......................................................................... >>> ......................................................................... >>> .................................................... >>> -creates: 18736 OK, 18802 EEXIST ( 37538 total, 50% EEXIST) >>> -removes: 18675 OK, 19927 ENOENT ( 38602 total, 51% ENOENT) >>> -lookups: 12000 OK, 11860 ENOENT ( 23860 total, 49% ENOENT) >>> -total : 49411 OK, 50589 w/error (100000 total, 50% w/error) >>> +creates: 18839 OK, 18890 EEXIST ( 37729 total, 50% EEXIST) >>> +removes: 18783 OK, 19951 ENOENT ( 38734 total, 51% ENOENT) >>> +lookups: 11858 OK, 11679 ENOENT ( 23537 total, 49% ENOENT) >>> +total : 49480 OK, 50520 w/error (100000 total, 50% w/error) >>> >>> -cleanup: 61 removes >>> +cleanup: 56 removes >>> >>> By bisecting, the first failed commit is this patch. After removing >>> the fstests internal lib/random.c, the output of src/nametest.c is >>> changed too, that breaks the g/007 (xfs/188 maybe too) test. >>> >>> It fails on all filesystems (e.g. xfs, ext2/3/4, btrfs, tmpfs, nfs, >>> cifs etc). I'll defer the release of this week (03.16), hope we can >>> fix this regression next week :) >> >> Oh no, I'm sorry. I thought that if this stuff was never used it'd >> be safe to just yank, but I clearly must have missed something. > > Ahahaha, it's linked into libtest.a, which is then linked into every > fstests binary. Therefore, any binary calling srandom and random get > this bespoke version instead of the one in the C library. From the > looks of it, the RNG code itself runs the same operations in the same > order every time, which is why you can pass a seed to fsx/fsstress to > get the exact same sequence of operations. > > Maybe lib/random.c should have its comment updated? Clearly two > reviewers, myself, and the patch author missed this point: > > /* > * modified by dxm@xxxxxxx so that this file acts as a drop in replacement > * for srandom and random. > */ > > Because that doesn't really explain /why/ the code is needed as a drop > in replacement. How about: > > /* > * modified by dxm@xxxxxxx so that this file acts as a drop in replacement > * for srandom and random. fstests programs rely on the exact sequence > * of integers generated by these functions for reproducibility and the > * golden output, which is why we override the C library. > */ Yeah, I agree. We need a stable random.c copy for reproducibility on tests that expect it. :( Sorry about that, it's my near-constant reminder that even "trivial" changes need testing :( And I'll figure out how to make sparse happier about it. -Eric > --D > >> It's probably best to just revert/remove this patch for now, so it >> doesn't delay any release. >> >> -Eric >> >
