diff mbox series

[2/2] block: add a partscan sysfs attribute for disks

Message ID 20240502130033.1958492-3-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series [1/2] block: add a disk_has_partscan helper | expand

Commit Message

Christoph Hellwig May 2, 2024, 1 p.m. UTC
This attribute reports if partition scanning is enabled for a given disk.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 Documentation/ABI/stable/sysfs-block | 10 ++++++++++
 block/genhd.c                        |  8 ++++++++
 2 files changed, 18 insertions(+)

Comments

Jens Axboe May 2, 2024, 5:05 p.m. UTC | #1
On 5/2/24 7:00 AM, Christoph Hellwig wrote:
> This attribute reports if partition scanning is enabled for a given disk.

This should, at least, have a reference to Lennart's posting, and
honestly a much better commit message as well. There's no reasoning
given here at all.

Maybe even a fixes tag and stable notation?
Christoph Hellwig May 3, 2024, 8:16 a.m. UTC | #2
On Thu, May 02, 2024 at 11:05:54AM -0600, Jens Axboe wrote:
> On 5/2/24 7:00 AM, Christoph Hellwig wrote:
> > This attribute reports if partition scanning is enabled for a given disk.
> 
> This should, at least, have a reference to Lennart's posting, and
> honestly a much better commit message as well. There's no reasoning
> given here at all.

I'm not sure I can come up with something much better, feel free to
throw in what you prefer.

> Maybe even a fixes tag and stable notation?

This is definitively not a Fixes as nothing it doesn't actually fix
any code.  It provides a proper interfaces for what was an abuse
of leaking internal bits out.
Keith Busch May 3, 2024, 9:12 a.m. UTC | #3
On Fri, May 03, 2024 at 10:16:12AM +0200, Christoph Hellwig wrote:
> On Thu, May 02, 2024 at 11:05:54AM -0600, Jens Axboe wrote:
> > On 5/2/24 7:00 AM, Christoph Hellwig wrote:
> > > This attribute reports if partition scanning is enabled for a given disk.
> > 
> > This should, at least, have a reference to Lennart's posting, and
> > honestly a much better commit message as well. There's no reasoning
> > given here at all.
> 
> I'm not sure I can come up with something much better, feel free to
> throw in what you prefer.

I think just explaining the "why" would be usesful for the git history.
How about this:

  Userspace had been unknowingly relying on a non-stable interface of
  kernel internals to determine if partition scanning is enabled for a
  given disk. Provide a stable interface for this purpose instead.

  Link: https://lore.kernel.org/linux-block/ZhQJf8mzq_wipkBH@gardel-login/
 
> > Maybe even a fixes tag and stable notation?
> 
> This is definitively not a Fixes as nothing it doesn't actually fix
> any code.  It provides a proper interfaces for what was an abuse
> of leaking internal bits out.

I kind of agree it's not a "Fixes:" in the traditional sense, but at a
"Cc: <stable>" sounds appropriate given the fallout.
Jens Axboe May 3, 2024, 2:57 p.m. UTC | #4
On 5/3/24 2:16 AM, Christoph Hellwig wrote:
> On Thu, May 02, 2024 at 11:05:54AM -0600, Jens Axboe wrote:
>> On 5/2/24 7:00 AM, Christoph Hellwig wrote:
>>> This attribute reports if partition scanning is enabled for a given disk.
>>
>> This should, at least, have a reference to Lennart's posting, and
>> honestly a much better commit message as well. There's no reasoning
>> given here at all.
> 
> I'm not sure I can come up with something much better, feel free to
> throw in what you prefer.

Really? You have literally nothing in there! It's good practice to
include reasoning for why the change is being done, particularly in this
case where there's quite strong reasonings for the addition.

>> Maybe even a fixes tag and stable notation?
> 
> This is definitively not a Fixes as nothing it doesn't actually fix
> any code.  It provides a proper interfaces for what was an abuse
> of leaking internal bits out.

I'm looking to bridge the gap between when we yanked the old garbage
interface and now added this one. So yeah it's not a pure fixes, but I
think we should still tie them together somehow so we can do a proper
stable backport of this.
Jens Axboe May 3, 2024, 2:59 p.m. UTC | #5
On 5/3/24 3:12 AM, Keith Busch wrote:
> On Fri, May 03, 2024 at 10:16:12AM +0200, Christoph Hellwig wrote:
>> On Thu, May 02, 2024 at 11:05:54AM -0600, Jens Axboe wrote:
>>> On 5/2/24 7:00 AM, Christoph Hellwig wrote:
>>>> This attribute reports if partition scanning is enabled for a given disk.
>>>
>>> This should, at least, have a reference to Lennart's posting, and
>>> honestly a much better commit message as well. There's no reasoning
>>> given here at all.
>>
>> I'm not sure I can come up with something much better, feel free to
>> throw in what you prefer.
> 
> I think just explaining the "why" would be usesful for the git history.
> How about this:
> 
>   Userspace had been unknowingly relying on a non-stable interface of
>   kernel internals to determine if partition scanning is enabled for a
>   given disk. Provide a stable interface for this purpose instead.
> 
>   Link: https://lore.kernel.org/linux-block/ZhQJf8mzq_wipkBH@gardel-login/

Yep this looks good, I can grab that.

>>> Maybe even a fixes tag and stable notation?
>>
>> This is definitively not a Fixes as nothing it doesn't actually fix
>> any code.  It provides a proper interfaces for what was an abuse
>> of leaking internal bits out.
> 
> I kind of agree it's not a "Fixes:" in the traditional sense, but at a
> "Cc: <stable>" sounds appropriate given the fallout.

It's definitely not a traditional kind of fixes, even a made-up tag
might be fine for this.

But probably just a Cc: stable@vger.kernel.org # version

and the added link would be good enough.
diff mbox series

Patch

diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block
index 1fe9a553c37b71..f0025d1c3d5acd 100644
--- a/Documentation/ABI/stable/sysfs-block
+++ b/Documentation/ABI/stable/sysfs-block
@@ -101,6 +101,16 @@  Description:
 		devices that support receiving integrity metadata.
 
 
+What:		/sys/block/<disk>/partscan
+Date:		May 2024
+Contact:	Christoph Hellwig <hch@lst.de>
+Description:
+		The /sys/block/<disk>/partscan files reports if partition
+		scanning is enabled for the disk.  It returns "1" if partition
+		scanning is enabled, or "0" if not.  The value type is a 32-bit
+		unsigned integer, but only "0" and "1" are valid values.
+
+
 What:		/sys/block/<disk>/<partition>/alignment_offset
 Date:		April 2009
 Contact:	Martin K. Petersen <martin.petersen@oracle.com>
diff --git a/block/genhd.c b/block/genhd.c
index 4b85963d09dbb4..dec2ee338fb44a 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1044,6 +1044,12 @@  static ssize_t diskseq_show(struct device *dev,
 	return sprintf(buf, "%llu\n", disk->diskseq);
 }
 
+static ssize_t partscan_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", disk_has_partscan(dev_to_disk(dev)));
+}
+
 static DEVICE_ATTR(range, 0444, disk_range_show, NULL);
 static DEVICE_ATTR(ext_range, 0444, disk_ext_range_show, NULL);
 static DEVICE_ATTR(removable, 0444, disk_removable_show, NULL);
@@ -1057,6 +1063,7 @@  static DEVICE_ATTR(stat, 0444, part_stat_show, NULL);
 static DEVICE_ATTR(inflight, 0444, part_inflight_show, NULL);
 static DEVICE_ATTR(badblocks, 0644, disk_badblocks_show, disk_badblocks_store);
 static DEVICE_ATTR(diskseq, 0444, diskseq_show, NULL);
+static DEVICE_ATTR(partscan, 0444, partscan_show, NULL);
 
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 ssize_t part_fail_show(struct device *dev,
@@ -1103,6 +1110,7 @@  static struct attribute *disk_attrs[] = {
 	&dev_attr_events_async.attr,
 	&dev_attr_events_poll_msecs.attr,
 	&dev_attr_diskseq.attr,
+	&dev_attr_partscan.attr,
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 	&dev_attr_fail.attr,
 #endif