No objections on my part as far as adding 'if (info & SACL_SECINFO) return'. I had originally those flags sent from the caller, but that was confusing at the top level (e.g. in cifsacl.c), so I have opted to passing only "extra" flags ("additional" was already taken). ------------- ... > > --- a/fs/cifs/smb2ops.c > > +++ b/fs/cifs/smb2ops.c > > @@ -3315,9 +3315,9 @@ get_smb2_acl(struct cifs_sb_info *cifs_sb, > > struct cifs_ntsd *pntsd = NULL; > > struct cifsFileInfo *open_file = NULL; > > > > - if (inode) > > + if (inode && !(info & SACL_SECINFO)) > > open_file = find_readable_file(CIFS_I(inode), true); > > - if (!open_file) > > + if (!open_file || (info & SACL_SECINFO)) > > return get_smb2_acl_by_path(cifs_sb, path, pacllen, info); > > > > pntsd = get_smb2_acl_by_fid(cifs_sb, &open_file->fid, pacllen, info); > Why not have an if (info & SACL_SECINFO) return > get_smb2_acl_by_path... right at the beginning? > Looks cleaner that way IMO. > ... > > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > > index 0aeb63694306..b207e1eb6803 100644 > > --- a/fs/cifs/smb2pdu.c > > +++ b/fs/cifs/smb2pdu.c > > @@ -3472,8 +3472,10 @@ SMB311_posix_query_info(const unsigned int xid, struct cifs_tcon *tcon, > > int > > SMB2_query_acl(const unsigned int xid, struct cifs_tcon *tcon, > > u64 persistent_fid, u64 volatile_fid, > > - void **data, u32 *plen, u32 additional_info) > > + void **data, u32 *plen, u32 extra_info) > > { > > + __u32 additional_info = OWNER_SECINFO | GROUP_SECINFO | DACL_SECINFO | > > + extra_info; > I still prefer having these flags set by the caller. That way, the > caller has the flexibility to query only the subset needed. > -- > -Shyam