On Fri, Nov 20, 2020 at 07:58:38PM -0500, Jeff Layton wrote:
> On Fri, 2020-11-20 at 17:39 -0500, J. Bruce Fields wrote:
> > diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
> > index 465fd9e048d4..b950fac3d7df 100644
> > --- a/fs/xfs/xfs_export.c
> > +++ b/fs/xfs/xfs_export.c
> > @@ -16,6 +16,7 @@
> > #include "xfs_inode_item.h"
> > #include "xfs_icache.h"
> > #include "xfs_pnfs.h"
> > +#include <linux/iversion.h>
> >
> >
> >
> >
> > /*
> > * Note that we only accept fileids which are long enough rather than allow
> > @@ -234,4 +235,5 @@ const struct export_operations xfs_export_operations = {
> > .map_blocks = xfs_fs_map_blocks,
> > .commit_blocks = xfs_fs_commit_blocks,
> > #endif
> > + .fetch_iversion = generic_fetch_iversion,
> > };
>
> It seems a little weird to call a static inline here. I imagine that
> means the compiler has to add a duplicate inline in every .o file that
> does this? It may be cleaner to move generic_fetch_iversion into
> fs/libfs.c so we only have one copy of it.
OK.
(To be honest, I was a little suprised this worked.)
--b.
>
> > diff --git a/include/linux/iversion.h b/include/linux/iversion.h
> > index 3bfebde5a1a6..ded74523c8a6 100644
> > --- a/include/linux/iversion.h
> > +++ b/include/linux/iversion.h
> > @@ -328,6 +328,32 @@ inode_query_iversion(struct inode *inode)
> > return cur >> I_VERSION_QUERIED_SHIFT;
> > }
> >
> >
> >
> >
> > +/*
> > + * We could use i_version alone as the NFSv4 change attribute. However,
> > + * i_version can go backwards after a reboot. On its own that doesn't
> > + * necessarily cause a problem, but if i_version goes backwards and then
> > + * is incremented again it could reuse a value that was previously used
> > + * before boot, and a client who queried the two values might
> > + * incorrectly assume nothing changed.
> > + *
> > + * By using both ctime and the i_version counter we guarantee that as
> > + * long as time doesn't go backwards we never reuse an old value.
> > + *
> > + * A filesystem that has an on-disk boot counter or similar might prefer
> > + * to use that to avoid the risk of the change attribute going backwards
> > + * if system time is set backwards.
> > + */
> > +static inline u64 generic_fetch_iversion(struct inode *inode)
> > +{
> > + u64 chattr;
> > +
> > + chattr = inode->i_ctime.tv_sec;
> > + chattr <<= 30;
> > + chattr += inode->i_ctime.tv_nsec;
> > + chattr += inode_query_iversion(inode);
> > + return chattr;
> > +}
> > +
> > /*
> > * For filesystems without any sort of change attribute, the best we can
> > * do is fake one up from the ctime:
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
>
--
Linux-cachefs mailing list
Linux-cachefs@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/linux-cachefs
