[Devel] [PATCH VZ10 v4] fs/fuse: align fuse_create_open()'s open path with fuse_open()
Alexey Kuznetsov
kuznet at virtuozzo.com
Mon Jun 29 16:07:25 MSK 2026
Acknowledged
On Mon, Jun 29, 2026 at 7:41 PM Liu Kui <kui.liu at virtuozzo.com> wrote:
>
> fuse_create_open()'s close_wait branch was not correct; in particular the
> fput() in its error path was wrong. fuse_open()'s close_wait branch should
> apply instead.
>
> The two sites differ in one important way, though: fuse_open() runs the
> block before FMODE_OPENED is set, where fuse_sync_release() is the correct
> cleanup on error, whereas fuse_create_open() runs it after FMODE_OPENED is
> set, where fuse_sync_release() must not be called. To unify them, refactor
> the sequence into fuse_open_close_wait() and call it from
> fuse_finish_open(), so the block runs before FMODE_OPENED on both paths.
> The error then propagates to the caller and the block itself no longer
> needs to call fuse_sync_release().
>
> Add a per-open FUSE_S_OPEN_INODE_LOCKED flag telling fuse_open_close_wait()
> that the caller already holds the inode lock, so it is not re-acquired and
> deadlocks on the atomic O_TRUNC path.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-136027
> Signed-off-by: Liu Kui <kui.liu at virtuozzo.com>
> ---
> Changes in v4:
> - Rename FUSE_S_INODE_LOCKED to FUSE_S_OPEN_INODE_LOCKED to make clear
> the flag applies to the open path.
> - Introduce a local lock_inode helper in fuse_open() for guarding
> against changes of lock condition from mainline in the future.
>
> Changes in v3:
> - Call fuse_open_close_wait() from fuse_finish_open() instead of from
> the fuse_open() and fuse_create_open() call sites, so the sequence
> runs before FMODE_OPENED is set on both paths.
> - Drop the fuse_sync_release() call inside fuse_open_close_wait(); the
> error now propagates to the caller, which releases the file before
> FMODE_OPENED is set.
> - Make fuse_open_close_wait() static and drop its fuse_i.h prototype.
> - Acquire the inode lock in fuse_open_close_wait() only when the caller
> does not already hold it, tracked by a per-open FUSE_S_INODE_LOCKED
> flag, so the atomic O_TRUNC path (which already holds it) does not
> self-deadlock.
>
> Changes in v2:
> - Factor the close_wait open branch into fuse_open_close_wait();
> call it from both fuse_open() and fuse_create_open() instead of
> duplicating the block in each.
> - Use fuse_sync_release() on the close_wait error path for both
> callers. It is functionally equivalent to fuse_release_common()
> used in fuse_open().
>
> fs/fuse/dir.c | 17 -----------
> fs/fuse/file.c | 78 ++++++++++++++++++++++++++++--------------------
> fs/fuse/fuse_i.h | 4 ++-
> 3 files changed, 49 insertions(+), 50 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 45ee4f1e4b68..9799c2b6eb4d 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -722,23 +722,6 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
> invalidate_inode_pages2(inode->i_mapping);
> }
>
> - if (fm->fc->close_wait) {
> - struct fuse_inode *fi = get_fuse_inode(inode);
> - bool need_open;
> -
> - inode_lock(inode);
> - spin_lock(&fi->lock);
> - need_open = (++fi->num_openers == 1);
> - spin_unlock(&fi->lock);
> -
> - if (need_open && fm->fc->kio.op && fm->fc->kio.op->file_open) {
> - err = fm->fc->kio.op->file_open(file, inode);
> - if (err)
> - fput(file);
> - }
> - inode_unlock(inode);
> - }
> -
> return err;
>
> out_free_ff:
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 58202a96517e..8884f5e522db 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -286,6 +286,43 @@ static void fuse_link_rw_file(struct file *file)
> spin_unlock(&fi->lock);
> }
>
> +static int fuse_open_close_wait(struct inode *inode, struct file *file)
> +{
> + struct fuse_conn *fc = get_fuse_conn(inode);
> + struct fuse_inode *fi = get_fuse_inode(inode);
> + struct fuse_file *ff = file->private_data;
> + /* the caller already holds the inode lock (e.g. truncate open) */
> + bool locked = test_and_clear_bit(FUSE_S_OPEN_INODE_LOCKED, &ff->ff_state);
> + int err = 0;
> +
> + if (!locked)
> + inode_lock(inode);
> + spin_lock(&fi->lock);
> +
> + if (++fi->num_openers == 1 || fi->i_size_unstable) {
> + fi->i_size_unstable = 1;
> + fi->inval_mask = ~0;
> + spin_unlock(&fi->lock);
> + err = fuse_update_attributes(inode, file, ~0);
> +
> + if (!err && fc->kio.op && fc->kio.op->file_open)
> + err = fc->kio.op->file_open(file, inode);
> +
> + spin_lock(&fi->lock);
> + fi->i_size_unstable = 0;
> + if (err)
> + fi->num_openers--;
> + }
> +
> + file->f_mode |= FMODE_NOWAIT;
> +
> + spin_unlock(&fi->lock);
> + if (!locked)
> + inode_unlock(inode);
> +
> + return err;
> +}
> +
> int fuse_finish_open(struct inode *inode, struct file *file)
> {
> struct fuse_file *ff = file->private_data;
> @@ -319,6 +356,9 @@ int fuse_finish_open(struct inode *inode, struct file *file)
> */
> if (!test_bit(FUSE_S_FAIL_IMMEDIATELY, &ff->ff_state))
> err = fuse_wait_on_inval_files(inode);
> +
> + if (!err)
> + err = fuse_open_close_wait(inode, file);
> }
>
> return err;
> @@ -347,6 +387,7 @@ static int fuse_open(struct inode *inode, struct file *file)
> bool is_truncate = (file->f_flags & O_TRUNC) && fc->atomic_o_trunc;
> bool is_wb_truncate = is_truncate && fc->writeback_cache;
> bool dax_truncate = is_truncate && FUSE_IS_DAX(inode);
> + bool lock_inode = is_wb_truncate || dax_truncate;
>
> if (fuse_is_bad(inode))
> return -EIO;
> @@ -362,7 +403,7 @@ static int fuse_open(struct inode *inode, struct file *file)
> if (err)
> return err;
>
> - if (is_wb_truncate || dax_truncate)
> + if (lock_inode)
> inode_lock(inode);
>
> if (dax_truncate) {
> @@ -378,6 +419,9 @@ static int fuse_open(struct inode *inode, struct file *file)
> err = fuse_do_open(fm, get_node_id(inode), file, false);
> if (!err) {
> ff = file->private_data;
> + /* tell fuse_open_close_wait() we already hold the inode lock */
> + if (lock_inode && fc->close_wait)
> + set_bit(FUSE_S_OPEN_INODE_LOCKED, &ff->ff_state);
> err = fuse_finish_open(inode, file);
> if (err)
> fuse_sync_release(fi, ff, file->f_flags);
> @@ -396,39 +440,9 @@ static int fuse_open(struct inode *inode, struct file *file)
> if (dax_truncate)
> filemap_invalidate_unlock(inode->i_mapping);
> out_inode_unlock:
> - if (is_wb_truncate || dax_truncate)
> + if (lock_inode)
> inode_unlock(inode);
>
> - if (!err && fc->close_wait) {
> - inode_lock(inode);
> - spin_lock(&fi->lock);
> -
> - if (++fi->num_openers == 1 || fi->i_size_unstable) {
> - fi->i_size_unstable = 1;
> - fi->inval_mask = ~0;
> - spin_unlock(&fi->lock);
> - err = fuse_update_attributes(inode, file, ~0);
> -
> - if (!err && fc->kio.op && fc->kio.op->file_open)
> - err = fc->kio.op->file_open(file, inode);
> -
> - spin_lock(&fi->lock);
> - fi->i_size_unstable = 0;
> - if (err)
> - fi->num_openers--;
> - }
> -
> - file->f_mode |= FMODE_NOWAIT;
> -
> - spin_unlock(&fi->lock);
> - inode_unlock(inode);
> -
> - if (err) {
> - fuse_release_common(file, false);
> - return err;
> - }
> - }
> -
> return err;
> }
>
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index a834a5b4dfe2..68e8df33103a 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -327,7 +327,9 @@ struct fuse_file {
> enum {
> /** Any fops on given ff should fail immediately */
> FUSE_S_FAIL_IMMEDIATELY = 0,
> - FUSE_S_CLOSING = 1
> + FUSE_S_CLOSING = 1,
> + /** The open path already holds the inode lock (close_wait) */
> + FUSE_S_OPEN_INODE_LOCKED = 2,
> };
>
> /** One input argument of a request */
> --
> 2.50.1 (Apple Git-155)
More information about the Devel
mailing list