With the infrastructure introduced to periodically invalidate expired
dentries, it is now possible to add an extra work queue to invalidate
dentries when an epoch is incremented. This work queue will only be
triggered when the 'inval_wq' parameter is set.
Signed-off-by: Luis Henriques <luis@igalia.com>
---
fs/fuse/dev.c | 7 ++++---
fs/fuse/dir.c | 34 ++++++++++++++++++++++++++++++++++
fs/fuse/fuse_i.h | 4 ++++
fs/fuse/inode.c | 41 ++++++++++++++++++++++-------------------
4 files changed, 64 insertions(+), 22 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index e80cd8f2c049..48c5c01c3e5b 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2033,13 +2033,14 @@ static int fuse_notify_resend(struct fuse_conn *fc)
/*
* Increments the fuse connection epoch. This will result of dentries from
- * previous epochs to be invalidated.
- *
- * XXX optimization: add call to shrink_dcache_sb()?
+ * previous epochs to be invalidated. Additionally, if inval_wq is set, a work
+ * queue is scheduled to trigger the invalidation.
*/
static int fuse_notify_inc_epoch(struct fuse_conn *fc)
{
atomic_inc(&fc->epoch);
+ if (inval_wq)
+ schedule_work(&fc->epoch_work);
return 0;
}
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index b7ddf0f2b3a4..f05cb8f4c555 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -182,6 +182,40 @@ static void fuse_dentry_tree_work(struct work_struct *work)
schedule_delayed_work(&dentry_tree_work, secs_to_jiffies(inval_wq));
}
+void fuse_epoch_work(struct work_struct *work)
+{
+ struct fuse_conn *fc = container_of(work, struct fuse_conn,
+ epoch_work);
+ struct fuse_dentry *fd;
+ struct rb_node *node, *next;
+ struct dentry *dentry;
+ int epoch;
+ int i;
+
+ epoch = atomic_read(&fc->epoch);
+
+ for (i = 0; i < HASH_SIZE; i++) {
+ spin_lock(&dentry_hash[i].lock);
+ node = rb_first(&dentry_hash[i].tree);
+ while (node) {
+ next = rb_next(node);
+ fd = rb_entry(node, struct fuse_dentry, node);
+ dentry = fd->dentry;
+ if ((fc == get_fuse_conn_super(dentry->d_sb)) &&
+ (dentry->d_time < epoch)) {
+ rb_erase(&fd->node, &dentry_hash[i].tree);
+ RB_CLEAR_NODE(&fd->node);
+ /* XXX use tmp tree and d_invalidate all at once? */
+ spin_unlock(&dentry_hash[i].lock);
+ d_invalidate(fd->dentry);
+ spin_lock(&dentry_hash[i].lock);
+ }
+ node = next;
+ }
+ spin_unlock(&dentry_hash[i].lock);
+ }
+}
+
void fuse_dentry_tree_init(void)
{
int i;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 214162f12353..1f102ecdb9ab 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -642,6 +642,8 @@ struct fuse_conn {
/** Current epoch for up-to-date dentries */
atomic_t epoch;
+ struct work_struct epoch_work;
+
struct rcu_head rcu;
/** The user id for this mount */
@@ -1261,6 +1263,8 @@ void fuse_check_timeout(struct work_struct *work);
void fuse_dentry_tree_init(void);
void fuse_dentry_tree_cleanup(void);
+void fuse_epoch_work(struct work_struct *work);
+
/**
* Invalidate inode attributes
*/
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 25e51efc82ee..853220413fd8 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -962,6 +962,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
refcount_set(&fc->count, 1);
atomic_set(&fc->dev_count, 1);
atomic_set(&fc->epoch, 1);
+ INIT_WORK(&fc->epoch_work, fuse_epoch_work);
init_waitqueue_head(&fc->blocked_waitq);
fuse_iqueue_init(&fc->iq, fiq_ops, fiq_priv);
INIT_LIST_HEAD(&fc->bg_queue);
@@ -1006,26 +1007,28 @@ static void delayed_release(struct rcu_head *p)
void fuse_conn_put(struct fuse_conn *fc)
{
- if (refcount_dec_and_test(&fc->count)) {
- struct fuse_iqueue *fiq = &fc->iq;
- struct fuse_sync_bucket *bucket;
-
- if (IS_ENABLED(CONFIG_FUSE_DAX))
- fuse_dax_conn_free(fc);
- if (fc->timeout.req_timeout)
- cancel_delayed_work_sync(&fc->timeout.work);
- if (fiq->ops->release)
- fiq->ops->release(fiq);
- put_pid_ns(fc->pid_ns);
- bucket = rcu_dereference_protected(fc->curr_bucket, 1);
- if (bucket) {
- WARN_ON(atomic_read(&bucket->count) != 1);
- kfree(bucket);
- }
- if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
- fuse_backing_files_free(fc);
- call_rcu(&fc->rcu, delayed_release);
+ struct fuse_iqueue *fiq = &fc->iq;
+ struct fuse_sync_bucket *bucket;
+
+ if (!refcount_dec_and_test(&fc->count))
+ return;
+
+ if (IS_ENABLED(CONFIG_FUSE_DAX))
+ fuse_dax_conn_free(fc);
+ if (fc->timeout.req_timeout)
+ cancel_delayed_work_sync(&fc->timeout.work);
+ cancel_work_sync(&fc->epoch_work);
+ if (fiq->ops->release)
+ fiq->ops->release(fiq);
+ put_pid_ns(fc->pid_ns);
+ bucket = rcu_dereference_protected(fc->curr_bucket, 1);
+ if (bucket) {
+ WARN_ON(atomic_read(&bucket->count) != 1);
+ kfree(bucket);
}
+ if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
+ fuse_backing_files_free(fc);
+ call_rcu(&fc->rcu, delayed_release);
}
EXPORT_SYMBOL_GPL(fuse_conn_put);On Thu, 28 Aug 2025 at 18:30, Luis Henriques <luis@igalia.com> wrote:
>
> With the infrastructure introduced to periodically invalidate expired
> dentries, it is now possible to add an extra work queue to invalidate
> dentries when an epoch is incremented. This work queue will only be
> triggered when the 'inval_wq' parameter is set.
>
> Signed-off-by: Luis Henriques <luis@igalia.com>
> ---
> fs/fuse/dev.c | 7 ++++---
> fs/fuse/dir.c | 34 ++++++++++++++++++++++++++++++++++
> fs/fuse/fuse_i.h | 4 ++++
> fs/fuse/inode.c | 41 ++++++++++++++++++++++-------------------
> 4 files changed, 64 insertions(+), 22 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index e80cd8f2c049..48c5c01c3e5b 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -2033,13 +2033,14 @@ static int fuse_notify_resend(struct fuse_conn *fc)
>
> /*
> * Increments the fuse connection epoch. This will result of dentries from
> - * previous epochs to be invalidated.
> - *
> - * XXX optimization: add call to shrink_dcache_sb()?
I guess it wouldn't hurt. Definitely simpler, so I'd opt for this.
> void fuse_conn_put(struct fuse_conn *fc)
> {
> - if (refcount_dec_and_test(&fc->count)) {
> - struct fuse_iqueue *fiq = &fc->iq;
> - struct fuse_sync_bucket *bucket;
> -
> - if (IS_ENABLED(CONFIG_FUSE_DAX))
> - fuse_dax_conn_free(fc);
> - if (fc->timeout.req_timeout)
> - cancel_delayed_work_sync(&fc->timeout.work);
> - if (fiq->ops->release)
> - fiq->ops->release(fiq);
> - put_pid_ns(fc->pid_ns);
> - bucket = rcu_dereference_protected(fc->curr_bucket, 1);
> - if (bucket) {
> - WARN_ON(atomic_read(&bucket->count) != 1);
> - kfree(bucket);
> - }
> - if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> - fuse_backing_files_free(fc);
> - call_rcu(&fc->rcu, delayed_release);
> + struct fuse_iqueue *fiq = &fc->iq;
> + struct fuse_sync_bucket *bucket;
> +
> + if (!refcount_dec_and_test(&fc->count))
> + return;
Please don't do this. It's difficult to see what actually changed this way.
Thanks,
Miklos
On Thu, Sep 04 2025, Miklos Szeredi wrote:
> On Thu, 28 Aug 2025 at 18:30, Luis Henriques <luis@igalia.com> wrote:
>>
>> With the infrastructure introduced to periodically invalidate expired
>> dentries, it is now possible to add an extra work queue to invalidate
>> dentries when an epoch is incremented. This work queue will only be
>> triggered when the 'inval_wq' parameter is set.
>>
>> Signed-off-by: Luis Henriques <luis@igalia.com>
>> ---
>> fs/fuse/dev.c | 7 ++++---
>> fs/fuse/dir.c | 34 ++++++++++++++++++++++++++++++++++
>> fs/fuse/fuse_i.h | 4 ++++
>> fs/fuse/inode.c | 41 ++++++++++++++++++++++-------------------
>> 4 files changed, 64 insertions(+), 22 deletions(-)
>>
>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>> index e80cd8f2c049..48c5c01c3e5b 100644
>> --- a/fs/fuse/dev.c
>> +++ b/fs/fuse/dev.c
>> @@ -2033,13 +2033,14 @@ static int fuse_notify_resend(struct fuse_conn *fc)
>>
>> /*
>> * Increments the fuse connection epoch. This will result of dentries from
>> - * previous epochs to be invalidated.
>> - *
>> - * XXX optimization: add call to shrink_dcache_sb()?
>
> I guess it wouldn't hurt. Definitely simpler, so I'd opt for this.
So, your suggesting to have the work queue simply calling this instead of
walking through the dentries? (Or even *not* having a work queue at all?)
>> void fuse_conn_put(struct fuse_conn *fc)
>> {
>> - if (refcount_dec_and_test(&fc->count)) {
>> - struct fuse_iqueue *fiq = &fc->iq;
>> - struct fuse_sync_bucket *bucket;
>> -
>> - if (IS_ENABLED(CONFIG_FUSE_DAX))
>> - fuse_dax_conn_free(fc);
>> - if (fc->timeout.req_timeout)
>> - cancel_delayed_work_sync(&fc->timeout.work);
>> - if (fiq->ops->release)
>> - fiq->ops->release(fiq);
>> - put_pid_ns(fc->pid_ns);
>> - bucket = rcu_dereference_protected(fc->curr_bucket, 1);
>> - if (bucket) {
>> - WARN_ON(atomic_read(&bucket->count) != 1);
>> - kfree(bucket);
>> - }
>> - if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
>> - fuse_backing_files_free(fc);
>> - call_rcu(&fc->rcu, delayed_release);
>> + struct fuse_iqueue *fiq = &fc->iq;
>> + struct fuse_sync_bucket *bucket;
>> +
>> + if (!refcount_dec_and_test(&fc->count))
>> + return;
>
> Please don't do this. It's difficult to see what actually changed this way.
Right, that didn't occur to me. Sorry. I'll probably add a separate
patch to re-indent this function, which makes it easier to read in my
opinion. Not sure that's acceptable, so feel free to drop it silently :-)
Cheers,
--
Luís
On Thu, 4 Sept 2025 at 16:11, Luis Henriques <luis@igalia.com> wrote: > > On Thu, Sep 04 2025, Miklos Szeredi wrote: > > > On Thu, 28 Aug 2025 at 18:30, Luis Henriques <luis@igalia.com> wrote: > >> > >> With the infrastructure introduced to periodically invalidate expired > >> dentries, it is now possible to add an extra work queue to invalidate > >> dentries when an epoch is incremented. This work queue will only be > >> triggered when the 'inval_wq' parameter is set. > >> > >> Signed-off-by: Luis Henriques <luis@igalia.com> > >> --- > >> fs/fuse/dev.c | 7 ++++--- > >> fs/fuse/dir.c | 34 ++++++++++++++++++++++++++++++++++ > >> fs/fuse/fuse_i.h | 4 ++++ > >> fs/fuse/inode.c | 41 ++++++++++++++++++++++------------------- > >> 4 files changed, 64 insertions(+), 22 deletions(-) > >> > >> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > >> index e80cd8f2c049..48c5c01c3e5b 100644 > >> --- a/fs/fuse/dev.c > >> +++ b/fs/fuse/dev.c > >> @@ -2033,13 +2033,14 @@ static int fuse_notify_resend(struct fuse_conn *fc) > >> > >> /* > >> * Increments the fuse connection epoch. This will result of dentries from > >> - * previous epochs to be invalidated. > >> - * > >> - * XXX optimization: add call to shrink_dcache_sb()? > > > > I guess it wouldn't hurt. Definitely simpler, so I'd opt for this. > > So, your suggesting to have the work queue simply calling this instead of > walking through the dentries? (Or even *not* having a work queue at all?) I think doing in in a work queue is useful, since walking the tree might take a significant amount of time. Not having to do the walk manually is definitely a simplification. It might throw out dentries that got looked up since the last epoch, but it's probably not a big loss in terms of performance. Thanks, Miklos
On Thu, Sep 04 2025, Miklos Szeredi wrote: > On Thu, 4 Sept 2025 at 16:11, Luis Henriques <luis@igalia.com> wrote: >> >> On Thu, Sep 04 2025, Miklos Szeredi wrote: >> >> > On Thu, 28 Aug 2025 at 18:30, Luis Henriques <luis@igalia.com> wrote: >> >> >> >> With the infrastructure introduced to periodically invalidate expired >> >> dentries, it is now possible to add an extra work queue to invalidate >> >> dentries when an epoch is incremented. This work queue will only be >> >> triggered when the 'inval_wq' parameter is set. >> >> >> >> Signed-off-by: Luis Henriques <luis@igalia.com> >> >> --- >> >> fs/fuse/dev.c | 7 ++++--- >> >> fs/fuse/dir.c | 34 ++++++++++++++++++++++++++++++++++ >> >> fs/fuse/fuse_i.h | 4 ++++ >> >> fs/fuse/inode.c | 41 ++++++++++++++++++++++------------------- >> >> 4 files changed, 64 insertions(+), 22 deletions(-) >> >> >> >> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c >> >> index e80cd8f2c049..48c5c01c3e5b 100644 >> >> --- a/fs/fuse/dev.c >> >> +++ b/fs/fuse/dev.c >> >> @@ -2033,13 +2033,14 @@ static int fuse_notify_resend(struct fuse_conn *fc) >> >> >> >> /* >> >> * Increments the fuse connection epoch. This will result of dentries from >> >> - * previous epochs to be invalidated. >> >> - * >> >> - * XXX optimization: add call to shrink_dcache_sb()? >> > >> > I guess it wouldn't hurt. Definitely simpler, so I'd opt for this. >> >> So, your suggesting to have the work queue simply calling this instead of >> walking through the dentries? (Or even *not* having a work queue at all?) > > I think doing in in a work queue is useful, since walking the tree > might take a significant amount of time. > > Not having to do the walk manually is definitely a simplification. > It might throw out dentries that got looked up since the last epoch, > but it's probably not a big loss in terms of performance. OK, so that definitely makes things simpler for v6. Thanks! Cheers, -- Luís
© 2016 - 2025 Red Hat, Inc.