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

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....

-Ajay