hfsplus: rework hfsplus_readdir() logic

The xfstests' test-case generic/637 fails with error:

FSTYP -- hfsplus
PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.15.0-rc4+ #8 SMP PREEMPT_DYNAMIC Thu May 1 16:43:22 PDT 2025
MKFS_OPTIONS -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch

QA output created by 637
entries 7 and 8 have duplicate d_off 8
Found unlinked files in open dir (see xfstests-dev/results//generic/637.full for details)

Debugging of the hfsplus_readdir() logic showed this:

hfsplus: hfsplus_readdir(): 163 ctx->pos 0
hfsplus: hfsplus_readdir(): 189 ctx->pos 1
hfsplus: hfsplus_readdir(): 264 ctx->pos 2, ino 18
hfsplus: hfsplus_readdir(): 264 ctx->pos 3, ino 19
hfsplus: hfsplus_readdir(): 264 ctx->pos 4, ino 28
hfsplus: hfsplus_readdir(): 264 ctx->pos 5, ino 118
hfsplus: hfsplus_readdir(): 264 ctx->pos 6, ino 29
hfsplus: hfsplus_readdir(): 264 ctx->pos 7, ino 30
hfsplus: hfsplus_readdir(): 264 ctx->pos 8, ino 31
hfsplus: hfsplus_readdir(): 304 ctx->pos 8
hfsplus: hfsplus_unlink():420 dir->i_ino 17, inode->i_ino 28
hfsplus: hfsplus_readdir(): 141 ctx->pos 7
hfsplus: hfsplus_readdir(): 264 ctx->pos 7, ino 31
hfsplus: hfsplus_readdir(): 264 ctx->pos 8, ino 32
hfsplus: hfsplus_readdir(): 264 ctx->pos 9, ino 33

It means that hfsplus_readdir() stopped the processing of
folder's items on ctx->pos 8, then, item with ino 28 has
been deleted and hfsplus_readdir() re-started the logic
from ctx->pos 7. As a result, previous and new sets of
folder's items have overlapping values for the case of
d_off 8.

Currently, HFS+ has very complicated and fragile logic
of rd->file->f_pos correction in hfsplus_delete_cat().
This patch removes this logic and it stores the current
pos into hfsplus_readdir_data. Finally, if rd->pos == ctx->pos
then hfsplus_readdir() tries to find the position in
b-tree's node by means of hfsplus_cat_key. This position is
used to re-start the folder's content traversal.

sudo ./check generic/637
FSTYP         -- hfsplus
PLATFORM      -- Linux/x86_64 hfsplus-testing-0001 7.1.0-rc1+ #44 SMP PREEMPT_DYNAMIC Mon May  4 15:58:45 PDT 2026
MKFS_OPTIONS  -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch

generic/637  22s ...  22s
Ran: generic/637
Passed all 1 tests

Closes: https://github.com/hfs-linux-kernel/hfs-linux-kernel/issues/198
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Link: https://lore.kernel.org/r/20260505220051.2854696-2-slava@dubeyko.com
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>

diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
index 9bc8a6c48fd3375071c8f787980fa2ef882a2b81..776ce36cf076b442056087dedeb714d9844c091c 100644 (file)
--- a/fs/hfsplus/catalog.c
+++ b/fs/hfsplus/catalog.c
@@ -333,7 +333,6 @@ int hfsplus_delete_cat(u32 cnid, struct inode *dir, const struct qstr *str)
        struct super_block *sb = dir->i_sb;
        struct hfs_find_data fd;
        struct hfsplus_fork_raw fork;
-       struct list_head *pos;
        int err, off;
        u16 type;
 
@@ -392,16 +391,6 @@ int hfsplus_delete_cat(u32 cnid, struct inode *dir, const struct qstr *str)
                hfsplus_free_fork(sb, cnid, &fork, HFSPLUS_TYPE_RSRC);
        }
 
-       /* we only need to take spinlock for exclusion with ->release() */
-       spin_lock(&HFSPLUS_I(dir)->open_dir_lock);
-       list_for_each(pos, &HFSPLUS_I(dir)->open_dir_list) {
-               struct hfsplus_readdir_data *rd =
-                       list_entry(pos, struct hfsplus_readdir_data, list);
-               if (fd.tree->keycmp(fd.search_key, (void *)&rd->key) < 0)
-                       rd->file->f_pos--;
-       }
-       spin_unlock(&HFSPLUS_I(dir)->open_dir_lock);
-
        err = hfs_brec_remove(&fd);
        if (err)
                goto out;

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 47194370c2c5e03c15cded7a29982d040821a823..8bf6c7cdd9a865d92e39c9226eeb2203ccd33f85 100644 (file)
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -185,7 +185,15 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
        }
        if (ctx->pos >= inode->i_size)
                goto out;
-       err = hfs_brec_goto(&fd, ctx->pos - 1);
+       rd = file->private_data;
+       if (rd && rd->pos == ctx->pos) {
+               memcpy(fd.search_key, &rd->key, sizeof(struct hfsplus_cat_key));
+               err = hfs_brec_find(&fd, hfs_find_rec_by_key);
+               if (err == -ENOENT)
+                       err = hfs_brec_goto(&fd, 1);
+       } else {
+               err = hfs_brec_goto(&fd, ctx->pos - 1);
+       }
        if (err)
                goto out;
        for (;;) {
@@ -261,7 +269,6 @@ next:
                if (err)
                        goto out;
        }
-       rd = file->private_data;
        if (!rd) {
                rd = kmalloc_obj(struct hfsplus_readdir_data);
                if (!rd) {
@@ -269,15 +276,8 @@ next:
                        goto out;
                }
                file->private_data = rd;
-               rd->file = file;
-               spin_lock(&HFSPLUS_I(inode)->open_dir_lock);
-               list_add(&rd->list, &HFSPLUS_I(inode)->open_dir_list);
-               spin_unlock(&HFSPLUS_I(inode)->open_dir_lock);
        }
-       /*
-        * Can be done after the list insertion; exclusion with
-        * hfsplus_delete_cat() is provided by directory lock.
-        */
+       rd->pos = ctx->pos;
        memcpy(&rd->key, fd.key, sizeof(struct hfsplus_cat_key));
 out:
        kfree(strbuf);
@@ -287,13 +287,7 @@ out:
 
 static int hfsplus_dir_release(struct inode *inode, struct file *file)
 {
-       struct hfsplus_readdir_data *rd = file->private_data;
-       if (rd) {
-               spin_lock(&HFSPLUS_I(inode)->open_dir_lock);
-               list_del(&rd->list);
-               spin_unlock(&HFSPLUS_I(inode)->open_dir_lock);
-               kfree(rd);
-       }
+       kfree(file->private_data);
        return 0;
 }
 

diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 0e4268de9e60e0e1d166dba26c702739bbd72573..ec04b82ad9278733112182f34c01b9d70077ecd9 100644 (file)
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -214,8 +214,6 @@ struct hfsplus_inode_info {
        sector_t fs_blocks;
        u8 userflags;           /* BSD user file flags */
        u32 subfolders;         /* Subfolder count (HFSX only) */
-       struct list_head open_dir_list;
-       spinlock_t open_dir_lock;
        loff_t phys_size;
 
        struct inode vfs_inode;
@@ -272,8 +270,7 @@ struct hfs_find_data {
 };
 
 struct hfsplus_readdir_data {
-       struct list_head list;
-       struct file *file;
+       loff_t pos;
        struct hfsplus_cat_key key;
 };
 

diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 036ca516d85146811913b04a5a7e34b5da2555f8..1de9cc97b42e3e883a4a6d05d76c28418825fa19 100644 (file)
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -483,8 +483,6 @@ struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
        simple_inode_init_ts(inode);
 
        hip = HFSPLUS_I(inode);
-       INIT_LIST_HEAD(&hip->open_dir_list);
-       spin_lock_init(&hip->open_dir_lock);
        mutex_init(&hip->extents_lock);
        atomic_set(&hip->opencnt, 0);
        hip->extent_state = 0;

diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 40a0feda716bfabbb429abd568b736dd48030ddb..5777e31de45ac356b603b481de699dfcca44ad9a 100644 (file)
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -91,8 +91,6 @@ struct inode *hfsplus_iget(struct super_block *sb, unsigned long ino)
        HFSPLUS_I(inode)->fs_blocks = 0;
        HFSPLUS_I(inode)->userflags = 0;
        HFSPLUS_I(inode)->subfolders = 0;
-       INIT_LIST_HEAD(&HFSPLUS_I(inode)->open_dir_list);
-       spin_lock_init(&HFSPLUS_I(inode)->open_dir_lock);
        HFSPLUS_I(inode)->phys_size = 0;
 
        if (inode->i_ino >= HFSPLUS_FIRSTUSER_CNID ||