svn commit: r326394 - head/sys/fs/devfs

[Conrad Meyer]

This moves the M_WAITOK (sleepable) memory allocation under
dirlist_mtx, which is an ordinary mutex.  If you run this under
WITNESS, it will (rightfully) complain.  Please revert this change.

One possible way to accomplish the goal you have in mind is a fallback
path if the devfs_dir_findent_locked misses.  E.g.:

=====================================8<==================================
--- a/sys/fs/devfs/devfs_dir.c
+++ b/sys/fs/devfs/devfs_dir.c
@@ -96,19 +96,27 @@ devfs_dir_ref(const char *dir)
        if (*dir == '\0')
                return;

-       dle_new = malloc(sizeof(*dle), M_DEVFS4, M_WAITOK);
-       dle_new->dir = strdup(dir, M_DEVFS4);
-       dle_new->refcnt = 1;
+       dle_new = NULL;

+again:
        mtx_lock(&dirlist_mtx);
        dle = devfs_dir_findent_locked(dir);
        if (dle != NULL) {
                dle->refcnt++;
                mtx_unlock(&dirlist_mtx);
-               free(dle_new->dir, M_DEVFS4);
-               free(dle_new, M_DEVFS4);
+               if (dle_new != NULL) {
+                       free(dle_new->dir, M_DEVFS4);
+                       free(dle_new, M_DEVFS4);
+               }
                return;
        }
+       if (dle_new == NULL) {
+               mtx_unlock(&dirlist_mtx);
+               dle_new = malloc(sizeof(*dle), M_DEVFS4, M_WAITOK);
+               dle_new->dir = strdup(dir, M_DEVFS4);
+               dle_new->refcnt = 1;
+               goto again;
+       }
        LIST_INSERT_HEAD(&devfs_dirlist, dle_new, link);
        mtx_unlock(&dirlist_mtx);
 }
=====================================8<==================================

The downside of this method is we potentially have to take the dirlist
mutex twice.  An even more "optimized" approach would be to M_NOWAIT
the allocations inside the lock and fallback if those fail.  But that
introduces even more code complexity.

Best,
Conrad

On Thu, Nov 30, 2017 at 4:38 AM, Emmanuel Vadot <manu at freebsd.org> wrote:
> Author: manu
> Date: Thu Nov 30 12:38:42 2017
> New Revision: 326394
> URL: https://svnweb.freebsd.org/changeset/base/326394
>
> Log:
>   devfs: Avoid a malloc/free if we just need to increment the refcount
>
>   MFC after:    1 week
>   Sponsored by: Gandi.net
>
> Modified:
>   head/sys/fs/devfs/devfs_dir.c
>
> Modified: head/sys/fs/devfs/devfs_dir.c
> ==============================================================================
> --- head/sys/fs/devfs/devfs_dir.c       Thu Nov 30 12:22:15 2017        (r326393)
> +++ head/sys/fs/devfs/devfs_dir.c       Thu Nov 30 12:38:42 2017        (r326394)
> @@ -98,19 +98,18 @@ devfs_dir_ref(const char *dir)
>         if (*dir == '\0')
>                 return;
>
> -       dle_new = malloc(sizeof(*dle), M_DEVFS4, M_WAITOK);
> -       dle_new->dir = strdup(dir, M_DEVFS4);
> -       dle_new->refcnt = 1;
> -
>         mtx_lock(&dirlist_mtx);
>         dle = devfs_dir_findent_locked(dir);
>         if (dle != NULL) {
>                 dle->refcnt++;
>                 mtx_unlock(&dirlist_mtx);
> -               free(dle_new->dir, M_DEVFS4);
> -               free(dle_new, M_DEVFS4);
>                 return;
>         }
> +
> +       dle_new = malloc(sizeof(*dle), M_DEVFS4, M_WAITOK);
> +       dle_new->dir = strdup(dir, M_DEVFS4);
> +       dle_new->refcnt = 1;
> +
>         LIST_INSERT_HEAD(&devfs_dirlist, dle_new, link);
>         mtx_unlock(&dirlist_mtx);
>  }
>