kernel - devfs cleanup
authorMatthew Dillon <dillon@apollo.backplane.com>
Thu, 22 Mar 2018 16:38:09 +0000 (09:38 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Thu, 22 Mar 2018 16:38:09 +0000 (09:38 -0700)
* staticize two functions, remove *_fff() from manual pages (now
  static).

* Adjust documentation.

* Rename internal function from devfs_clone_bitmap_resize() to
  devfs_clone_bitmap_extend().  Document the slight weirdness in
  the extent of the passed-in 'newchunks' parameter, but do not change
  the API.

Submitted-by: Aaron LI, with added code comments by Matt
Makefile_upgrade.inc
share/man/man9/Makefile
share/man/man9/make_autoclone_dev.9
sys/sys/devfs.h
sys/vfs/devfs/devfs_helper.c

index fa0fb0b..881f6db 100644 (file)
@@ -3378,6 +3378,7 @@ TO_REMOVE+=/etc/periodic/weekly/120.clean-kvmdb
 TO_REMOVE+=/etc/periodic/daily/470.status-named
 TO_REMOVE+=/etc/periodic/daily/430.status-rwho
 TO_REMOVE+=/etc/mtree/BSD.local.dist
+TO_REMOVE+=/usr/share/man/man9/devfs_clone_bitmap_fff.9.gz
 
 .if !defined(WANT_INSTALLER)
 TO_REMOVE+=/usr/sbin/dfuibe_installer
index 30edd18..66e90f8 100644 (file)
@@ -635,7 +635,6 @@ MLINKS+=lwbuf.9 lwbuf_alloc.9 \
 MLINKS+=make_autoclone_dev.9 destroy_autoclone_dev.9 \
        make_autoclone_dev.9 DEVFS_CLONE_BITMAP.9 \
        make_autoclone_dev.9 devfs_clone_bitmap_chk.9 \
-       make_autoclone_dev.9 devfs_clone_bitmap_fff.9 \
        make_autoclone_dev.9 devfs_clone_bitmap_get.9 \
        make_autoclone_dev.9 devfs_clone_bitmap_init.9 \
        make_autoclone_dev.9 devfs_clone_bitmap_put.9 \
index acb8ab0..122ded7 100644 (file)
@@ -37,7 +37,6 @@
 .Nm destroy_autoclone_dev ,
 .Nm devfs_clone_bitmap_init ,
 .Nm devfs_clone_bitmap_uninit ,
-.Nm devfs_clone_bitmap_fff ,
 .Nm devfs_clone_bitmap_chk ,
 .Nm devfs_clone_bitmap_set ,
 .Nm devfs_clone_bitmap_get ,
@@ -59,8 +58,6 @@
 .Ft void
 .Fn devfs_clone_bitmap_uninit "struct devfs_bitmap *bitmap"
 .Ft int
-.Fn devfs_clone_bitmap_fff "struct devfs_bitmap *bitmap"
-.Ft int
 .Fn devfs_clone_bitmap_chk "struct devfs_bitmap *bitmap" "int unit"
 .Ft void
 .Fn devfs_clone_bitmap_set "struct devfs_bitmap *bitmap" "int unit"
@@ -176,12 +173,6 @@ frees the memory associated with the specified clone
 .Fa bitmap
 and leaves it unusable.
 .Pp
-.Fn devfs_clone_bitmap_fff
-returns the first unused unit in
-.Fa bitmap .
-To use this unit, it has to be acquired by a call to
-.Fn devfs_clone_bitmap_set .
-.Pp
 .Fn devfs_clone_bitmap_chk
 checks if
 .Fa unit
@@ -193,8 +184,6 @@ marks
 in
 .Fa bitmap
 as used, so further calls to
-.Fn devfs_clone_bitmap_fff
-or
 .Fn devfs_clone_bitmap_get
 cannot retrieve it.
 If one intends to use a clone handler along with preallocated devices, it
@@ -213,13 +202,15 @@ as unused so further calls to
 or
 .Fn devfs_clone_bitmap_get
 can retrieve it again.
+It is recommended that the associated device be destroyed prior to
+making the
+.Fa unit
+available in the bitmap again, to avoid racing against a new clone.
 .Pp
 .Fn devfs_clone_bitmap_get
-is a shortcut to
-.Fn devfs_clone_bitmap_fff
-and
-.Fn devfs_clone_bitmap_set .
-It will return the first unused unit number and also mark it as used.
+will return the first unused unit number and also mark it as used
+in the
+.Fa bitmap .
 .Pp
 The
 .Fn DEVFS_DEFINE_CLONE_BITMAP
index 0e8b898..857fafd 100644 (file)
@@ -336,8 +336,6 @@ struct devfs_unit_hash {
 
 void devfs_clone_bitmap_init(struct devfs_bitmap *);
 void devfs_clone_bitmap_uninit(struct devfs_bitmap *);
-void devfs_clone_bitmap_resize(struct devfs_bitmap *, int);
-int devfs_clone_bitmap_fff(struct devfs_bitmap *);
 void devfs_clone_bitmap_set(struct devfs_bitmap *, int);
 int devfs_clone_bitmap_get(struct devfs_bitmap *, int);
 int devfs_clone_bitmap_chk(struct devfs_bitmap *, int);
@@ -442,7 +440,7 @@ void devfs_clear_cdevpriv(struct file *file);
 int devfs_WildCmp(const char *w, const char *s);
 int devfs_WildCaseCmp(const char *w, const char *s);
 
-#endif /* KERNEL */
+#endif /* KERNEL || _KERNEL_STRUCTURES */
 
 #define DEVFS_MNT_RULESET      0x01
 #define DEVFS_MNT_JAIL         0x02
index 5f8b084..4044439 100644 (file)
@@ -59,9 +59,13 @@ devfs_clone_bitmap_uninit(struct devfs_bitmap *bitmap)
        kfree(bitmap->bitmap, M_DEVFS);
 }
 
-
-void
-devfs_clone_bitmap_resize(struct devfs_bitmap *bitmap, int newchunks)
+/*
+ * Extend the bitmap past (not just up to) 'newchunks' chunks.  While
+ * probably not necessary, we go a little further and give ourselves
+ * one extra chunk's worth of bitmap beyond what is required.
+ */
+static void
+devfs_clone_bitmap_extend(struct devfs_bitmap *bitmap, int newchunks)
 {
        int oldchunks = bitmap->chunks;
 
@@ -73,8 +77,11 @@ devfs_clone_bitmap_resize(struct devfs_bitmap *bitmap, int newchunks)
               sizeof(u_long) * (bitmap->chunks - oldchunks));
 }
 
-
-int
+/*
+ * This helper function determines the next available free unit in the
+ * bitmap, extending the bitmap if necessary.  It cannot fail.
+ */
+static int
 devfs_clone_bitmap_fff(struct devfs_bitmap *bitmap)
 {
        u_long curbitmap;
@@ -85,7 +92,7 @@ devfs_clone_bitmap_fff(struct devfs_bitmap *bitmap)
 
        for (i = 0; i < chunks + 1; i++) {
                if (i == chunks)
-                       devfs_clone_bitmap_resize(bitmap, i);
+                       devfs_clone_bitmap_extend(bitmap, i);
                curbitmap = bitmap->bitmap[i];
 
                if (curbitmap) {
@@ -108,6 +115,8 @@ devfs_clone_bitmap_fff(struct devfs_bitmap *bitmap)
  * Caller wants to know if the specified unit has been allocated
  * or not.  Return 0, indicating that it has not been allocated.
  *
+ * Caller must hold a lock to prevent chk-to-set races.
+ *
  * (the bitmap implements 0=allocated, 1=not-allocated but the
  * return value is inverted).
  */
@@ -125,7 +134,14 @@ devfs_clone_bitmap_chk(struct devfs_bitmap *bitmap, int unit)
        return !((bitmap->bitmap[chunk]) & (1L << unit));
 }
 
-
+/*
+ * Unconditionally mark the specified unit as allocated in the bitmap.
+ *
+ * Caller must hold a lock to prevent chk-to-set races.  If the unit had
+ * never been allocated in the past, a token lock might not be sufficient
+ * to avoid races (if this function must extend the bitmap it could block
+ * temporary in kmalloc()).
+ */
 void
 devfs_clone_bitmap_set(struct devfs_bitmap *bitmap, int unit)
 {
@@ -135,14 +151,14 @@ devfs_clone_bitmap_set(struct devfs_bitmap *bitmap, int unit)
        unit -= chunk * (sizeof(u_long) * 8);
 
        if (chunk >= bitmap->chunks)
-               devfs_clone_bitmap_resize(bitmap, chunk);
+               devfs_clone_bitmap_extend(bitmap, chunk);
        bitmap->bitmap[chunk] &= ~(1L << unit);
 }
 
 /*
- * Deallocate a unit number.  We must synchronize any destroy_dev()'s
- * the device called before we clear the bitmap bit to avoid races
- * against a new clone.
+ * Unconditionally deallocate a unit number.  Caller must synchronize any
+ * destroy_dev()'s the device called before clearing the bitmap bit to
+ * avoid races against a new clone.
  */
 void
 devfs_clone_bitmap_put(struct devfs_bitmap *bitmap, int unit)
@@ -160,7 +176,11 @@ devfs_clone_bitmap_put(struct devfs_bitmap *bitmap, int unit)
 }
 
 /*
- * Allocate a unit number for a device
+ * Conditionally allocate a unit from the bitmap.  Returns -1 if no
+ * more units are available.
+ *
+ * Caller must hold a lock to avoid bitmap races.  Since *_fff() below
+ * pre-extends the bitmap as necessary, a token lock is sufficient.
  */
 int
 devfs_clone_bitmap_get(struct devfs_bitmap *bitmap, int limit)
@@ -174,4 +194,3 @@ devfs_clone_bitmap_get(struct devfs_bitmap *bitmap, int limit)
 
        return unit;
 }
-