On Mon, 2022-04-18 at 18:25 +0800, Xiubo Li wrote:
> On 4/18/22 6:15 PM, Jeff Layton wrote:
> > On Mon, 2022-04-11 at 17:34 +0800, xiubli@xxxxxxxxxx wrote:
> > > From: Xiubo Li <xiubli@xxxxxxxxxx>
> > >
> > > From the posix and the initial statx supporting commit comments,
> > > the AT_STATX_DONT_SYNC is a lightweight stat flag and the
> > > AT_STATX_FORCE_SYNC is a heaverweight one. And also checked all
> > > the other current usage about these two flags they are all doing
> > > the same, that is only when the AT_STATX_FORCE_SYNC is not set
> > > and the AT_STATX_DONT_SYNC is set will they skip sync retriving
> > > the attributes from storage.
> > >
> > > Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
> > > ---
> > > fs/ceph/inode.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > > index 6788a1f88eb6..1ee6685def83 100644
> > > --- a/fs/ceph/inode.c
> > > +++ b/fs/ceph/inode.c
> > > @@ -2887,7 +2887,7 @@ int ceph_getattr(struct user_namespace *mnt_userns, const struct path *path,
> > > return -ESTALE;
> > >
> > > /* Skip the getattr altogether if we're asked not to sync */
> > > - if (!(flags & AT_STATX_DONT_SYNC)) {
> > > + if ((flags & AT_STATX_SYNC_TYPE) != AT_STATX_DONT_SYNC) {
> > > err = ceph_do_getattr(inode,
> > > statx_to_caps(request_mask, inode->i_mode),
> > > flags & AT_STATX_FORCE_SYNC);
> > I don't get it.
> >
> > The only way I can see that this is a problem is if someone sent down a
> > mask with both DONT_SYNC and FORCE_SYNC set in it, and in that case I
> > don't see that ignoring FORCE_SYNC would be wrong...
> >
> There has 3 cases for the flags:
>
> case1: flags & AT_STATX_SYNC_TYPE == 0
>
> case2: flags & AT_STATX_SYNC_TYPE == AT_STATX_DONT_SYNC
>
> case3: flags & AT_STATX_SYNC_TYPE == AT_STATX_DONT_SYNC |
> AT_STATX_FORCE_SYNC
>
>
> Only in case2, which is only the DONT_SYNC bit is set, will ignore
> calling ceph_do_getattr() here. And for case3 it will ignore the
> DONT_SYNC bit.
>
Sure, but the patch doesn't functionally change the behavior of the
code. It may make the condition more idiomatic to read, but I don't
think there is a bug here.
--
Jeff Layton <jlayton@xxxxxxxxxx>
