Re: [PATCH RFC 0/7] proc: move procfs to what kernfs does

[Christian Brauner <brauner@kernel.org>]

On Wed, 04 Nov 2026 22:23:51 +0000, Josef Bacik wrote:
> This is a summary of my Claude review of everything.  It didn't find
> anything wrong with the conversions in parts 3 to 6, the findings are in
> part 1 and on the procfs side.  1-3 are reproduced in a VM, 4-6 are from
> reading the code.

Thanks. This is useful. There's some stuff that I already took care in
v2 fwiw.

> 1) bugfixes 19/38 ("proc: resume readdir after the last name it
>    returned")
>
> proc_readdir_de() now returns 1 instead of 0 when there's nothing left
> to emit:
> [...]
> So at the end of the directory readdir(2) returns 1 without having
> written a dirent, and keeps doing that.  This is syscall 89 through

Indeed. Fixing that.

> 2) cleanup 5/5 ("proc: drop the _buf suffix from the new methods")
>
> 4/5 renames the old ->proc_read() so that "A module that sets one of
> them doesn't build anymore".  5/5 hands the name to the new method, and
> the old and the new prototype only differ in __user:
> [...]
> builds without a warning with W=1.  pde_set_flags() only refuses the
> release helpers for seq_file entries, which this isn't one of, so it
> registers too.  With a module that has exactly that loaded on top of the
> series there's no warning, open() works and every read() fails with
> [...]
> there without anybody noticing.  Either the read method keeps a name of
> its own or pde_set_flags() refuses seq_read, and either way this

Meh, let me think. I think not renaming them back to proc_{read,write}
at the end is ok. Not my favorite but it will cause the compiler to
yell.

> 3) bugfixes 38/38 ("selftests/proc: check that a faulting readlink()
>    doesn't hold up removal")
> [...]
> own.  The two lines want to move to core 08/16.

Fixed in v2.

> 4) core 04/16 ("proc: let procfs own the seq_file of seq_file entries")
> [...]
> return v after pde_seq_stop_pinned() takes care of both.

Gone in v2 as there's a simpler way to do it like kernfs.

> 5) bugfixes 35/38 ("proc: sysctl: don't hold the tables while readdir
>    copies to userspace")
> [...]
> lines that get rewritten.  Handling -ENOENT like d.skip would carry on
> after that name.  -ENOMEM probably wants to be returned instead of
> turning into EOF.

v2 does both.

> 6) stable tags, bugfixes 34/38 and 37/38
> [...]
> table registered.  35/38 applies cleanly without 34/38, so nothing will
> complain if only the tagged one gets picked up.

Right.

> ->free_inode()"), which went into 6.8.  Before that the inode drops the
> entry in ->evict_inode() without a grace period, and the pin is what
> keeps ->data around for a walker that got it from ->get_link().  Which

Right.

> - The cursors in bugfixes 19/38 and 31/38 hang off an open file and
>   anybody can have as many as they have fds, so GFP_KERNEL_ACCOUNT.

That's a generic problem for nearly all of procfs and is another series
I have but I don't want to get chased with pitchforks.

> - copy_to_user_fromio() has no callers left after drivers 22/23.

I'll kill it.

> - fs 07/27 says ntfs_set_label() refused a label that's too long with
>   -ENAMETOOLONG or -EFBIG.  It doesn't always, cpu_str.len is a u8 and
>   ntfs_nls_to_utf16() is allowed (PATH_MAX - 2) / 2 here, so 300
>   characters come out as 44 and pass the check.  That's an old ntfs3
>   bug, it's only the sentence in the changelog.

Right.

> Two more old ones that you might want in part 1, the series doesn't
> change either of them:
>
> - allocinfo can deadlock against a module load.

Yeah, that's in v2 and I think someone else also sent a fix for this.

> - snd_info_text_entry_write() allocates the larger buffer and frees the
>   old one without copying what's in it, so a text entry written in
>   pieces loses the start once it crosses a page.  It's been like that
>   since ffb73b08e794.

Yay, more bugfixes... You share the blame for more mail traffic now.

> This part is me. I have a selftest that my claude wrote to validate #1.
> If you want I can send it along, but we should probably have a test for
> that.

Right, I can probably also cook something up.