| Message ID | 20211102052604.59462-7-jefflexu@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fuse,virtiofs: support per-file DAX | expand |
On Tue, 2 Nov 2021 at 06:26, Jeffle Xu <jefflexu@linux.alibaba.com> wrote: > > When the per inode DAX hint changes while the file is still *opened*, it > is quite complicated and maybe fragile to dynamically change the DAX > state. > > Hence mark the inode and corresponding dentries as DONE_CACHE once the > per inode DAX hint changes, so that the inode instance will be evicted > and freed as soon as possible once the file is closed and the last > reference to the inode is put. And then when the file gets reopened next > time, the new instantiated inode will reflect the new DAX state. > > In summary, when the per inode DAX hint changes for an *opened* file, the > DAX state of the file won't be updated until this file is closed and > reopened later. This patch does nothing, since fuse already uses .drop_inode = generic_delete_inode, which is has the same effect as setting I_DONTCACHE, at least in the fuse case (inode should never be dirty at eviction). In fact it may be cleaner to set I_DONTCACHE unconditionally and remove the .drop_inode callback setting. Thanks, Miklos
On 11/10/21 11:50 PM, Miklos Szeredi wrote: > On Tue, 2 Nov 2021 at 06:26, Jeffle Xu <jefflexu@linux.alibaba.com> wrote: >> >> When the per inode DAX hint changes while the file is still *opened*, it >> is quite complicated and maybe fragile to dynamically change the DAX >> state. >> >> Hence mark the inode and corresponding dentries as DONE_CACHE once the >> per inode DAX hint changes, so that the inode instance will be evicted >> and freed as soon as possible once the file is closed and the last >> reference to the inode is put. And then when the file gets reopened next >> time, the new instantiated inode will reflect the new DAX state. >> >> In summary, when the per inode DAX hint changes for an *opened* file, the >> DAX state of the file won't be updated until this file is closed and >> reopened later. > > This patch does nothing, since fuse already uses .drop_inode = > generic_delete_inode, which is has the same effect as setting > I_DONTCACHE, at least in the fuse case (inode should never be dirty at > eviction). Yes, it is. .drop_inode() of FUSE will always free inode. Here we only need to set dentry as DCACHE_DONTCACHE. Here I just call d_mark_dontcache() directly, though I_DONTCACHE is useless but harmless in the case of FUSE... > In fact it may be cleaner to set I_DONTCACHE > unconditionally and remove the .drop_inode callback setting. It works in both cases, I mean, in current case (current code retained untouched) and the case you described above.
On Wed, Nov 10, 2021 at 04:50:25PM +0100, Miklos Szeredi wrote: > On Tue, 2 Nov 2021 at 06:26, Jeffle Xu <jefflexu@linux.alibaba.com> wrote: > > > > When the per inode DAX hint changes while the file is still *opened*, it > > is quite complicated and maybe fragile to dynamically change the DAX > > state. > > > > Hence mark the inode and corresponding dentries as DONE_CACHE once the > > per inode DAX hint changes, so that the inode instance will be evicted > > and freed as soon as possible once the file is closed and the last > > reference to the inode is put. And then when the file gets reopened next > > time, the new instantiated inode will reflect the new DAX state. > > > > In summary, when the per inode DAX hint changes for an *opened* file, the > > DAX state of the file won't be updated until this file is closed and > > reopened later. > > This patch does nothing, since fuse already uses .drop_inode = > generic_delete_inode, which is has the same effect as setting > I_DONTCACHE, at least in the fuse case (inode should never be dirty at > eviction). In fact it may be cleaner to set I_DONTCACHE > unconditionally and remove the .drop_inode callback setting. I thought idea was to drop dentry and not cache it which in turn will drop dentry's reference on inode and lead to cleanup of inode. Otherwise dentry might remain cached and which in-turn will keep inode in cache. Am I missing something. Thanks Vivek
On 11/12/21 4:33 AM, Vivek Goyal wrote: > On Wed, Nov 10, 2021 at 04:50:25PM +0100, Miklos Szeredi wrote: >> On Tue, 2 Nov 2021 at 06:26, Jeffle Xu <jefflexu@linux.alibaba.com> wrote: >>> >>> When the per inode DAX hint changes while the file is still *opened*, it >>> is quite complicated and maybe fragile to dynamically change the DAX >>> state. >>> >>> Hence mark the inode and corresponding dentries as DONE_CACHE once the >>> per inode DAX hint changes, so that the inode instance will be evicted >>> and freed as soon as possible once the file is closed and the last >>> reference to the inode is put. And then when the file gets reopened next >>> time, the new instantiated inode will reflect the new DAX state. >>> >>> In summary, when the per inode DAX hint changes for an *opened* file, the >>> DAX state of the file won't be updated until this file is closed and >>> reopened later. >> >> This patch does nothing, since fuse already uses .drop_inode = >> generic_delete_inode, which is has the same effect as setting >> I_DONTCACHE, at least in the fuse case (inode should never be dirty at >> eviction). In fact it may be cleaner to set I_DONTCACHE >> unconditionally and remove the .drop_inode callback setting. > > I thought idea was to drop dentry and not cache it which in turn > will drop dentry's reference on inode and lead to cleanup of inode. > > Otherwise dentry might remain cached and which in-turn will keep > inode in cache. Am I missing something. > Yes that's the case.
diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c index c8ee601b94b8..e8766420c708 100644 --- a/fs/fuse/dax.c +++ b/fs/fuse/dax.c @@ -1362,6 +1362,15 @@ void fuse_dax_inode_init(struct inode *inode, unsigned int flags) inode->i_data.a_ops = &fuse_dax_file_aops; } +void fuse_dax_dontcache(struct inode *inode, unsigned int flags) +{ + struct fuse_conn *fc = get_fuse_conn(inode); + + if (fuse_is_inode_dax_mode(fc->dax_mode) && + (!!IS_DAX(inode) != !!(flags & FUSE_ATTR_DAX))) + d_mark_dontcache(inode); +} + bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment) { if (fc->dax && (map_alignment > FUSE_DAX_SHIFT)) { diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 58e54b5a4d65..fe8364b522ac 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -1282,6 +1282,7 @@ void fuse_dax_conn_free(struct fuse_conn *fc); bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi); void fuse_dax_inode_init(struct inode *inode, unsigned int flags); void fuse_dax_inode_cleanup(struct inode *inode); +void fuse_dax_dontcache(struct inode *inode, unsigned int flags); bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int map_alignment); void fuse_dax_cancel_work(struct fuse_conn *fc); diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index 0512d8cb36c3..4a2a58cc71af 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -268,6 +268,9 @@ void fuse_change_attributes(struct inode *inode, struct fuse_attr *attr, if (inval) invalidate_inode_pages2(inode->i_mapping); } + + if (IS_ENABLED(CONFIG_FUSE_DAX)) + fuse_dax_dontcache(inode, attr->flags); } static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr)
When the per inode DAX hint changes while the file is still *opened*, it is quite complicated and maybe fragile to dynamically change the DAX state. Hence mark the inode and corresponding dentries as DONE_CACHE once the per inode DAX hint changes, so that the inode instance will be evicted and freed as soon as possible once the file is closed and the last reference to the inode is put. And then when the file gets reopened next time, the new instantiated inode will reflect the new DAX state. In summary, when the per inode DAX hint changes for an *opened* file, the DAX state of the file won't be updated until this file is closed and reopened later. Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com> --- fs/fuse/dax.c | 9 +++++++++ fs/fuse/fuse_i.h | 1 + fs/fuse/inode.c | 3 +++ 3 files changed, 13 insertions(+)