Re: [RFC PATCH v3 8/8] selftests/fuse: add fuse readdir caching test

[Luis Henriques <luis@igalia.com>]

On Sat, Sep 05 2026, Amir Goldstein wrote:

> On Fri, Sep 4, 2026 at 12:38 PM Luis Henriques <luis@igalia.com> wrote:
>>
>> This new test will check the caching behaviour using combinations of two
>> opendir flags: FOPEN_KEEP_CACHE and FOPEN_CACHE_DIR.
>>
>> Signed-off-by: Luis Henriques <luis@igalia.com>
>> ---
>>  .../selftests/filesystems/fuse/.gitignore     |   1 +
>>  .../selftests/filesystems/fuse/Makefile       |   2 +
>>  .../fuse/fuse_readdir_cache_test.c            | 274 ++++++++++++++++++
>>  3 files changed, 277 insertions(+)
>>  create mode 100644 tools/testing/selftests/filesystems/fuse/fuse_readdir_cache_test.c
>>
>> diff --git a/tools/testing/selftests/filesystems/fuse/.gitignore b/tools/testing/selftests/filesystems/fuse/.gitignore
>> index ebfe7133d811..ffa356f23e8c 100644
>> --- a/tools/testing/selftests/filesystems/fuse/.gitignore
>> +++ b/tools/testing/selftests/filesystems/fuse/.gitignore
>> @@ -4,3 +4,4 @@ fusectl_test
>>  write_extend_eof_test
>>  fuse_acl_cache_test
>>  fuse_symlink_cache_test
>> +fuse_readdir_cache_test
>> diff --git a/tools/testing/selftests/filesystems/fuse/Makefile b/tools/testing/selftests/filesystems/fuse/Makefile
>> index 3a5a557dde7a..570ced181168 100644
>> --- a/tools/testing/selftests/filesystems/fuse/Makefile
>> +++ b/tools/testing/selftests/filesystems/fuse/Makefile
>> @@ -6,6 +6,7 @@ TEST_GEN_PROGS := fusectl_test
>>  TEST_GEN_PROGS += write_extend_eof_test
>>  TEST_GEN_PROGS += fuse_acl_cache_test
>>  TEST_GEN_PROGS += fuse_symlink_cache_test
>> +TEST_GEN_PROGS += fuse_readdir_cache_test
>>  TEST_GEN_FILES := fuse_mnt
>>
>>  include ../../lib.mk
>> @@ -30,5 +31,6 @@ $(OUTPUT)/fuse_mnt: LDLIBS += $(VAR_LDLIBS)
>>
>>  $(OUTPUT)/fuse_acl_cache_test: fuse_common.c fuse_acl_cache_test.c
>>  $(OUTPUT)/fuse_symlink_cache_test: fuse_common.c fuse_symlink_cache_test.c
>> +$(OUTPUT)/fuse_readdir_cache_test: fuse_common.c fuse_readdir_cache_test.c
>>
>>  EXTRA_CLEAN := fuse_common.o
>> diff --git a/tools/testing/selftests/filesystems/fuse/fuse_readdir_cache_test.c b/tools/testing/selftests/filesystems/fuse/fuse_readdir_cache_test.c
>> new file mode 100644
>> index 000000000000..7c49cc6a23e3
>> --- /dev/null
>> +++ b/tools/testing/selftests/filesystems/fuse/fuse_readdir_cache_test.c
>> @@ -0,0 +1,274 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Simple filesystem to test FUSE readdir cache
>> + *
>> + * It will simply perform readdir operations on a directory checking how many
>> + * times a request is sent to user-space using all the possible caching
>> + * combination setting (FOPEN_KEEP_CACHE and FOPEN_CACHE_DIR flags).
>> + */
>> +
>> +#include <stdio.h>
>> +#include <limits.h>
>> +#include <dirent.h>
>> +
>> +#include "kselftest_harness.h"
>> +
>> +#include "fuse_common.h"
>> +
>> +#define DIRNAME "mydir"
>> +#define FILENAME "myfile"
>> +
>> +#define DIR_INO 42
>> +#define FILE_INO 43
>> +#define DOT_INO 40
>> +#define DOTDOT_INO 41
>> +
>> +#define TIMEOUT        86400.0f
>> +
>> +struct test_state {
>> +       pthread_mutex_t lock;
>> +       bool cache_readdir;
>> +       bool keep_cache;
>> +       int readdir_counter;
>> +} test_state = {
>> +       .lock = PTHREAD_MUTEX_INITIALIZER,
>> +};
>> +
>> +static void fs_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
>> +{
>> +       struct fuse_entry_param e = {};
>> +
>> +       if (parent != FUSE_ROOT_ID || strcmp(name, DIRNAME) != 0)
>> +               fuse_reply_err(req, ENOENT);
>
> Again: allergic to non matching {} and can just return here no need
> for all this nesting

ACK.

>> +       else {
>> +               if (!strcmp(name, DIRNAME)) {
>
> This condition is already guaranteed when we get here

*sigh*
Yeah, likely leftover code from the symlink cache test I used as base.

>> +                       e.ino = DIR_INO;
>> +                       e.attr.st_mode = S_IFDIR | 0755;
>> +                       e.attr.st_nlink = 1;
>> +               } else {
>> +                       e.ino = FUSE_ROOT_ID;
>> +                       e.attr.st_mode = S_IFDIR | 0755;
>> +                       e.attr.st_nlink = 2;
>> +               }
>> +               e.attr.st_mtime = time(NULL);
>> +               e.attr_timeout = TIMEOUT;
>> +               e.entry_timeout = TIMEOUT;
>> +               fuse_reply_entry(req, &e);
>> +       }
>> +}
>> +
>> +static int fill_stat(fuse_ino_t ino, struct stat *st)
>> +{
>> +       int ret = 0;
>> +
>> +       st->st_ino = ino;
>> +       st->st_mtime = time(NULL);
>
> Doesn't this need to be a const value?
> to make sure that it does not change from here to lookup time?

Good point.  That value should probably be kept in the test state
structure.

Cheers,
-- 
Luís

> Thanks,
> Amir.
>
>> +
>> +       switch (ino) {
>> +       case FUSE_ROOT_ID:
>> +               st->st_mode = S_IFDIR | 0755;
>> +               st->st_nlink = 2;
>> +               break;
>> +       case DOT_INO:
>> +       case DOTDOT_INO:
>> +       case DIR_INO:
>> +               st->st_mode = S_IFDIR | 0755;
>> +               st->st_nlink = 1;
>> +               break;
>> +       case FILE_INO:
>> +               st->st_mode = S_IFREG | 0444;
>> +               st->st_nlink = 1;
>> +               break;
>> +       default:
>> +               ret = -1;
>> +               break;
>> +       }
>> +
>> +       return ret;
>> +}
>> +
>> +static void fs_getattr(fuse_req_t req, fuse_ino_t ino,
>> +                      struct fuse_file_info *fi)
>> +{
>> +       struct stat st = {};
>> +
>> +       if (fill_stat(ino, &st) < 0)
>> +               fuse_reply_err(req, ENOENT);
>> +       else
>> +               fuse_reply_attr(req, &st, TIMEOUT);
>> +}
>> +
>> +static void fs_opendir(fuse_req_t req, fuse_ino_t ino,
>> +                      struct fuse_file_info *fi)
>> +{
>> +       pthread_mutex_lock(&test_state.lock);
>> +       fi->keep_cache = test_state.keep_cache;
>> +       fi->cache_readdir = test_state.cache_readdir;
>> +       pthread_mutex_unlock(&test_state.lock);
>> +       fuse_reply_open(req, fi);
>> +}
>> +
>> +static void fs_readdir(fuse_req_t req, fuse_ino_t ino, size_t size,
>> +                      off_t offset, struct fuse_file_info *fi)
>> +{
>> +       struct stat st = {};
>> +       char buf[1024];
>> +       char *pbuf;
>> +       size_t rem = size;
>> +       size_t sz;
>> +       int nextoff = 0;
>> +
>> +       if (ino != DIR_INO) {
>> +               fuse_reply_err(req, ENOTDIR);
>> +               return;
>> +       }
>> +       if (offset) {
>> +               fuse_reply_buf(req, NULL, 0);
>> +               return;
>> +       }
>> +       pbuf = buf;
>> +       fill_stat(DOT_INO, &st);
>> +       sz = fuse_add_direntry(req, pbuf, rem, ".", &st, nextoff++);
>> +       rem -= sz;
>> +       pbuf += sz;
>> +       fill_stat(DOTDOT_INO, &st);
>> +       sz = fuse_add_direntry(req, pbuf, rem, "..", &st, nextoff++);
>> +       rem -= sz;
>> +       pbuf += sz;
>> +       fill_stat(FILE_INO, &st);
>> +       sz = fuse_add_direntry(req, pbuf, rem, FILENAME, &st, nextoff++);
>> +       rem -= sz;
>> +
>> +       fuse_reply_buf(req, buf, size - rem);
>> +
>> +       pthread_mutex_lock(&test_state.lock);
>> +       test_state.readdir_counter++;
>> +       pthread_mutex_unlock(&test_state.lock);
>> +}
>> +
>> +static const struct fuse_lowlevel_ops fs_ops = {
>> +       .lookup         = fs_lookup,
>> +       .getattr        = fs_getattr,
>> +       .opendir        = fs_opendir,
>> +       .readdir        = fs_readdir,
>> +};
>> +
>> +FIXTURE(readdir_cache)
>> +{
>> +       struct fuse_session *se;
>> +       char mountpoint[MOUNTPOINT_SZ];
>> +       pthread_t thread;
>> +};
>> +
>> +FIXTURE_VARIANT(readdir_cache)
>> +{
>> +       bool cache_readdir;
>> +       bool keep_cache;
>> +};
>> +FIXTURE_VARIANT_ADD(readdir_cache, nocache)
>> +{
>> +       .cache_readdir = false,
>> +       .keep_cache = false,
>> +};
>> +FIXTURE_VARIANT_ADD(readdir_cache, cache_readdir)
>> +{
>> +       .cache_readdir = true,
>> +       .keep_cache = false,
>> +};
>> +FIXTURE_VARIANT_ADD(readdir_cache, keep_cache)
>> +{
>> +       .cache_readdir = false,
>> +       .keep_cache = true,
>> +};
>> +FIXTURE_VARIANT_ADD(readdir_cache, cache)
>> +{
>> +       .cache_readdir = true,
>> +       .keep_cache = true,
>> +};
>> +
>> +FIXTURE_SETUP(readdir_cache)
>> +{
>> +       char err[MAX_ERR_MSG];
>> +
>> +       pthread_mutex_lock(&test_state.lock);
>> +       test_state.readdir_counter = 0;
>> +       test_state.cache_readdir = variant->cache_readdir;
>> +       test_state.keep_cache = variant->keep_cache;
>> +       pthread_mutex_unlock(&test_state.lock);
>> +
>> +       if (fs_setup(&self->se, self->mountpoint, &fs_ops, &self->thread, err))
>> +               SKIP(return, err);
>> +}
>> +
>> +FIXTURE_TEARDOWN(readdir_cache)
>> +{
>> +       fs_teardown(self->se, self->thread, self->mountpoint);
>> +}
>> +
>> +TEST_F(readdir_cache, test_readdir_cache)
>> +{
>> +       struct dirent *dentry;
>> +       DIR *dir;
>> +       char pathname[PATH_MAX];
>> +       int total_counter, rewind_counter;
>> +       int dentrycount;
>> +
>> +       sprintf(pathname, "%s/%s", self->mountpoint, DIRNAME);
>> +
>> +       dir = opendir(pathname);
>> +       if (dir == NULL)
>> +               TH_LOG("opendir(): %s", strerror(errno));
>> +       ASSERT_NE(dir, NULL);
>> +
>> +       errno = 0;
>> +       dentrycount = 0;
>> +       while ((dentry = readdir(dir)))
>> +               dentrycount++;
>> +       ASSERT_EQ(errno, 0);
>> +       ASSERT_EQ(dentrycount, 3);
>> +
>> +       rewinddir(dir);
>> +       errno = 0;
>> +       dentrycount = 0;
>> +       while ((dentry = readdir(dir)))
>> +               dentrycount++;
>> +       ASSERT_EQ(errno, 0);
>> +       ASSERT_EQ(dentrycount, 3);
>> +
>> +       ASSERT_EQ(closedir(dir), 0);
>> +
>> +       pthread_mutex_lock(&test_state.lock);
>> +       rewind_counter = test_state.readdir_counter;
>> +       pthread_mutex_unlock(&test_state.lock);
>> +
>> +       dir = opendir(pathname);
>> +       if (dir == NULL)
>> +               TH_LOG("opendir(): %s", strerror(errno));
>> +       ASSERT_NE(dir, NULL);
>> +
>> +       errno = 0;
>> +       dentrycount = 0;
>> +       while ((dentry = readdir(dir)))
>> +               dentrycount++;
>> +       ASSERT_EQ(errno, 0);
>> +       ASSERT_EQ(dentrycount, 3);
>> +
>> +       ASSERT_EQ(closedir(dir), 0);
>> +
>> +       pthread_mutex_lock(&test_state.lock);
>> +       total_counter = test_state.readdir_counter;
>> +       pthread_mutex_unlock(&test_state.lock);
>> +
>> +       if (!variant->cache_readdir) {
>> +               ASSERT_EQ(rewind_counter, 2);
>> +               ASSERT_EQ(total_counter, 3);
>> +       } else if (!variant->keep_cache) {
>> +               ASSERT_EQ(rewind_counter, 1);
>> +               ASSERT_EQ(total_counter, 2);
>> +       } else {
>> +               ASSERT_EQ(rewind_counter, 1);
>> +               ASSERT_EQ(total_counter, 1);
>> +       }
>> +}
>> +
>> +TEST_HARNESS_MAIN