On Wed, Jul 15, 2020 at 6:34 PM Jan Kara <jack@xxxxxxx> wrote:
>
> On Wed 08-07-20 14:11:55, Amir Goldstein wrote:
> > The fanotify_fh struct has an inline buffer of size 12 which is enough
> > to store the most common local filesystem file handles (e.g. ext4, xfs).
> > For file handles that do not fit in the inline buffer (e.g. btrfs), an
> > external buffer is allocated to store the file handle.
> >
> > When allocating a variable size fanotify_name_event, there is no point
> > in allocating also an external fh buffer when file handle does not fit
> > in the inline buffer.
> >
> > Check required size for encoding fh, preallocate an event buffer
> > sufficient to contain both file handle and name and store the name after
> > the file handle.
> >
> > At this time, when not reporting name in event, we still allocate
> > the fixed size fanotify_fid_event and an external buffer for large
> > file handles, but fanotify_alloc_name_event() has already been prepared
> > to accept a NULL file_name.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>
> Just one tiny nit below:
>
> > @@ -305,27 +323,34 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
> > * Return 0 on failure to encode.
> > */
> > static int fanotify_encode_fh(struct fanotify_fh *fh, struct inode *inode,
> > - gfp_t gfp)
> > + unsigned int fh_len, gfp_t gfp)
> > {
> > - int dwords, type, bytes = 0;
> > + int dwords, bytes, type = 0;
> > char *ext_buf = NULL;
> > void *buf = fh->buf;
> > int err;
> >
> > fh->type = FILEID_ROOT;
> > fh->len = 0;
> > + fh->flags = 0;
> > if (!inode)
> > return 0;
> >
> > - dwords = 0;
> > + /*
> > + * !gpf means preallocated variable size fh, but fh_len could
> > + * be zero in that case if encoding fh len failed.
> > + */
> > err = -ENOENT;
> > - type = exportfs_encode_inode_fh(inode, NULL, &dwords, NULL);
> > - if (!dwords)
> > + if (!gfp)
> > + bytes = fh_len;
> > + else
> > + bytes = fanotify_encode_fh_len(inode);
>
> Any reason why proper fh len is not passed in by both callers?
No good reason.
It's just how the function evolved and I missed this simplification.
> We could then get rid of this 'if' and 'bytes' variable.
Yap. sounds good.
I will test and push the branches.
Let me know if you want me to re-post anything.
Thanks,
Amir.
