linux-kernel - Re: [PATCH] Document genhd capability flags

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200306172309.GD25710@bombadil.infradead.org>
Date:   Fri, 6 Mar 2020 09:23:09 -0800
From:   Matthew Wilcox <willy@...radead.org>
To:     Stephen Kitt <steve@....org>
Cc:     Jens Axboe <axboe@...nel.dk>, Jan Kara <jack@...e.cz>,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Document genhd capability flags

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?