Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fuse passthrough: fix cache files closed by lru in passthrough model #1876

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

wswsmao
Copy link
Contributor

@wswsmao wswsmao commented Nov 27, 2024

In passthough model, close will be toke over by go-fuse, so file.Close is unnecessary

@wswsmao
Copy link
Contributor Author

wswsmao commented Nov 27, 2024

before:
cache file will be close and go-fuse ioctl will return bad descriptor
https://github.com/hanwen/go-fuse/blob/master/fuse/passthrough_linux.go#L27

after:
Registration successful,and we can find read passthrough log by ftrace:

 9)   9.637 us    |    } /* fuse_passthrough_read_iter [fuse] */
  9)   9.955 us    |  } /* fuse_file_read_iter [fuse] */
  9)               |  fuse_file_read_iter [fuse]() {
  9)               |    fuse_passthrough_read_iter [fuse]() {
  9)               |      backing_file_read_iter() {
  9)   0.150 us    |        override_creds();
  9)               |        vfs_iter_read() {
  9)               |          rw_verify_area() {
  9)               |            security_file_permission() {
  9)               |              selinux_file_permission() {
  9)               |                inode_security() {
  9)   0.164 us    |                  __cond_resched();
  9)   0.494 us    |                }
  9)   0.148 us    |                avc_policy_seqno();
  9)   1.080 us    |              }
  9)   0.144 us    |              bpf_lsm_file_permission();
  9)   1.699 us    |            }
  9)   2.014 us    |          }
  9)               |          do_iter_readv_writev() {
  9)               |            ext4_file_read_iter() {
  9)               |              generic_file_read_iter() {
  9)               |                filemap_read() {
  9)   0.151 us    |                  __cond_resched();
  9)               |                  filemap_get_pages() {
  9)               |                    filemap_get_read_batch() {
  9)   0.147 us    |                      __rcu_read_lock();
  9)   0.144 us    |                      __rcu_read_lock();
  9)   0.146 us    |                      __rcu_read_unlock();
  9)   0.146 us    |                      __rcu_read_unlock();
  9)   1.457 us    |                    }
  9)   1.756 us    |                  }
  9)   0.152 us    |                  folio_mark_accessed();
  9)               |                  touch_atime() {
  9)               |                    atime_needs_update() {
  9)   0.152 us    |                      make_vfsuid();
  9)   0.146 us    |                      make_vfsgid();
  9)   0.749 us    |                    }
  9)   1.039 us    |                  }
  9)   4.186 us    |                }
  9)   4.467 us    |              }
  9)   4.750 us    |            }
  9)   5.053 us    |          }
  9)   0.177 us    |          __fsnotify_parent();
  9)   7.849 us    |        }
  9)   0.146 us    |        revert_creds();
  9)               |        fuse_file_accessed [fuse]() {
  9)   0.145 us    |          fuse_invalidate_atime [fuse]();
  9)   0.429 us    |        }
  9)   9.322 us    |      }
  9)   9.739 us    |    }
  9) + 10.051 us   |  }

FdCache: fCache,
BufPool: bufPool,
Direct: dcc.Direct,
PassThrough: cfg.FuseConfig.PassThrough,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't enable this for blobCache.

Rather than globally control this mode, can we pass the PassThrough flag to the cache as an option of *directoryCache.Get?

In passthough model, close will be toke over by go-fuse, so file.Close is unnecessary

Signed-off-by: abushwang <[email protected]>
@ktock ktock merged commit 56c7b0f into containerd:main Nov 27, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants