[PATCH RFC 31/38] proc: sysctl: resume readdir after the last name it returned

[Jan Kara]

On Mon 02-11-26 15:21:46, Christian Brauner wrote:
> proc_sys_readdir() counts the entries of a directory and resumes a
> listing at the number getdents64() stopped at. The entries are sorted
> by name, so one that is registered or unregistered in between shifts
> everything behind it by one. Listing /proc/sys/net/ipv4/conf with a
> buffer that holds ".", ".." and "all" and registering a device whose
> name sorts before "all" makes the next call return "all" a second time.
> Unregistering one that sorted before the position makes it skip an
> entry instead. So a listing that takes more than one call shows a name
> twice, or not at all. Even the 64k buffer of ls needs a second call to
> find the end of the directory, so a device that comes up right then
> repeats the last entry.
> 
> So remember the name of the last entry a call emitted in the file and
> resume at the first usable entry after it, like kernfs resumes after
> the node it stopped at. The entry itself can't be pinned across calls,
> unregistering waits for that. If the position was moved by a seek in
> between the listing counts from the start like before, seeking to an
> arbitrary position has never been better than that.
> 
> That also takes one walk down the tree to resume instead of a walk over
> every entry in front of the position with a round trip through
> sysctl_lock for each of them.
> 
> Fixes: 77b14db502cb ("[PATCH] sysctl: reimplement the sysctl proc support")
> Cc: stable at vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner at kernel.org>

Just one coding style nit below. Otherwise feel free to add:

Reviewed-by: Jan Kara <jack at suse.cz>

> +	if (!name) {
> +		after = rb_first(&dir->root);
> +	} else for (node = dir->root.rb_node; node;) {

This indentation looks too inventive to me :) I'd just use
	} else {
		for (...) {

								Honza
-- 
Jan Kara <jack at suse.com>
SUSE Labs, CR