usb: gadget: f_fs: simplify ffs_dev name handling

Currently ffs_dev::name can be either allocated by the client of
the ffs_dev structure or by the f_fs.c core itself.  The former
is used by g_ffs while the latter happens with configfs.

Historically, g_ffs did not need to allocate separate buffer for
the name so what is now f_fs.c core never cared about freeing
that space.  With configfs the name needs to be copied since the
memory is not guaranteed to be availeble after ffs_set_inst_name
finishes.

The complication is therefore here to avoid allocations in the
g_ffs case but it complicates the code inproportinally to
benefits it provides.  In particular, g_ffs is considered
‘legacy’ so optimising for its sake is unlikely to be worth the
effort.

With that observation in mind, simplify the code by unifying the
code paths in g_ffs and configfs paths.  Furthermore, instead of
allocating a new buffer for the name, simply embed it in the
ffs_dev structure.  This further makes the memory management
less convoluted and error-prone.

The configfs interface for functionfs imposed a limit of 40
characters for the name so this results in a 41-byte buffer
added to the structure.  (For short names this may lead to
wasted memory but the actual amount is not immediately obvious
and depends on pointer size and which slab buckets the structure
and name would fall into).

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index a0085571824d9b4352c7245625a30c3248d789f5..db6935c51d91923f5da267d3c1fa5f8156516c2b 100644 (file)
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -246,7 +246,6 @@ EXPORT_SYMBOL_GPL(ffs_lock);
 
 static struct ffs_dev *_ffs_find_dev(const char *name);
 static struct ffs_dev *_ffs_alloc_dev(void);
-static int _ffs_name_dev(struct ffs_dev *dev, const char *name);
 static void _ffs_free_dev(struct ffs_dev *dev);
 static void *ffs_acquire_dev(const char *dev_name);
 static void ffs_release_dev(struct ffs_data *ffs_data);
@@ -3302,9 +3301,10 @@ static struct ffs_dev *_ffs_do_find_dev(const char *name)
 {
        struct ffs_dev *dev;
 
+       if (!name)
+               return NULL;
+
        list_for_each_entry(dev, &ffs_devices, entry) {
-               if (!dev->name || !name)
-                       continue;
                if (strcmp(dev->name, name) == 0)
                        return dev;
        }
@@ -3380,42 +3380,11 @@ static void ffs_free_inst(struct usb_function_instance *f)
        kfree(opts);
 }
 
-#define MAX_INST_NAME_LEN      40
-
 static int ffs_set_inst_name(struct usb_function_instance *fi, const char *name)
 {
-       struct f_fs_opts *opts;
-       char *ptr;
-       const char *tmp;
-       int name_len, ret;
-
-       name_len = strlen(name) + 1;
-       if (name_len > MAX_INST_NAME_LEN)
+       if (strlen(name) >= FIELD_SIZEOF(struct ffs_dev, name))
                return -ENAMETOOLONG;
-
-       ptr = kstrndup(name, name_len, GFP_KERNEL);
-       if (!ptr)
-               return -ENOMEM;
-
-       opts = to_f_fs_opts(fi);
-       tmp = NULL;
-
-       ffs_dev_lock();
-
-       tmp = opts->dev->name_allocated ? opts->dev->name : NULL;
-       ret = _ffs_name_dev(opts->dev, ptr);
-       if (ret) {
-               kfree(ptr);
-               ffs_dev_unlock();
-               return ret;
-       }
-       opts->dev->name_allocated = true;
-
-       ffs_dev_unlock();
-
-       kfree(tmp);
-
-       return 0;
+       return ffs_name_dev(to_f_fs_opts(fi)->dev, name);
 }
 
 static struct usb_function_instance *ffs_alloc_inst(void)
@@ -3545,32 +3514,19 @@ static struct ffs_dev *_ffs_alloc_dev(void)
        return dev;
 }
 
-/*
- * ffs_lock must be taken by the caller of this function
- * The caller is responsible for "name" being available whenever f_fs needs it
- */
-static int _ffs_name_dev(struct ffs_dev *dev, const char *name)
+int ffs_name_dev(struct ffs_dev *dev, const char *name)
 {
        struct ffs_dev *existing;
+       int ret = 0;
 
-       existing = _ffs_do_find_dev(name);
-       if (existing)
-               return -EBUSY;
-
-       dev->name = name;
-
-       return 0;
-}
+       ffs_dev_lock();
 
-/*
- * The caller is responsible for "name" being available whenever f_fs needs it
- */
-int ffs_name_dev(struct ffs_dev *dev, const char *name)
-{
-       int ret;
+       existing = _ffs_do_find_dev(name);
+       if (!existing)
+               strlcpy(dev->name, name, ARRAY_SIZE(dev->name));
+       else if (existing != dev)
+               ret = -EBUSY;
 
-       ffs_dev_lock();
-       ret = _ffs_name_dev(dev, name);
        ffs_dev_unlock();
 
        return ret;
@@ -3600,8 +3556,6 @@ EXPORT_SYMBOL_GPL(ffs_single_dev);
 static void _ffs_free_dev(struct ffs_dev *dev)
 {
        list_del(&dev->entry);
-       if (dev->name_allocated)
-               kfree(dev->name);
 
        /* Clear the private_data pointer to stop incorrect dev access */
        if (dev->ffs_data)

diff --git a/drivers/usb/gadget/function/u_fs.h b/drivers/usb/gadget/function/u_fs.h
index 4b6969451cdc28dcfabb4db8806a403df1199e0d..7ff485af3bee943c03e29a4da19299db29cbf453 100644 (file)
--- a/drivers/usb/gadget/function/u_fs.h
+++ b/drivers/usb/gadget/function/u_fs.h
@@ -39,15 +39,16 @@
 struct f_fs_opts;
 
 struct ffs_dev {
-       const char *name;
-       bool name_allocated;
-       bool mounted;
-       bool desc_ready;
-       bool single;
        struct ffs_data *ffs_data;
        struct f_fs_opts *opts;
        struct list_head entry;
 
+       char name[41];
+
+       bool mounted;
+       bool desc_ready;
+       bool single;
+
        int (*ffs_ready_callback)(struct ffs_data *ffs);
        void (*ffs_closed_callback)(struct ffs_data *ffs);
        void *(*ffs_acquire_dev_callback)(struct ffs_dev *dev);