mbox series

[0/4] scsi: ufs-debugfs: Add UFS Exception Event reporting

Message ID 20210119141542.3808-1-adrian.hunter@intel.com
Headers show
Series scsi: ufs-debugfs: Add UFS Exception Event reporting | expand

Message

Adrian Hunter Jan. 19, 2021, 2:15 p.m. UTC
Hi

Here are patches to add a tracepoint for UFS Exception Events and to allow
users to enable specific exception events without affecting the driver's
use of exception events.


Adrian Hunter (4):
      scsi: ufs: Add exception event tracepoint
      scsi: ufs: Add exception event definitions
      scsi: ufs-debugfs: Add user-defined exception_event_mask
      scsi: ufs-debugfs: Add user-defined exception event rate limiting

 drivers/scsi/ufs/ufs-debugfs.c | 90 ++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-debugfs.h |  2 +
 drivers/scsi/ufs/ufs.h         | 10 ++++-
 drivers/scsi/ufs/ufshcd.c      | 87 +++++++++++++++++++++++++---------------
 drivers/scsi/ufs/ufshcd.h      | 26 +++++++++++-
 include/trace/events/ufs.h     | 21 ++++++++++
 6 files changed, 201 insertions(+), 35 deletions(-)


Regards
Adrian

Comments

Adrian Hunter Feb. 3, 2021, 7:51 a.m. UTC | #1
On 19/01/21 4:15 pm, Adrian Hunter wrote:
> Hi

> 

> Here are patches to add a tracepoint for UFS Exception Events and to allow

> users to enable specific exception events without affecting the driver's

> use of exception events.

> 

> 

> Adrian Hunter (4):

>       scsi: ufs: Add exception event tracepoint

>       scsi: ufs: Add exception event definitions

>       scsi: ufs-debugfs: Add user-defined exception_event_mask

>       scsi: ufs-debugfs: Add user-defined exception event rate limiting

> 

>  drivers/scsi/ufs/ufs-debugfs.c | 90 ++++++++++++++++++++++++++++++++++++++++++

>  drivers/scsi/ufs/ufs-debugfs.h |  2 +

>  drivers/scsi/ufs/ufs.h         | 10 ++++-

>  drivers/scsi/ufs/ufshcd.c      | 87 +++++++++++++++++++++++++---------------

>  drivers/scsi/ufs/ufshcd.h      | 26 +++++++++++-

>  include/trace/events/ufs.h     | 21 ++++++++++

>  6 files changed, 201 insertions(+), 35 deletions(-)


Any comments on this?
Bean Huo Feb. 3, 2021, 8:25 a.m. UTC | #2
On Tue, 2021-01-19 at 16:15 +0200, Adrian Hunter wrote:
> For readability and completeness, add exception event definitions.

> 

> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Reviewed-by: Bean Huo <beanhuo@micron.com>
Bean Huo Feb. 3, 2021, 9:45 a.m. UTC | #3
On Tue, 2021-01-19 at 16:15 +0200, Adrian Hunter wrote:
> Allow users to enable specific exception events via debugfs.

> 

> The bits enabled by the driver ee_drv_ctrl are separated from the

> bits

> enabled by the user ee_usr_ctrl. The control mask ee_mask_ctrl is the

> logical-or of those two. A mutex is needed to ensure that the masks

> match

> what was written to the device.


Hallo Adrian

Would you like sharing the advantage of this debugfs node comparing to
sysfs node "attributes/exception_event_control(if it is writable)"?
what is the value of this?
Also, now I can disable/enable UFS event over ufs-bsg.

Bean
Adrian Hunter Feb. 3, 2021, 9:56 a.m. UTC | #4
On 3/02/21 11:45 am, Bean Huo wrote:
> On Tue, 2021-01-19 at 16:15 +0200, Adrian Hunter wrote:

>> Allow users to enable specific exception events via debugfs.

>>

>> The bits enabled by the driver ee_drv_ctrl are separated from the

>> bits

>> enabled by the user ee_usr_ctrl. The control mask ee_mask_ctrl is the

>> logical-or of those two. A mutex is needed to ensure that the masks

>> match

>> what was written to the device.

> 

> Hallo Adrian


Hi Bean

Thanks for the review

> 

> Would you like sharing the advantage of this debugfs node comparing to

> sysfs node "attributes/exception_event_control(if it is writable)"?


Primarily this is being done as a debug interface, but the user's exception
events also need to be kept separate from the driver's ones.

> what is the value of this?


To be able to determine if the UFS device is being affected by exception events.

> Also, now I can disable/enable UFS event over ufs-bsg.


That will be overwritten by the driver when it updates the e.g. bkops
control, or sometimes also suspend/resume.
Bean Huo Feb. 4, 2021, 2:58 p.m. UTC | #5
On Wed, 2021-02-03 at 11:56 +0200, Adrian Hunter wrote:
> > 

> > Hallo Adrian

> 

> Hi Bean

> 

> Thanks for the review

> 

> > 

> > Would you like sharing the advantage of this debugfs node comparing

> > to

> > sysfs node "attributes/exception_event_control(if it is writable)"?

> 

> Primarily this is being done as a debug interface, but the user's

> exception

> events also need to be kept separate from the driver's ones.

> 

> > what is the value of this?

> 

> To be able to determine if the UFS device is being affected by

> exception events.

> 

> > Also, now I can disable/enable UFS event over ufs-bsg.

> 

> That will be overwritten by the driver when it updates the e.g. bkops

> control, or sometimes also suspend/resume.


Hi Adrian
yes, I saw that, they are not tracked by driver.

I have one question that why "exception_event_mask" cannot represent
the current QUERY_ATTR_IDN_EE_CONTROL value? only after writing it.


thanks,
Bean
Adrian Hunter Feb. 4, 2021, 3:25 p.m. UTC | #6
On 4/02/21 4:58 pm, Bean Huo wrote:
> On Wed, 2021-02-03 at 11:56 +0200, Adrian Hunter wrote:

>>>

>>> Hallo Adrian

>>

>> Hi Bean

>>

>> Thanks for the review

>>

>>>

>>> Would you like sharing the advantage of this debugfs node comparing

>>> to

>>> sysfs node "attributes/exception_event_control(if it is writable)"?

>>

>> Primarily this is being done as a debug interface, but the user's

>> exception

>> events also need to be kept separate from the driver's ones.

>>

>>> what is the value of this?

>>

>> To be able to determine if the UFS device is being affected by

>> exception events.

>>

>>> Also, now I can disable/enable UFS event over ufs-bsg.

>>

>> That will be overwritten by the driver when it updates the e.g. bkops

>> control, or sometimes also suspend/resume.

> 

> Hi Adrian

> yes, I saw that, they are not tracked by driver.

> 

> I have one question that why "exception_event_mask" cannot represent

> the current QUERY_ATTR_IDN_EE_CONTROL value? only after writing it.


It represents only the user's exception events (ee_usr_mask), not the
driver's ones (ee_drv_mask) as well.  ee_usr_mask is updated after
successfully ensuring it is set on the device.