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

[Jaco Kroon <jaco@uls.co.za>]

ACK.  Fixed locally.

Thank you.

Will give a few more days before re-sending.

Kind regards,
Jaco

On 2025/03/29 11:20, Christophe JAILLET wrote:

> 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@szeredi.hu> 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 *);
>