Skip to content

Commit 4b04964

Browse files
committed
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: hfs-linux-kernel#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>
1 parent d67aade commit 4b04964

5 files changed

Lines changed: 12 additions & 36 deletions

File tree

fs/hfsplus/catalog.c

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,6 @@ int hfsplus_delete_cat(u32 cnid, struct inode *dir, const struct qstr *str)
333333
struct super_block *sb = dir->i_sb;
334334
struct hfs_find_data fd;
335335
struct hfsplus_fork_raw fork;
336-
struct list_head *pos;
337336
int err, off;
338337
u16 type;
339338

@@ -392,16 +391,6 @@ int hfsplus_delete_cat(u32 cnid, struct inode *dir, const struct qstr *str)
392391
hfsplus_free_fork(sb, cnid, &fork, HFSPLUS_TYPE_RSRC);
393392
}
394393

395-
/* we only need to take spinlock for exclusion with ->release() */
396-
spin_lock(&HFSPLUS_I(dir)->open_dir_lock);
397-
list_for_each(pos, &HFSPLUS_I(dir)->open_dir_list) {
398-
struct hfsplus_readdir_data *rd =
399-
list_entry(pos, struct hfsplus_readdir_data, list);
400-
if (fd.tree->keycmp(fd.search_key, (void *)&rd->key) < 0)
401-
rd->file->f_pos--;
402-
}
403-
spin_unlock(&HFSPLUS_I(dir)->open_dir_lock);
404-
405394
err = hfs_brec_remove(&fd);
406395
if (err)
407396
goto out;

fs/hfsplus/dir.c

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,15 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
185185
}
186186
if (ctx->pos >= inode->i_size)
187187
goto out;
188-
err = hfs_brec_goto(&fd, ctx->pos - 1);
188+
rd = file->private_data;
189+
if (rd && rd->pos == ctx->pos) {
190+
memcpy(fd.search_key, &rd->key, sizeof(struct hfsplus_cat_key));
191+
err = hfs_brec_find(&fd, hfs_find_rec_by_key);
192+
if (err == -ENOENT)
193+
err = hfs_brec_goto(&fd, 1);
194+
} else {
195+
err = hfs_brec_goto(&fd, ctx->pos - 1);
196+
}
189197
if (err)
190198
goto out;
191199
for (;;) {
@@ -261,23 +269,15 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
261269
if (err)
262270
goto out;
263271
}
264-
rd = file->private_data;
265272
if (!rd) {
266273
rd = kmalloc_obj(struct hfsplus_readdir_data);
267274
if (!rd) {
268275
err = -ENOMEM;
269276
goto out;
270277
}
271278
file->private_data = rd;
272-
rd->file = file;
273-
spin_lock(&HFSPLUS_I(inode)->open_dir_lock);
274-
list_add(&rd->list, &HFSPLUS_I(inode)->open_dir_list);
275-
spin_unlock(&HFSPLUS_I(inode)->open_dir_lock);
276279
}
277-
/*
278-
* Can be done after the list insertion; exclusion with
279-
* hfsplus_delete_cat() is provided by directory lock.
280-
*/
280+
rd->pos = ctx->pos;
281281
memcpy(&rd->key, fd.key, sizeof(struct hfsplus_cat_key));
282282
out:
283283
kfree(strbuf);
@@ -287,13 +287,7 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
287287

288288
static int hfsplus_dir_release(struct inode *inode, struct file *file)
289289
{
290-
struct hfsplus_readdir_data *rd = file->private_data;
291-
if (rd) {
292-
spin_lock(&HFSPLUS_I(inode)->open_dir_lock);
293-
list_del(&rd->list);
294-
spin_unlock(&HFSPLUS_I(inode)->open_dir_lock);
295-
kfree(rd);
296-
}
290+
kfree(file->private_data);
297291
return 0;
298292
}
299293

fs/hfsplus/hfsplus_fs.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,6 @@ struct hfsplus_inode_info {
214214
sector_t fs_blocks;
215215
u8 userflags; /* BSD user file flags */
216216
u32 subfolders; /* Subfolder count (HFSX only) */
217-
struct list_head open_dir_list;
218-
spinlock_t open_dir_lock;
219217
loff_t phys_size;
220218

221219
struct inode vfs_inode;
@@ -272,8 +270,7 @@ struct hfs_find_data {
272270
};
273271

274272
struct hfsplus_readdir_data {
275-
struct list_head list;
276-
struct file *file;
273+
loff_t pos;
277274
struct hfsplus_cat_key key;
278275
};
279276

fs/hfsplus/inode.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,6 @@ struct inode *hfsplus_new_inode(struct super_block *sb, struct inode *dir,
483483
simple_inode_init_ts(inode);
484484

485485
hip = HFSPLUS_I(inode);
486-
INIT_LIST_HEAD(&hip->open_dir_list);
487-
spin_lock_init(&hip->open_dir_lock);
488486
mutex_init(&hip->extents_lock);
489487
atomic_set(&hip->opencnt, 0);
490488
hip->extent_state = 0;

fs/hfsplus/super.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,6 @@ struct inode *hfsplus_iget(struct super_block *sb, unsigned long ino)
9191
HFSPLUS_I(inode)->fs_blocks = 0;
9292
HFSPLUS_I(inode)->userflags = 0;
9393
HFSPLUS_I(inode)->subfolders = 0;
94-
INIT_LIST_HEAD(&HFSPLUS_I(inode)->open_dir_list);
95-
spin_lock_init(&HFSPLUS_I(inode)->open_dir_lock);
9694
HFSPLUS_I(inode)->phys_size = 0;
9795

9896
if (inode->i_ino >= HFSPLUS_FIRSTUSER_CNID ||

0 commit comments

Comments
 (0)