Re: [Cluster-devel] [PATCH v7 12/13] ext4: switch to multigrain timestamps

On Tue 19-09-23 15:05:24, Xi Ruoyao wrote:
> On Mon, 2023-08-07 at 15:38 -0400, Jeff Layton wrote:
> > Enable multigrain timestamps, which should ensure that there is an
> > apparent change to the timestamp whenever it has been written after
> > being actively observed via getattr.
> >=20
> > For ext4, we only need to enable the FS_MGTIME flag.
>=20
> Hi Jeff,
>=20
> This patch causes a gnulib test failure:
>=20
> $ ~/sources/lfs/grep-3.11/gnulib-tests/test-stat-time
> test-stat-time.c:141: assertion 'statinfo[0].st_mtime < statinfo[2].st_mt=
ime || (statinfo[0].st_mtime =3D=3D statinfo[2].st_mtime && (get_stat_mtime=
_ns (&statinfo[0]) < get_stat_mtime_ns (&statinfo[2])))' failed
> Aborted (core dumped)
>=20
> The source code of the test:
> https://git.savannah.gnu.org/cgit/gnulib.git/tree/tests/test-stat-time.c
>=20
> Is this an expected change?

Kind of yes. The test first tries to estimate filesystem timestamp
granularity in nap() function - due to this patch, the detected granularity
will likely be 1 ns so effectively all the test calls will happen
immediately one after another. But we don't bother setting the timestamps
with more than 1 jiffy (usually 4 ms) precision unless we think someone is
watching. So as a result timestamps of all stamp1 and stamp2 files are
going to be equal which makes the test fail.

The ultimate problem is that a sequence like:

write(f1)
stat(f2)
write(f2)
stat(f2)
write(f1)
stat(f1)

can result in f1 timestamp to be (slightly) lower than the final f2
timestamp because the second write to f1 didn't bother updating the
timestamp. That can indeed be a bit confusing to programs if they compare
timestamps between two files. Jeff?

=09=09=09=09=09=09=09=09Honza


> > Acked-by: Theodore Ts'o <[email protected]>
> > Reviewed-by: Jan Kara <[email protected]>
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> > =A0fs/ext4/super.c | 2 +-
> > =A01 file changed, 1 insertion(+), 1 deletion(-)
> >=20
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index b54c70e1a74e..cb1ff47af156 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -7279,7 +7279,7 @@ static struct file_system_type ext4_fs_type =3D {
> > =A0=09.init_fs_context=09=3D ext4_init_fs_context,
> > =A0=09.parameters=09=09=3D ext4_param_specs,
> > =A0=09.kill_sb=09=09=3D kill_block_super,
> > -=09.fs_flags=09=09=3D FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
> > +=09.fs_flags=09=09=3D FS_REQUIRES_DEV | FS_ALLOW_IDMAP |
> > FS_MGTIME,
> > =A0};
> > =A0MODULE_ALIAS_FS("ext4");
> > =A0
> >=20
>=20
--=20
Jan Kara <[email protected]>
SUSE Labs, CR