On Sun, 7 Apr 2024 at 17:58, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> There is a confusion with fuse_file_uncached_io_{start,end} interface.
> These helpers do two things when called from passthrough open()/release():
> 1. Take/drop negative refcount of fi->iocachectr (inode uncached io mode)
> 2. State change ff->iomode IOM_NONE <-> IOM_UNCACHED (file uncached open)
>
> The calls from parallel dio write path need to take a reference on
> fi->iocachectr, but they should not be changing ff->iomode state,
> because in this case, the fi->iocachectr reference does not stick around
> until file release().
Okay.
>
> Factor out helpers fuse_inode_uncached_io_{start,end}, to be used from
> parallel dio write path and rename fuse_file_*cached_io_{start,end}
> helpers to fuse_file_*cached_io_{open,release} to clarify the difference.
>
> Add a check of ff->iomode in mmap(), so that fuse_file_cached_io_open()
> is called only on first mmap of direct_io file.
Is this supposed to be an optimization? AFAICS it's wrong, because it
moves the check outside of any relevant locks.
> @@ -56,8 +57,7 @@ int fuse_file_cached_io_start(struct inode *inode, struct fuse_file *ff)
> return -ETXTBSY;
> }
>
> - WARN_ON(ff->iomode == IOM_UNCACHED);
> - if (ff->iomode == IOM_NONE) {
> + if (!WARN_ON(ff->iomode != IOM_NONE)) {
This double negation is ugly. Just let the compiler optimize away the
second comparison.
Thanks,
Miklos
