diff mbox series

[1/3] fuse: fix wrong ff->iomode state changes from parallel dio write

Message ID 20240407155758.575216-2-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show
Series FUSE passthrough fixes | expand

Commit Message

Amir Goldstein April 7, 2024, 3:57 p.m. UTC 
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().

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.

Fixes: 205c1d802683 ("fuse: allow parallel dio writes with FUSE_DIRECT_IO_ALLOW_MMAP")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/fuse/file.c   | 18 +++++++++------
 fs/fuse/fuse_i.h |  7 +++---
 fs/fuse/inode.c  |  1 +
 fs/fuse/iomode.c | 59 ++++++++++++++++++++++++++++++++----------------
 4 files changed, 56 insertions(+), 29 deletions(-)

Comments

Miklos Szeredi April 9, 2024, 1:33 p.m. UTC | #1 
On Sun, 7 Apr 2024 at 17:58, Amir Goldstein <amir73il@gmail.com> 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
Amir Goldstein April 9, 2024, 3:10 p.m. UTC | #2 
On Tue, Apr 9, 2024 at 4:33 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Sun, 7 Apr 2024 at 17:58, Amir Goldstein <amir73il@gmail.com> 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?

No.
The reason I did this is because I wanted to differentiate
the refcount semantics (start/end)
from the state semantics (open/release)
and to make it clearer that there is only one state change
and refcount increment on the first mmap().

> AFAICS it's wrong, because it
> moves the check outside of any relevant locks.
>

Aren't concurrent mmap serialized on some lock?

Anyway, I think that the only "bug" that this can trigger is the
WARN_ON(ff->iomode != IOM_NONE)
so if we ....

>
> > @@ -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.

...drop this change, we should be good.

If you agree, do you need me to re-post?

Thanks,
Amir.
Miklos Szeredi April 9, 2024, 3:32 p.m. UTC | #3 
On Tue, 9 Apr 2024 at 17:10, Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Tue, Apr 9, 2024 at 4:33 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Sun, 7 Apr 2024 at 17:58, Amir Goldstein <amir73il@gmail.com> 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?
>
> No.
> The reason I did this is because I wanted to differentiate
> the refcount semantics (start/end)
> from the state semantics (open/release)
> and to make it clearer that there is only one state change
> and refcount increment on the first mmap().
>
> > AFAICS it's wrong, because it
> > moves the check outside of any relevant locks.
> >
>
> Aren't concurrent mmap serialized on some lock?

Only on current->mm, which doesn't serialize mmaps of the same file in
different processes.

>
> Anyway, I think that the only "bug" that this can trigger is the
> WARN_ON(ff->iomode != IOM_NONE)
> so if we ....
>
> >
> > > @@ -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.
>
> ...drop this change, we should be good.
>
> If you agree, do you need me to re-post?

Okay, but then what's the point of the unlocked check?

Thanks,
Miklos
Amir Goldstein April 9, 2024, 4:18 p.m. UTC | #4 
On Tue, Apr 9, 2024 at 6:32 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 9 Apr 2024 at 17:10, Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Tue, Apr 9, 2024 at 4:33 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > >
> > > On Sun, 7 Apr 2024 at 17:58, Amir Goldstein <amir73il@gmail.com> 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?
> >
> > No.
> > The reason I did this is because I wanted to differentiate
> > the refcount semantics (start/end)
> > from the state semantics (open/release)
> > and to make it clearer that there is only one state change
> > and refcount increment on the first mmap().
> >
> > > AFAICS it's wrong, because it
> > > moves the check outside of any relevant locks.
> > >
> >
> > Aren't concurrent mmap serialized on some lock?
>
> Only on current->mm, which doesn't serialize mmaps of the same file in
> different processes.
>
> >
> > Anyway, I think that the only "bug" that this can trigger is the
> > WARN_ON(ff->iomode != IOM_NONE)
> > so if we ....
> >
> > >
> > > > @@ -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.
> >
> > ...drop this change, we should be good.
> >
> > If you agree, do you need me to re-post?
>
> Okay, but then what's the point of the unlocked check?

As I wrote, I just did it to emphasize the open-once
semantics.
If you do not like the unlocked check, feel free to remove it.

Thanks,
Amir.
Amir Goldstein April 13, 2024, 6:50 a.m. UTC | #5 
On Tue, Apr 9, 2024 at 7:18 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Tue, Apr 9, 2024 at 6:32 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Tue, 9 Apr 2024 at 17:10, Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > On Tue, Apr 9, 2024 at 4:33 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > >
> > > > On Sun, 7 Apr 2024 at 17:58, Amir Goldstein <amir73il@gmail.com> 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?
> > >
> > > No.
> > > The reason I did this is because I wanted to differentiate
> > > the refcount semantics (start/end)
> > > from the state semantics (open/release)
> > > and to make it clearer that there is only one state change
> > > and refcount increment on the first mmap().
> > >
> > > > AFAICS it's wrong, because it
> > > > moves the check outside of any relevant locks.
> > > >
> > >
> > > Aren't concurrent mmap serialized on some lock?
> >
> > Only on current->mm, which doesn't serialize mmaps of the same file in
> > different processes.
> >
> > >
> > > Anyway, I think that the only "bug" that this can trigger is the
> > > WARN_ON(ff->iomode != IOM_NONE)
> > > so if we ....
> > >
> > > >
> > > > > @@ -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.
> > >
> > > ...drop this change, we should be good.
> > >
> > > If you agree, do you need me to re-post?
> >
> > Okay, but then what's the point of the unlocked check?
>
> As I wrote, I just did it to emphasize the open-once
> semantics.
> If you do not like the unlocked check, feel free to remove it.

Miklos,

I see that you removed the unlocked check in the fix staged for-next.
Please also remove this from commit message:

    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.

Thanks,
Amir.
Miklos Szeredi April 15, 2024, 8:14 a.m. UTC | #6 
On Sat, 13 Apr 2024 at 08:50, Amir Goldstein <amir73il@gmail.com> wrote:

> I see that you removed the unlocked check in the fix staged for-next.
> Please also remove this from commit message:
>
>     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.

Done.

Thanks,
Miklos
diff mbox series

Patch

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index a56e7bffd000..fcf20b304093 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1362,7 +1362,7 @@  static void fuse_dio_lock(struct kiocb *iocb, struct iov_iter *from,
 			  bool *exclusive)
 {
 	struct inode *inode = file_inode(iocb->ki_filp);
-	struct fuse_file *ff = iocb->ki_filp->private_data;
+	struct fuse_inode *fi = get_fuse_inode(inode);
 
 	*exclusive = fuse_dio_wr_exclusive_lock(iocb, from);
 	if (*exclusive) {
@@ -1377,7 +1377,7 @@  static void fuse_dio_lock(struct kiocb *iocb, struct iov_iter *from,
 		 * have raced, so check it again.
 		 */
 		if (fuse_io_past_eof(iocb, from) ||
-		    fuse_file_uncached_io_start(inode, ff, NULL) != 0) {
+		    fuse_inode_uncached_io_start(fi, NULL) != 0) {
 			inode_unlock_shared(inode);
 			inode_lock(inode);
 			*exclusive = true;
@@ -1388,13 +1388,13 @@  static void fuse_dio_lock(struct kiocb *iocb, struct iov_iter *from,
 static void fuse_dio_unlock(struct kiocb *iocb, bool exclusive)
 {
 	struct inode *inode = file_inode(iocb->ki_filp);
-	struct fuse_file *ff = iocb->ki_filp->private_data;
+	struct fuse_inode *fi = get_fuse_inode(inode);
 
 	if (exclusive) {
 		inode_unlock(inode);
 	} else {
 		/* Allow opens in caching mode after last parallel dio end */
-		fuse_file_uncached_io_end(inode, ff);
+		fuse_inode_uncached_io_end(fi);
 		inode_unlock_shared(inode);
 	}
 }
@@ -2574,10 +2574,14 @@  static int fuse_file_mmap(struct file *file, struct vm_area_struct *vma)
 		 * First mmap of direct_io file enters caching inode io mode.
 		 * Also waits for parallel dio writers to go into serial mode
 		 * (exclusive instead of shared lock).
+		 * After first mmap, the inode stays in caching io mode until
+		 * the direct_io file release.
 		 */
-		rc = fuse_file_cached_io_start(inode, ff);
-		if (rc)
-			return rc;
+		if (READ_ONCE(ff->iomode) != IOM_CACHED) {
+			rc = fuse_file_cached_io_open(inode, ff);
+			if (rc)
+				return rc;
+		}
 	}
 
 	if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index b24084b60864..f23919610313 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1394,9 +1394,10 @@  int fuse_fileattr_set(struct mnt_idmap *idmap,
 		      struct dentry *dentry, struct fileattr *fa);
 
 /* iomode.c */
-int fuse_file_cached_io_start(struct inode *inode, struct fuse_file *ff);
-int fuse_file_uncached_io_start(struct inode *inode, struct fuse_file *ff, struct fuse_backing *fb);
-void fuse_file_uncached_io_end(struct inode *inode, struct fuse_file *ff);
+int fuse_file_cached_io_open(struct inode *inode, struct fuse_file *ff);
+int fuse_inode_uncached_io_start(struct fuse_inode *fi,
+				 struct fuse_backing *fb);
+void fuse_inode_uncached_io_end(struct fuse_inode *fi);
 
 int fuse_file_io_open(struct file *file, struct inode *inode);
 void fuse_file_io_release(struct fuse_file *ff, struct inode *inode);
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 3a5d88878335..99e44ea7d875 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -175,6 +175,7 @@  static void fuse_evict_inode(struct inode *inode)
 		}
 	}
 	if (S_ISREG(inode->i_mode) && !fuse_is_bad(inode)) {
+		WARN_ON(fi->iocachectr != 0);
 		WARN_ON(!list_empty(&fi->write_files));
 		WARN_ON(!list_empty(&fi->queued_writes));
 	}
diff --git a/fs/fuse/iomode.c b/fs/fuse/iomode.c
index c653ddcf0578..1519f895f0a9 100644
--- a/fs/fuse/iomode.c
+++ b/fs/fuse/iomode.c
@@ -21,12 +21,13 @@  static inline bool fuse_is_io_cache_wait(struct fuse_inode *fi)
 }
 
 /*
- * Start cached io mode.
+ * Called on cached file open() and on first mmap() of direct_io file.
+ * Takes cached_io inode mode reference to be dropped on file release.
  *
  * Blocks new parallel dio writes and waits for the in-progress parallel dio
  * writes to complete.
  */
-int fuse_file_cached_io_start(struct inode *inode, struct fuse_file *ff)
+int fuse_file_cached_io_open(struct inode *inode, struct fuse_file *ff)
 {
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
@@ -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)) {
 		ff->iomode = IOM_CACHED;
 		if (fi->iocachectr == 0)
 			set_bit(FUSE_I_CACHE_IO_MODE, &fi->state);
@@ -67,10 +67,9 @@  int fuse_file_cached_io_start(struct inode *inode, struct fuse_file *ff)
 	return 0;
 }
 
-static void fuse_file_cached_io_end(struct inode *inode, struct fuse_file *ff)
+static void fuse_file_cached_io_release(struct fuse_file *ff,
+					struct fuse_inode *fi)
 {
-	struct fuse_inode *fi = get_fuse_inode(inode);
-
 	spin_lock(&fi->lock);
 	WARN_ON(fi->iocachectr <= 0);
 	WARN_ON(ff->iomode != IOM_CACHED);
@@ -82,9 +81,8 @@  static void fuse_file_cached_io_end(struct inode *inode, struct fuse_file *ff)
 }
 
 /* Start strictly uncached io mode where cache access is not allowed */
-int fuse_file_uncached_io_start(struct inode *inode, struct fuse_file *ff, struct fuse_backing *fb)
+int fuse_inode_uncached_io_start(struct fuse_inode *fi, struct fuse_backing *fb)
 {
-	struct fuse_inode *fi = get_fuse_inode(inode);
 	struct fuse_backing *oldfb;
 	int err = 0;
 
@@ -99,9 +97,7 @@  int fuse_file_uncached_io_start(struct inode *inode, struct fuse_file *ff, struc
 		err = -ETXTBSY;
 		goto unlock;
 	}
-	WARN_ON(ff->iomode != IOM_NONE);
 	fi->iocachectr--;
-	ff->iomode = IOM_UNCACHED;
 
 	/* fuse inode holds a single refcount of backing file */
 	if (!oldfb) {
@@ -115,15 +111,29 @@  int fuse_file_uncached_io_start(struct inode *inode, struct fuse_file *ff, struc
 	return err;
 }
 
-void fuse_file_uncached_io_end(struct inode *inode, struct fuse_file *ff)
+/* Takes uncached_io inode mode reference to be dropped on file release */
+static int fuse_file_uncached_io_open(struct inode *inode,
+				      struct fuse_file *ff,
+				      struct fuse_backing *fb)
 {
 	struct fuse_inode *fi = get_fuse_inode(inode);
+	int err;
+
+	err = fuse_inode_uncached_io_start(fi, fb);
+	if (err)
+		return err;
+
+	WARN_ON(ff->iomode != IOM_NONE);
+	ff->iomode = IOM_UNCACHED;
+	return 0;
+}
+
+void fuse_inode_uncached_io_end(struct fuse_inode *fi)
+{
 	struct fuse_backing *oldfb = NULL;
 
 	spin_lock(&fi->lock);
 	WARN_ON(fi->iocachectr >= 0);
-	WARN_ON(ff->iomode != IOM_UNCACHED);
-	ff->iomode = IOM_NONE;
 	fi->iocachectr++;
 	if (!fi->iocachectr) {
 		wake_up(&fi->direct_io_waitq);
@@ -134,6 +144,15 @@  void fuse_file_uncached_io_end(struct inode *inode, struct fuse_file *ff)
 		fuse_backing_put(oldfb);
 }
 
+/* Drop uncached_io reference from passthrough open */
+static void fuse_file_uncached_io_release(struct fuse_file *ff,
+					  struct fuse_inode *fi)
+{
+	WARN_ON(ff->iomode != IOM_UNCACHED);
+	ff->iomode = IOM_NONE;
+	fuse_inode_uncached_io_end(fi);
+}
+
 /*
  * Open flags that are allowed in combination with FOPEN_PASSTHROUGH.
  * A combination of FOPEN_PASSTHROUGH and FOPEN_DIRECT_IO means that read/write
@@ -163,7 +182,7 @@  static int fuse_file_passthrough_open(struct inode *inode, struct file *file)
 		return PTR_ERR(fb);
 
 	/* First passthrough file open denies caching inode io mode */
-	err = fuse_file_uncached_io_start(inode, ff, fb);
+	err = fuse_file_uncached_io_open(inode, ff, fb);
 	if (!err)
 		return 0;
 
@@ -216,7 +235,7 @@  int fuse_file_io_open(struct file *file, struct inode *inode)
 	if (ff->open_flags & FOPEN_PASSTHROUGH)
 		err = fuse_file_passthrough_open(inode, file);
 	else
-		err = fuse_file_cached_io_start(inode, ff);
+		err = fuse_file_cached_io_open(inode, ff);
 	if (err)
 		goto fail;
 
@@ -236,8 +255,10 @@  int fuse_file_io_open(struct file *file, struct inode *inode)
 /* No more pending io and no new io possible to inode via open/mmapped file */
 void fuse_file_io_release(struct fuse_file *ff, struct inode *inode)
 {
+	struct fuse_inode *fi = get_fuse_inode(inode);
+
 	/*
-	 * Last parallel dio close allows caching inode io mode.
+	 * Last passthrough file close allows caching inode io mode.
 	 * Last caching file close exits caching inode io mode.
 	 */
 	switch (ff->iomode) {
@@ -245,10 +266,10 @@  void fuse_file_io_release(struct fuse_file *ff, struct inode *inode)
 		/* Nothing to do */
 		break;
 	case IOM_UNCACHED:
-		fuse_file_uncached_io_end(inode, ff);
+		fuse_file_uncached_io_release(ff, fi);
 		break;
 	case IOM_CACHED:
-		fuse_file_cached_io_end(inode, ff);
+		fuse_file_cached_io_release(ff, fi);
 		break;
 	}
 }