Re: [PATCH v5 1/3] libfs: Add directory operations for stable offsets

[Chuck Lever III <chuck.lever@oracle.com>]

> On Jun 28, 2023, at 5:27 AM, Christian Brauner <brauner@kernel.org> wrote:
> 
> On Tue, Jun 27, 2023 at 04:53:09PM -0400, Chuck Lever wrote:
>> From: Chuck Lever <chuck.lever@oracle.com>
>> 
>> Create a vector of directory operations in fs/libfs.c that handles
>> directory seeks and readdir via stable offsets instead of the
>> current cursor-based mechanism.
>> 
>> For the moment these are unused.
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
> 
> Could we just drop the "stable_" prefix and just have:
> 
> // internal helpers
> offset_dir_emit()
> offset_get()
> offset_set()
> offset_dir_llseek()
> find_next_offset()
> I'd also collapse offset_iterate_dir() into offset_readdir().

We need to keep offset_iterate_dir(). XA_STATE() works only as a
declaration, so it has to be at the top of a function.

Everything else I can do.


> // exported and published helpers
> simple_offset_init()
> simple_offset_add()
> simple_offset_rename_exchange()
> simple_offset_destroy()
> 
> struct offset_ctx *(*get_offset)(struct inode *inode);
> 
> const struct file_operations simple_offset_dir_operations;
> 
>> fs/libfs.c         |  252 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/fs.h |   19 ++++
>> 2 files changed, 271 insertions(+)
>> 
>> diff --git a/fs/libfs.c b/fs/libfs.c
>> index 89cf614a3271..9940dce049e6 100644
>> --- a/fs/libfs.c
>> +++ b/fs/libfs.c
>> @@ -239,6 +239,258 @@ const struct inode_operations simple_dir_inode_operations = {
>> };
>> EXPORT_SYMBOL(simple_dir_inode_operations);
>> 
>> +static struct stable_offset_ctx *stable_ctx_get(struct inode *inode)
>> +{
>> + return inode->i_op->get_so_ctx(inode);
>> +}
> 
> I would suggest to get rid of this helper. It just needlessly hides that
> all we do is inode->i_op->().

--
Chuck Lever