[PATCH] Document genhd capability flags

[PATCH] Document genhd capability flags

[Stephen Kitt]

The kernel documentation includes a brief section about genhd
capabilities, but it turns out that the only documented
capability (GENHD_FL_MEDIA_CHANGE_NOTIFY) isn't used any more.

This patch removes that flag, and documents the rest, based on my
understanding of the current uses of these flags in the kernel. The
documentation is kept in the header file, alongside the declarations,
in the hope that it will be kept up-to-date in future; the kernel
documentation is changed to include the documentation generated from
the header file.

Because the ultimate goal is to provide some end-user
documentation (or end-administrator documentation), the comments are
perhaps more user-oriented than might be expected.

Signed-off-by: Stephen Kitt <steve@sk2.org>
---
 Documentation/block/capability.rst | 16 +++------
 include/linux/genhd.h              | 52 ++++++++++++++++++++++++++++--
 2 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/Documentation/block/capability.rst b/Documentation/block/capability.rst
index 2cf258d64bbe..26b9eccc48eb 100644
--- a/Documentation/block/capability.rst
+++ b/Documentation/block/capability.rst
@@ -2,17 +2,9 @@
 Generic Block Device Capability
 ===============================
 
-This file documents the sysfs file block/<disk>/capability
+This file documents the sysfs file ``block/<disk>/capability``.
 
-capability is a hex word indicating which capabilities a specific disk
-supports.  For more information on bits not listed here, see
-include/linux/genhd.h
+``capability`` is a hex word indicating which capabilities a specific
+disk supports:
 
-GENHD_FL_MEDIA_CHANGE_NOTIFY
-----------------------------
-
-Value: 4
-
-When this bit is set, the disk supports Asynchronous Notification
-of media change events.  These events will be broadcast to user
-space via kernel uevent.
+.. kernel-doc:: include/linux/genhd.h
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 6fbe58538ad6..d56aa4ccd34f 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -133,13 +133,59 @@ struct hd_struct {
 	struct rcu_work rcu_work;
 };
 
+/**
+ * DOC: genhd capability flags
+ *
+ * ``GENHD_FL_REMOVABLE`` (1): indicates that the block device gives
+ * access to removable media.
+ * When set, the device remains present even when media is not
+ * inserted.
+ * Must not be set for devices which are removed entirely when the
+ * media is removed.
+ *
+ * ``GENHD_FL_CD`` (8): the block device is a CD-ROM-style device.
+ * Affects responses to the ``CDROM_GET_CAPABILITY`` ioctl.
+ *
+ * ``GENHD_FL_UP`` (16): indicates that the block device is "up", with
+ * a similar meaning to network interfaces.
+ *
+ * ``GENHD_FL_SUPPRESS_PARTITION_INFO`` (32): don't include partition
+ * information in ``/proc/partitions`` or in the output of
+ * printk_all_partitions().
+ * Used for the null block device and some MMC devices.
+ *
+ * ``GENHD_FL_EXT_DEVT`` (64): the driver supports extended dynamic
+ * ``dev_t``, i.e. it wants extended device numbers
+ * (``BLOCK_EXT_MAJOR``).
+ * This affects the maximum number of partitions.
+ *
+ * ``GENHD_FL_NATIVE_CAPACITY`` (128): based on information in the
+ * partition table, the device's capacity has been extended to its
+ * native capacity; i.e. the device has hidden capacity used by one
+ * of the partitions (this is a flag used so that native capacity is
+ * only ever unlocked once).
+ *
+ * ``GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE`` (256): event polling is
+ * blocked whenever a writer holds an exclusive lock.
+ *
+ * ``GENHD_FL_NO_PART_SCAN`` (512): partition scanning is disabled.
+ * Used for loop devices in their default settings and some MMC
+ * devices.
+ *
+ * ``GENHD_FL_HIDDEN`` (1024): the block device is hidden; it doesn't
+ * produce events, doesn't appear in sysfs, and doesn't have an
+ * associated ``bdev``.
+ * Implies ``GENHD_FL_SUPPRESS_PARTITION_INFO`` and
+ * ``GENHD_FL_NO_PART_SCAN``.
+ * Used for multipath devices.
+ */
 #define GENHD_FL_REMOVABLE			1
-/* 2 is unused */
-#define GENHD_FL_MEDIA_CHANGE_NOTIFY		4
+/* 2 is unused (used to be GENHD_FL_DRIVERFS) */
+/* 4 is unused (used to be GENHD_FL_MEDIA_CHANGE_NOTIFY) */
 #define GENHD_FL_CD				8
 #define GENHD_FL_UP				16
 #define GENHD_FL_SUPPRESS_PARTITION_INFO	32
-#define GENHD_FL_EXT_DEVT			64 /* allow extended devt */
+#define GENHD_FL_EXT_DEVT			64
 #define GENHD_FL_NATIVE_CAPACITY		128
 #define GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE	256
 #define GENHD_FL_NO_PART_SCAN			512

base-commit: aeb542a1b5c507ea117d21c3e3e012ba16f065ac
-- 
2.20.1

Re: [PATCH] Document genhd capability flags

[Matthew Wilcox]

On Fri, Mar 06, 2020 at 06:16:21PM +0100, Stephen Kitt wrote:
> The kernel documentation includes a brief section about genhd
> capabilities, but it turns out that the only documented
> capability (GENHD_FL_MEDIA_CHANGE_NOTIFY) isn't used any more.
> 
> This patch removes that flag, and documents the rest, based on my
> understanding of the current uses of these flags in the kernel. The
> documentation is kept in the header file, alongside the declarations,
> in the hope that it will be kept up-to-date in future; the kernel
> documentation is changed to include the documentation generated from
> the header file.
> 
> Because the ultimate goal is to provide some end-user
> documentation (or end-administrator documentation), the comments are
> perhaps more user-oriented than might be expected.

Thank you!  I have some suggestions for further improvement ...

> -capability is a hex word indicating which capabilities a specific disk
> -supports.  For more information on bits not listed here, see
> -include/linux/genhd.h
> +``capability`` is a hex word indicating which capabilities a specific
> +disk supports:

``capability`` is a bitfield, printed in hexadecimal, indicating ...

> + * ``GENHD_FL_UP`` (16): indicates that the block device is "up", with
> + * a similar meaning to network interfaces.

Since we're printing it in hex, the value here should be displayed in hex
to help the end-user.  So ``GENHD_FL_UP`` (0x10), etc.

>  #define GENHD_FL_REMOVABLE			1
> -/* 2 is unused */
> -#define GENHD_FL_MEDIA_CHANGE_NOTIFY		4
> +/* 2 is unused (used to be GENHD_FL_DRIVERFS) */
> +/* 4 is unused (used to be GENHD_FL_MEDIA_CHANGE_NOTIFY) */
>  #define GENHD_FL_CD				8
>  #define GENHD_FL_UP				16
>  #define GENHD_FL_SUPPRESS_PARTITION_INFO	32
> -#define GENHD_FL_EXT_DEVT			64 /* allow extended devt */
> +#define GENHD_FL_EXT_DEVT			64
>  #define GENHD_FL_NATIVE_CAPACITY		128
>  #define GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE	256
>  #define GENHD_FL_NO_PART_SCAN			512

Maybe these should also be converted to hex to match?

Re: [PATCH] Document genhd capability flags

[Stephen Kitt]

[-- Attachment #1: Type: text/plain, Size: 2581 bytes --]

[Re-send, apparently this didn’t make it out of my computer.]

On Fri, 6 Mar 2020 09:23:09 -0800, Matthew Wilcox <willy@infradead.org> wrote:
> On Fri, Mar 06, 2020 at 06:16:21PM +0100, Stephen Kitt wrote:
> > The kernel documentation includes a brief section about genhd
> > capabilities, but it turns out that the only documented
> > capability (GENHD_FL_MEDIA_CHANGE_NOTIFY) isn't used any more.
> > 
> > This patch removes that flag, and documents the rest, based on my
> > understanding of the current uses of these flags in the kernel. The
> > documentation is kept in the header file, alongside the declarations,
> > in the hope that it will be kept up-to-date in future; the kernel
> > documentation is changed to include the documentation generated from
> > the header file.
> > 
> > Because the ultimate goal is to provide some end-user
> > documentation (or end-administrator documentation), the comments are
> > perhaps more user-oriented than might be expected.  
> 
> Thank you!  I have some suggestions for further improvement ...

Thanks for the quick review and the suggestions!

> > -capability is a hex word indicating which capabilities a specific disk
> > -supports.  For more information on bits not listed here, see
> > -include/linux/genhd.h
> > +``capability`` is a hex word indicating which capabilities a specific
> > +disk supports:  
> 
> ``capability`` is a bitfield, printed in hexadecimal, indicating ...
> 
> > + * ``GENHD_FL_UP`` (16): indicates that the block device is "up", with
> > + * a similar meaning to network interfaces.  
> 
> Since we're printing it in hex, the value here should be displayed in hex
> to help the end-user.  So ``GENHD_FL_UP`` (0x10), etc.
> 
> >  #define GENHD_FL_REMOVABLE			1
> > -/* 2 is unused */
> > -#define GENHD_FL_MEDIA_CHANGE_NOTIFY		4
> > +/* 2 is unused (used to be GENHD_FL_DRIVERFS) */
> > +/* 4 is unused (used to be GENHD_FL_MEDIA_CHANGE_NOTIFY) */
> >  #define GENHD_FL_CD				8
> >  #define GENHD_FL_UP				16
> >  #define GENHD_FL_SUPPRESS_PARTITION_INFO	32
> > -#define GENHD_FL_EXT_DEVT			64 /* allow extended
> > devt */ +#define GENHD_FL_EXT_DEVT			64
> >  #define GENHD_FL_NATIVE_CAPACITY		128
> >  #define GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE	256
> >  #define GENHD_FL_NO_PART_SCAN			512  
> 
> Maybe these should also be converted to hex to match?

Yes, these are all excellent ideas, I’ll follow up with a v2 (also updating
the terminology in capability.rst since this is about block devices, not only
disks).

Regards,

Stephen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]