| Message ID | 20250318194111.19419-2-James.Bottomley@HansenPartnership.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | create simple libfs directory iterator and make efivarfs use it | expand |
Hi James, Thanks for persisting with this. On Tue, 18 Mar 2025 at 20:44, James Bottomley <James.Bottomley@hansenpartnership.com> wrote: > > No functional change. Preparatory to using the internal function to > iterate a directory with just a dentry not a file. > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > --- > fs/libfs.c | 41 +++++++++++++++++++++++++++++------------ > 1 file changed, 29 insertions(+), 12 deletions(-) > > diff --git a/fs/libfs.c b/fs/libfs.c > index 8444f5cc4064..816bfe6c0430 100644 > --- a/fs/libfs.c > +++ b/fs/libfs.c > @@ -189,28 +189,21 @@ EXPORT_SYMBOL(dcache_dir_lseek); > * for ramfs-type trees they can't go away without unlink() or rmdir(), > * both impossible due to the lock on directory. > */ > - > -int dcache_readdir(struct file *file, struct dir_context *ctx) > +static void internal_readdir(struct dentry *dentry, struct dentry *cursor, It might make sense to make this __always_inline, so that the callback argument is guaranteed to become a compile time constant when the caller is dcache_readdir(). Otherwise, the indirect call overhead might impact its performance. > + void *data, bool start, > + bool (*callback)(void *, struct dentry *)) > { > - struct dentry *dentry = file->f_path.dentry; > - struct dentry *cursor = file->private_data; > struct dentry *next = NULL; > struct hlist_node **p; > > - if (!dir_emit_dots(file, ctx)) > - return 0; > - > - if (ctx->pos == 2) > + if (start) > p = &dentry->d_children.first; > else > p = &cursor->d_sib.next; > > while ((next = scan_positives(cursor, p, 1, next)) != NULL) { > - if (!dir_emit(ctx, next->d_name.name, next->d_name.len, > - d_inode(next)->i_ino, > - fs_umode_to_dtype(d_inode(next)->i_mode))) > + if (!callback(data, next)) > break; > - ctx->pos++; > p = &next->d_sib.next; > } > spin_lock(&dentry->d_lock); > @@ -219,6 +212,30 @@ int dcache_readdir(struct file *file, struct dir_context *ctx) > hlist_add_before(&cursor->d_sib, &next->d_sib); > spin_unlock(&dentry->d_lock); > dput(next); > +} > + > +static bool dcache_readdir_callback(void *data, struct dentry *entry) > +{ > + struct dir_context *ctx = data; > + > + if (!dir_emit(ctx, entry->d_name.name, entry->d_name.len, > + d_inode(entry)->i_ino, > + fs_umode_to_dtype(d_inode(entry)->i_mode))) > + return false; > + ctx->pos++; > + return true; > +} > + > +int dcache_readdir(struct file *file, struct dir_context *ctx) > +{ > + struct dentry *dentry = file->f_path.dentry; > + struct dentry *cursor = file->private_data; > + > + if (!dir_emit_dots(file, ctx)) > + return 0; > + > + internal_readdir(dentry, cursor, ctx, ctx->pos == 2, > + dcache_readdir_callback); > > return 0; > } > -- > 2.43.0 >
On Tue, 2025-03-18 at 22:32 +0100, Ard Biesheuvel wrote: > Hi James, > > Thanks for persisting with this. Heh, well, it is starting to feel a bit like the swamp I can't get out of ... > On Tue, 18 Mar 2025 at 20:44, James Bottomley > <James.Bottomley@hansenpartnership.com> wrote: > > > > No functional change. Preparatory to using the internal function > > to iterate a directory with just a dentry not a file. > > > > Signed-off-by: James Bottomley > > <James.Bottomley@HansenPartnership.com> > > --- > > fs/libfs.c | 41 +++++++++++++++++++++++++++++------------ > > 1 file changed, 29 insertions(+), 12 deletions(-) > > > > diff --git a/fs/libfs.c b/fs/libfs.c > > index 8444f5cc4064..816bfe6c0430 100644 > > --- a/fs/libfs.c > > +++ b/fs/libfs.c > > @@ -189,28 +189,21 @@ EXPORT_SYMBOL(dcache_dir_lseek); > > * for ramfs-type trees they can't go away without unlink() or > > rmdir(), > > * both impossible due to the lock on directory. > > */ > > - > > -int dcache_readdir(struct file *file, struct dir_context *ctx) > > +static void internal_readdir(struct dentry *dentry, struct dentry > > *cursor, > > It might make sense to make this __always_inline, so that the > callback argument is guaranteed to become a compile time constant > when the caller is dcache_readdir(). Otherwise, the indirect call > overhead might impact its performance. I was hoping the compiler would pick that up ... especially as it's a tail call, but I can add it if necessary. Regards, James
diff --git a/fs/libfs.c b/fs/libfs.c index 8444f5cc4064..816bfe6c0430 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -189,28 +189,21 @@ EXPORT_SYMBOL(dcache_dir_lseek); * for ramfs-type trees they can't go away without unlink() or rmdir(), * both impossible due to the lock on directory. */ - -int dcache_readdir(struct file *file, struct dir_context *ctx) +static void internal_readdir(struct dentry *dentry, struct dentry *cursor, + void *data, bool start, + bool (*callback)(void *, struct dentry *)) { - struct dentry *dentry = file->f_path.dentry; - struct dentry *cursor = file->private_data; struct dentry *next = NULL; struct hlist_node **p; - if (!dir_emit_dots(file, ctx)) - return 0; - - if (ctx->pos == 2) + if (start) p = &dentry->d_children.first; else p = &cursor->d_sib.next; while ((next = scan_positives(cursor, p, 1, next)) != NULL) { - if (!dir_emit(ctx, next->d_name.name, next->d_name.len, - d_inode(next)->i_ino, - fs_umode_to_dtype(d_inode(next)->i_mode))) + if (!callback(data, next)) break; - ctx->pos++; p = &next->d_sib.next; } spin_lock(&dentry->d_lock); @@ -219,6 +212,30 @@ int dcache_readdir(struct file *file, struct dir_context *ctx) hlist_add_before(&cursor->d_sib, &next->d_sib); spin_unlock(&dentry->d_lock); dput(next); +} + +static bool dcache_readdir_callback(void *data, struct dentry *entry) +{ + struct dir_context *ctx = data; + + if (!dir_emit(ctx, entry->d_name.name, entry->d_name.len, + d_inode(entry)->i_ino, + fs_umode_to_dtype(d_inode(entry)->i_mode))) + return false; + ctx->pos++; + return true; +} + +int dcache_readdir(struct file *file, struct dir_context *ctx) +{ + struct dentry *dentry = file->f_path.dentry; + struct dentry *cursor = file->private_data; + + if (!dir_emit_dots(file, ctx)) + return 0; + + internal_readdir(dentry, cursor, ctx, ctx->pos == 2, + dcache_readdir_callback); return 0; }
No functional change. Preparatory to using the internal function to iterate a directory with just a dentry not a file. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- fs/libfs.c | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-)