Re: [PATCH v2 2/9] eventfs: adding eventfs dir add functions

[Ajay Kaher <akaher@vmware.com>]

> On 10-May-2023, at 4:55 PM, Ajay Kaher <akaher@vmware.com> wrote:
>> 
>> Hi,
>> eventfs_add_subsystem_dir() and eventfs_add_dir() are almost the same,
>> how about extract a common help function to simplify them, like:
>> 
>> +static struct eventfs_file *__eventfs_add_dir(const char *name,
>> +                                             struct dentry *d_parent,
>> +                                             struct eventfs_inode
>> *ei_parent,
>> +                                             struct rw_semaphore
>> *eventfs_rwsem)
>> +{
>> +       struct eventfs_file *ef;
>> +
>> +       ef = kzalloc(sizeof(*ef), GFP_KERNEL);
>> +       if (!ef)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       ef->ei = kzalloc(sizeof(*ef->ei), GFP_KERNEL);
>> +       if (!ef->ei) {
>> +               kfree(ef);
>> +               return ERR_PTR(-ENOMEM);
>> +       }
>> +
>> +       INIT_LIST_HEAD(&ef->ei->e_top_files);
>> +
>> +       ef->name = kstrdup(name, GFP_KERNEL);
>> +       if (!ef->name) {
>> +               kfree(ef->ei);
>> +               kfree(ef);
>> +               return ERR_PTR(-ENOMEM);
>> +       }
>> +
>> +       ef->mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
>> +       ef->iop = &eventfs_root_dir_inode_operations;
>> +       ef->fop = &eventfs_file_operations;
>> +       ef->dentry = NULL;
>> +       ef->created = false;
>> +       ef->d_parent = d_parent;
>> +       ef->data = eventfs_rwsem;
>> +
>> +       eventfs_down_write(eventfs_rwsem);
>> +       list_add_tail(&ef->list, &ei_parent->e_top_files);
>> +       eventfs_up_write(eventfs_rwsem);
>> +       return ef;
>> +}
>> +
>> +struct eventfs_file *eventfs_add_subsystem_dir(const char *name,
>> +                                              struct dentry *parent,
>> +                                              struct rw_semaphore
>> *eventfs_rwsem)
>> +{
>> +       struct tracefs_inode *ti_parent;
>> +       struct eventfs_inode *ei_parent;
>> +
>> +       if (!parent)
>> +               return ERR_PTR(-EINVAL);
>> +       ti_parent = get_tracefs(parent->d_inode);
>> +       ei_parent = ti_parent->private;
>> +       return __eventfs_add_dir(name, parent, ei_parent, eventfs_rwsem);
>> +}
>> +
>> +struct eventfs_file *eventfs_add_dir(const char *name,
>> +                                    struct eventfs_file *ef_parent,
>> +                                    struct rw_semaphore *eventfs_rwsem)
>> +{
>> +       if (!ef_parent)
>> +               return ERR_PTR(-EINVAL);
>> +       return __eventfs_add_dir(name, NULL, ef_parent->ei, eventfs_rwsem);
>> +}
> 
> Sounds good. Thanks for sharing code snippet. I will consider in v3.
> 

Hi Zheng, I have moved common code to eventfs_prepare_ef() in v3 3/10, 4/10:
https://lore.kernel.org/all/1685610013-33478-1-git-send-email-akaher@vmware.com/

-Ajay