Re: [RFC PATCH] generic: test negative timespecs are accurate

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Tue, Sep 10, 2024 at 05:04:03PM +0200, Alyssa Ross wrote:
> Zorro Lang <zlang@xxxxxxxxxx> writes:
> 
> > On Tue, Sep 10, 2024 at 12:02:05PM +0200, Alyssa Ross wrote:
> >> Zorro Lang <zlang@xxxxxxxxxx> writes:
> >> 
> >> > On Sat, Sep 07, 2024 at 05:45:28PM +0200, Alyssa Ross wrote:
> >> >> Link: https://github.com/koverstreet/bcachefs/issues/743
> >> >
> >> > Great, a bcachefs regression test case :)
> >> >
> >> > Can you add a bit more details in commit log, not only a link.
> >> >
> >> >> Signed-off-by: Alyssa Ross <hi@xxxxxxxxx>
> >> >> ---
> >> >> This is an adapted version of generic/258, but it tests that the stored 
> >> >> timestamp is accurate to the second, rather than just testing the 
> >> >> timestamp remains negative.  I created a new test rather than just
> >> >> making 258 more precise, because I understand that there may be
> >> >> filesystems that don't store timestamps that accurately by design.
> >> >> As an example, I've heard that FAT only has 2 second precision, so this
> >> >> patch is an RFC because I'm not sure how I should write a _require
> >> >> function (if at all) that restricts the test to filesystems that are
> >> >> expected to be able to do this.
> >> >> 
> >> >>  tests/generic/363     | 26 ++++++++++++++++++++++++++
> >> >>  tests/generic/363.out |  2 ++
> >> >
> >> > The g/363 has been taken, please rebase to latest for-next branch.
> >> > You can use `xfstests/tools/mvtest generic/363 generic/365` to change
> >> > the case number, before rebasing.
> >> >
> >> >>  2 files changed, 28 insertions(+)
> >> >>  create mode 100755 tests/generic/363
> >> >>  create mode 100644 tests/generic/363.out
> >> >> 
> >> >> diff --git a/tests/generic/363 b/tests/generic/363
> >> >> new file mode 100755
> >> >> index 00000000..50459d01
> >> >> --- /dev/null
> >> >> +++ b/tests/generic/363
> >> >> @@ -0,0 +1,26 @@
> >> >> +#! /bin/bash
> >> >> +# SPDX-License-Identifier: GPL-2.0
> >> >> +# Copyright (c) 2011 Red Hat, Inc.  All Rights Reserved.
> >> >> +# Copyright (c) 2024 Alyssa Ross.  All Rights Reserved.
> >> >> +#
> >> >> +# FS QA Test 363
> >> >> +#
> >> >> +# Test timestamps prior to epoch with nanosecond components are
> >> >> +# accurate to the second.
> >> >> +# bcachefs was slightly off.
> >> >> +#
> >> >> +. ./common/preamble
> >> >> +_begin_fstest auto quick bigtime
> >> >> +
> >> >
> >> > As this's a bcachefs regression test:
> >> > https://lore.kernel.org/linux-bcachefs/20240907160024.605850-3-hi@xxxxxxxxx/
> >> >
> >> > So better to mark as:
> >> > if [ "$FSTYP" = "bcachefs" ];then
> >> > 	_fixed_by_kernel_commit xxxxxxxxxxxx "bcachefs: Fix negative timespecs"
> >> > fi
> >> > (replace the xxxxxxx if it's merged on mainline linux)
> >> >
> >> > Others looks good to me, with above changes, I'd like to
> >> >
> >> > Reviewed-by: Zorro Lang <zlang@xxxxxxxxxx>
> >> 
> >> Thanks!  So you don't think the test needs to express any extra
> >> requirement that filesystems support to-the-second precision?
> >
> > Oh, does this test needs 1 second precision? I thought you concerned some
> > filesystems cannot be the epoch you set. The _require_negative_timestamps
> > will skip the test from exfat and ceph. It doesn't help the FAT, maybe you
> > want to add vfat to _require_negative_timestamps.
> >
> > I thought you might want to output the stored timestamp (accurate to the
> > second) to reproduce this bug, if not, why you hope to write this new one,
> > not use the g/258 to be the reproducer directly?
> 
> I'm sorry, I don't understand the question.  I explained why I created a
> new test in my commentary — g/258 doesn't require 1 second precision,
> and this test does.

Sorry, I mixed several questions together. I mean:

1) Can g/258 reproduce this bug. If not, that means you need the accurate
"-14245441" output, which might break the test on fs not support to-the-second
precision, right?

2) If the 1) is yes. Then I think this case doesn't need to care about
"fs not support to-the-second", as you say you don't have a good way to
"_require" that.

3) If you agree 2). I think most of filesystems (fstests supports) support
the to-the-second precision. And the _require_negative_timestamps() helper
can help to skip this test from some improper filesystems for this test.
If you concern vfat, you can add it beside the exfat in that helper.

4) If the 1) is right, the fs which doesn't support to-the-second precision
also can be added into the blacklist of "_supported_fs" now or later, e.g.
_supported_fs ^cifs.

Hope I explain clearly this time :)

Thanks,
Zorro





[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux