Re: [PATCH 1/2] fs: Supply dir_context.count as readdir buffer size hint

[Christophe JAILLET]

Hi,

a few nitpicks below to reduce the diffpatch with unrelated changes. 
(trainling spaces)


Le 14/03/2025 à 23:16, Jaco Kroon a écrit :
> This was provided by Miklos <miklos@xxxxxxxxxx> via LKML on 2023/07/27
> subject "Re: [PATCH] fuse: enable larger read buffers for readdir.".
>
> This is thus preperation for an improved fuse readdir() patch.  The

s/preperation/preparation/

> description he provided:
>
> "The best strategy would be to find the optimal buffer size based on the size of
> the userspace buffer.  Putting that info into struct dir_context doesn't sound
> too complicated...
>
> "Here's a patch.  It doesn't touch readdir.  Simply setting the fuse buffer size
> to the userspace buffer size should work, the record sizes are similar (fuse's
> is slightly larger than libc's, so no overflow should ever happen)."

...

> @@ -239,7 +240,7 @@ SYSCALL_DEFINE3(old_readdir, unsigned int, fd,
>   
>   /*
>    * New, all-improved, singing, dancing, iBCS2-compliant getdents()
> - * interface.
> + * interface.

Unrelated change.

>    */
>   struct linux_dirent {
>   	unsigned long	d_ino;

...

> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 2788df98080f..1e426e2b5999 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -308,7 +308,7 @@ struct iattr {
>    */
>   #define FILESYSTEM_MAX_STACK_DEPTH 2
>   
> -/**
> +/**

Unrelated change.

>    * enum positive_aop_returns - aop return codes with specific semantics
>    *
>    * @AOP_WRITEPAGE_ACTIVATE: Informs the caller that page writeback has
> @@ -318,7 +318,7 @@ struct iattr {
>    * 			    be a candidate for writeback again in the near
>    * 			    future.  Other callers must be careful to unlock
>    * 			    the page if they get this return.  Returned by
> - * 			    writepage();
> + * 			    writepage();

Unrelated change.

>    *
>    * @AOP_TRUNCATED_PAGE: The AOP method that was handed a locked page has
>    *  			unlocked it and the page might have been truncated.
> @@ -1151,8 +1151,8 @@ struct file *get_file_active(struct file **f);
>   
>   #define	MAX_NON_LFS	((1UL<<31) - 1)
>   
> -/* Page cache limit. The filesystems should put that into their s_maxbytes
> -   limits, otherwise bad things can happen in VM. */
> +/* Page cache limit. The filesystems should put that into their s_maxbytes
> +   limits, otherwise bad things can happen in VM. */

Unrelated change.

>   #if BITS_PER_LONG==32
>   #define MAX_LFS_FILESIZE	((loff_t)ULONG_MAX << PAGE_SHIFT)
>   #elif BITS_PER_LONG==64
> @@ -2073,6 +2073,13 @@ typedef bool (*filldir_t)(struct dir_context *, const char *, int, loff_t, u64,
>   struct dir_context {
>   	filldir_t actor;
>   	loff_t pos;
> +	/*
> +	 * Filesystems MUST NOT MODIFY count, but may use as a hint:
> +	 * 0	    unknown
> +	 * > 0      space in buffer (assume at least one entry)
> +	 * INT_MAX  unlimited
> +	 */
> +	int count;
>   };
>   
>   /*
> @@ -2609,7 +2616,7 @@ int sync_inode_metadata(struct inode *inode, int wait);
>   struct file_system_type {
>   	const char *name;
>   	int fs_flags;
> -#define FS_REQUIRES_DEV		1
> +#define FS_REQUIRES_DEV		1

Unrelated change.

>   #define FS_BINARY_MOUNTDATA	2
>   #define FS_HAS_SUBTYPE		4
>   #define FS_USERNS_MOUNT		8	/* Can be mounted by userns root */
> @@ -3189,7 +3196,7 @@ ssize_t __kernel_read(struct file *file, void *buf, size_t count, loff_t *pos);
>   extern ssize_t kernel_write(struct file *, const void *, size_t, loff_t *);
>   extern ssize_t __kernel_write(struct file *, const void *, size_t, loff_t *);
>   extern struct file * open_exec(const char *);
> -
> +

Unrelated change.

>   /* fs/dcache.c -- generic fs support functions */
>   extern bool is_subdir(struct dentry *, struct dentry *);
>   extern bool path_is_under(const struct path *, const struct path *);