Re: [PATCH] proc: Add a way to make proc files writable

[Qingjie Xing <xqjcool@gmail.com>]

Thank you for your feedback.

The motivation for this change is to simplify the creation and management of writable proc files. Many existing parts of the kernel codebase require writable proc files and currently need to set up a proc_ops struct with both read and write methods, which introduces redundancy and boilerplate code.

By providing proc_create_single_write_data() and the associated macro proc_create_single_write, we reduce the need for each caller to explicitly set up write methods, making the code simpler and more maintainable. This can benefit areas where writable proc files are commonly used, as it streamlines the implementation and improves readability.

In the future, we foresee potential use cases where other components of the kernel may need to adopt this approach for their writable proc files, thus justifying the addition of this interface.

for example:
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 34f68ef74b8f..e6fb61505e51 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -513,26 +513,13 @@ static int pgctrl_show(struct seq_file *seq, void *v)
        return 0;
 }

-static ssize_t pgctrl_write(struct file *file, const char __user *buf,
-                           size_t count, loff_t *ppos)
+static int pgctrl_write(struct file *file, char *data, size_t count)
 {
-       char data[128];
        struct pktgen_net *pn = net_generic(current->nsproxy->net_ns, pg_net_id);

        if (!capable(CAP_NET_ADMIN))
                return -EPERM;

-       if (count == 0)
-               return -EINVAL;
-
-       if (count > sizeof(data))
-               count = sizeof(data);
-
-       if (copy_from_user(data, buf, count))
-               return -EFAULT;
-
-       data[count - 1] = 0;    /* Strip trailing '\n' and terminate string */
-
        if (!strcmp(data, "stop"))
                pktgen_stop_all_threads(pn);
        else if (!strcmp(data, "start"))
@@ -542,22 +529,9 @@ static ssize_t pgctrl_write(struct file *file, const char __user *buf,
        else
                return -EINVAL;

-       return count;
-}
-
-static int pgctrl_open(struct inode *inode, struct file *file)
-{
-       return single_open(file, pgctrl_show, pde_data(inode));
+       return 0;
 }

-static const struct proc_ops pktgen_proc_ops = {
-       .proc_open      = pgctrl_open,
-       .proc_read      = seq_read,
-       .proc_lseek     = seq_lseek,
-       .proc_write     = pgctrl_write,
-       .proc_release   = single_release,
-};
-
 static int pktgen_if_show(struct seq_file *seq, void *v)
 {
        const struct pktgen_dev *pkt_dev = seq->private;
@@ -3982,7 +3956,7 @@ static int __net_init pg_net_init(struct net *net)
                pr_warn("cannot create /proc/net/%s\n", PG_PROC_DIR);
                return -ENODEV;
        }
-       pe = proc_create(PGCTRL, 0600, pn->proc_dir, &pktgen_proc_ops);
+       pe = proc_create_single_write(PGCTRL, 0600, pn->proc_dir, pgctrl_show, pgctrl_write);
        if (pe == NULL) {
                pr_err("cannot create %s procfs entry\n", PGCTRL);
                ret = -EINVAL;