mtip32xx: Remove 'registers' and 'flags' from sysfs
PATCH 1/2 mtip32xx: Remove 'registers' and 'flags' from sysfs
This patch removes entries 'registers' and 'flags' from sysfs. Updated ABI file to reflect this change.
--- Documentation/ABI/testing/sysfs-block-rssd | 21 ------ drivers/block/mtip32xx/mtip32xx.c | 92 +--------------------------- 2 files changed, 1 insertions(+), 112 deletions(-)
Documentation/ABI/testing/sysfs-block-rssd +0 -21
@@ -1,26 +1,5 @@ -What: /sys/block/rssd*/registers -Date: March 2012 -KernelVersion: 3.3 -Contact: Asai Thambi S P <[email protected]> -Description: This is a read-only file. Dumps below driver information and - hardware registers. - - S ACTive - - Command Issue - - Completed - - PORT IRQ STAT - - HOST IRQ STAT - - Allocated - - Commands in Q - What: /sys/block/rssd*/status Date: April 2012 KernelVersion: 3.4 Contact: Asai Thambi S P <[email protected]> Description: This is a read-only file. Indicates the status of the device. - -What: /sys/block/rssd*/flags -Date: May 2012 -KernelVersion: 3.5 -Contact: Asai Thambi S P <[email protected]> -Description: This is a read-only file. Dumps the flags in port and driver - data structure
drivers/block/mtip32xx/mtip32xx.c +1 -91
127 lines not on this page · raw message
Signature · 1 line
1.7.1
Quoted 3 lines · Asai Thambi S P
This patch removes entries 'registers' and 'flags' from sysfs. Updated ABI file to reflect this change.
Applied, thanks.
Signature · 1 line
Jens Axboe
Quoted 6 lines · Asai Thambi S P
This patch removes entries 'registers' and 'flags' from sysfs. Updated ABI file to reflect this change.
Reported-by Greg Kroah-Hartman <[email protected]>Signed-off-by Asai Thambi S P <[email protected]>
Much nicer, thanks for doing this:
But, one question on a different sysfs file:
Quoted 5 lines
What: /sys/block/rssd*/status Date: April 2012 KernelVersion: 3.4 Contact: Asai Thambi S P <[email protected]> Description: This is a read-only file. Indicates the status of the device.
What "status" is this showing? Why is this a sysfs file? Who needs/wants it?
Also, if you really want to properly create sysfs files, use the default attributes for the driver. As it is, you are creating them after userspace is notified about the device showing up, which races with the creation of your additional file(s). Use the properly driver core field to have the core create, and remove them, automatically, which saves you code, and fixes bugs you didn't realize you had :)
thanks,
greg k-h
Quoted 14 lines · Greg KH
Quoted 6 lines · Asai Thambi S P
This patch removes entries 'registers' and 'flags' from sysfs. Updated ABI file to reflect this change.
Reported-by Greg Kroah-Hartman <[email protected]>Signed-off-by Asai Thambi S P <[email protected]>Much nicer, thanks for doing this:
But, one question on a different sysfs file:
Quoted 8 lines
Quoted 5 lines
What: /sys/block/rssd*/status Date: April 2012 KernelVersion: 3.4 Contact: Asai Thambi S P <[email protected]> Description: This is a read-only file. Indicates the status of the device.What "status" is this showing? Why is this a sysfs file? Who needs/wants it?
This shows the device status - online, write_protect or thermal_shutdown. This would be used by management application.
Quoted 7 lines
Also, if you really want to properly create sysfs files, use the default attributes for the driver. As it is, you are creating them after userspace is notified about the device showing up, which races with the creation of your additional file(s). Use the properly driver core field to have the core create, and remove them, automatically, which saves you code, and fixes bugs you didn't realize you had :)
I have not thought about this. Thanks for the insight. I will look into it.
--
Regards,
Asai Thambi
Quoted 22 lines · Asai Thambi S P
Quoted 20 lines · Greg KH
Quoted 6 lines · Asai Thambi S P
This patch removes entries 'registers' and 'flags' from sysfs. Updated ABI file to reflect this change.
Reported-by Greg Kroah-Hartman <[email protected]>Signed-off-by Asai Thambi S P <[email protected]>Much nicer, thanks for doing this:
But, one question on a different sysfs file:
Quoted 5 lines
What: /sys/block/rssd*/status Date: April 2012 KernelVersion: 3.4 Contact: Asai Thambi S P <[email protected]> Description: This is a read-only file. Indicates the status of the device.
Quoted 6 lines
Quoted 2 lines
What "status" is this showing? Why is this a sysfs file? Who needs/wants it?
This shows the device status - online,
write_protectorthermal_shutdown. This would be used by management application.
Is it used by a management application? Shouldn't such a tool use the "standard" block device status files instead? I thought we exported that information already in the /sys/block/* files.
thanks,
greg k-h
Quoted 27 lines · Greg KH
Quoted 26 lines · Asai Thambi S P
Quoted 22 lines · Greg KH
Quoted 6 lines · Asai Thambi S P
This patch removes entries 'registers' and 'flags' from sysfs. Updated ABI file to reflect this change.
Reported-by Greg Kroah-Hartman <[email protected]>Signed-off-by Asai Thambi S P <[email protected]>Much nicer, thanks for doing this:
But, one question on a different sysfs file:
Quoted 5 lines
What: /sys/block/rssd*/status Date: April 2012 KernelVersion: 3.4 Contact: Asai Thambi S P <[email protected]> Description: This is a read-only file. Indicates the status of the device.What "status" is this showing? Why is this a sysfs file? Who needs/wants it?
Quoted 7 lines
Quoted 2 lines
This shows the device status - online,
write_protectorthermal_shutdown. This would be used by management application.Is it used by a management application? Shouldn't such a tool use the "standard" block device status files instead? I thought we exported that information already in the
/sys/block/*files.
device status is not exported by standard block interface.
--
Regards,
Asai Thambi
Quoted 32 lines · Asai Thambi S P
Quoted 30 lines · Greg KH
Quoted 28 lines · Asai Thambi S P
Quoted 14 lines · Greg KH
Quoted 6 lines · Asai Thambi S P
This patch removes entries 'registers' and 'flags' from sysfs. Updated ABI file to reflect this change.
Reported-by Greg Kroah-Hartman <[email protected]>Signed-off-by Asai Thambi S P <[email protected]>Much nicer, thanks for doing this:
But, one question on a different sysfs file:
Quoted 8 lines
Quoted 5 lines
What: /sys/block/rssd*/status Date: April 2012 KernelVersion: 3.4 Contact: Asai Thambi S P <[email protected]> Description: This is a read-only file. Indicates the status of the device.What "status" is this showing? Why is this a sysfs file? Who needs/wants it?
This shows the device status - online,
write_protectorthermal_shutdown. This would be used by management application.
Quoted 7 lines
Quoted 4 lines
Is it used by a management application? Shouldn't such a tool use the "standard" block device status files instead? I thought we exported that information already in the
/sys/block/*files.device status is not exported by standard block interface.
Ok, but don't you think it should be? Why make this unique to only your driver?
greg k-h
Quoted 39 lines · Greg KH
Quoted 38 lines · Asai Thambi S P
Quoted 34 lines · Greg KH
Quoted 22 lines · Asai Thambi S P
Quoted 20 lines · Greg KH
Quoted 6 lines · Asai Thambi S P
This patch removes entries 'registers' and 'flags' from sysfs. Updated ABI file to reflect this change.
Reported-by Greg Kroah-Hartman <[email protected]>Signed-off-by Asai Thambi S P <[email protected]>Much nicer, thanks for doing this:
But, one question on a different sysfs file:
Quoted 5 lines
What: /sys/block/rssd*/status Date: April 2012 KernelVersion: 3.4 Contact: Asai Thambi S P <[email protected]> Description: This is a read-only file. Indicates the status of the device.Quoted 6 lines
Quoted 2 lines
What "status" is this showing? Why is this a sysfs file? Who needs/wants it?
This shows the device status - online,
write_protectorthermal_shutdown. This would be used by management application.Is it used by a management application? Shouldn't such a tool use the "standard" block device status files instead? I thought we exported that information already in the
/sys/block/*files.
Quoted 5 lines
Quoted 1 line
device status is not exported by standard block interface.
Ok, but don't you think it should be? Why make this unique to only your driver?
It is unique to our driver because it exports information that is unique. In addition to write protect and thermal status, in future it would export the status of what we call “ftl rebuild”, which is a rebuilding of the logical to physical mappings internal to the flash controller. If and when other devices need to export device status, it could be moved to standard block interface.
--
Regards,
Asai Thambi