Re: [PATCH 2/2] hfsplus: do not end readdir at i_size after an unlink

[Hengyu Liang]

On Thu, 2026-11-12 at 12:44 -0800, Viacheslav Dubeyko wrote:
> Frankly speaking, I don't quite follow what is the issue of the fix. Do
> you mean that we create and delete folder in one thread and execute the
> readdir in another one? Or do you mean that application has several
> threads and one thread do readdir but another one delete file by file?
> How this deletion is executed? I mean which files are deleted?

Sorry, the commit message was too short. There is only one thread. It
reads a part of the directory with getdents64(), deletes the files that
this call returned, and then calls getdents64() again on the same fd.
This is what "rm -r" of BusyBox does.

> Even if we delete files in folder then i_size is dropping gradually. If
> we traversing the folder's content and ctx->pos >= inode->i_size, then
> it sounds that we already showed the existing content. Do you mean that
> if we deleted the range that we already processed, then we miss the
> rest of undeleted items?

Yes, exactly. ctx->pos counts the entries returned since the start, and
i_size is the number of entries that exist now (valence + 2). When the
entries that were already returned are deleted, ctx->pos stays and
i_size drops, so ctx->pos can reach i_size while entries are still
unread.

A small example is a directory with 4 files and a buffer that takes 4
entries:

  getdents64() returns ".", "..", f1, f2   ctx->pos = 4, i_size = 6
                                           rd->pos = 4, rd->key = f3
  unlink(f1)                               i_size = 5
  unlink(f2)                               i_size = 4
  getdents64() returns 0                   f_pos (4) >= i_size (4)

f3 and f4 were never returned. Before commit 4b0496432844,
hfsplus_delete_cat() moved f_pos back for every deleted entry before
the saved key, so the second call started with f_pos = 2 and returned
f3 and f4.

With 1500 files and a 32768 byte buffer, the first call returns 1022
files. After they are deleted, i_size is 480 and f_pos is 1024, so the
second call returns 0 and 478 files are left.

> > +	bool resume = rd && rd->pos == file->f_pos;
>
> Is it possible that rd is NULL here?

Yes. file->private_data is allocated at the end of the first
hfsplus_readdir() call that stops in the middle of the directory, so it
is NULL before that. The current code has the same check
("if (rd && rd->pos == ctx->pos)").

> I don't really follow how the resume variable can help at all.
> Probably, I am missing the case. Could you please explain the issue at
> first?

resume tells apart the two ways the function finds the next record:
from the key that the previous call saved (rd->key), or by counting
records from the start of the directory with ctx->pos. It is the
condition the current code already uses to choose between them, only
computed at the start of the function.

When the call continues from the saved key, ctx->pos no longer says
where the walk is in the directory, because entries returned by
earlier calls may have been deleted. So comparing it with i_size
cannot tell if the directory has ended. The patch skips the three
i_size checks for such a call, and ends the walk when the next record
belongs to another parent or the catalog has no more records.

A call that finds its position with ctx->pos (the first call, or a
call after lseek()) keeps the i_size checks and the "walked past end
of dir" error as they are.

> I really dislike the comment in the variable declaration section.

I will move it out of the declarations in v2.

The HFS patch (1/2) is the same case.

Thanks,
Hengyu