Re: [PATCH] hfs: avoid reusing occupied CNIDs after deletion

[Hengyu Liang]

Hi Slava,

On Mon, Oct 19, 2026 at 04:29:16PM -0700, Viacheslav Dubeyko wrote:
> Frankly speaking, I don't see how your modification can fix anything.
> From my point of view, the current logic is correct. Probably, the
> call of this method is not in the proper place.

Sorry, my description was not clear enough. Here is the reproducer
with the actual CNIDs, on a freshly formatted volume:

    operation    allocated CNID    next_id afterwards
    mkdir z            16                 17
    create a           17                 18
    create b           18                 19

rm b calls hfs_correct_next_unused_CNID() with cnid 18 and next_id 19,
after b's file and thread records have been removed. The catalog leaf
records are then, in key order:

    key                type           CNID
    (1, "untitled")    dir              2
    (2, "")            dir thread
    (2, "a")           file            17
    (2, "z")           dir             16
    (16, "")           dir thread
    (17, "")           file thread

The backward scan skips the two thread records at the end and stops at
z, so it changes next_id from 19 to 17, although a still uses 17. The
next create gets CNID 17, hfs_cat_create() finds a's thread record and
returns -EEXIST. The correct value is 18.

So the call site is not the problem: the records are already gone when
the correction runs, and calling it earlier or later would not change
the result, because z still sorts last. The problem is that file and
directory records are ordered by (parent ID, name), not by their own
CNID, so the last one need not hold the largest remaining CNID. The
patch keeps the call where it is, so the drNxtCNID correction that
generic/736 needs stays, and only makes the scan take the maximum over
all of them.

The unpatched run in my previous mail matches this: the first create
of c failed, and the retry got CNID 18, so the failed attempt had been
given 17.

> I don't quite follow why do we need to change this check? If cnid + 1
> is smaller than the maximum one, then cnid + 1 is in use yet. We
> don't need to execute any additional logic. What am I missing here?

Nothing; both checks return early in that case. != additionally
returns early when cnid + 1 > next_id, where the cmpxchg in
hfs_set_next_unused_CNID() cannot succeed anyway. It is not needed for
the fix (in the example both checks go on to the correction), and I
can drop it in v2.

> Why do we need to check something for previous one? If the bigger
> cnid is in use, then we don't need to do anything here.

The lookup only runs when cnid + 1 == next_id, i.e. the deleted CNID
was the largest one allocated, so no bigger CNID is in use. If
cnid - 1 still exists, it is the largest remaining CNID, and next_id
can be set to cnid without walking the catalog: in the example,
deleting 18 finds 17 and sets next_id to 18. Because the fallback now
walks the whole catalog instead of stopping at the first record, this
keeps the common case cheap. Only if the lookup returns -ENOENT does
the patch scan all records, which also covers files without the
optional thread record.

Thanks,
Hengyu