All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rt-tests: hwlatdetect: Gracefully handle lack of /dev/cpu_dma_latency
@ 2021-09-28  8:17 Punit Agrawal
  2021-09-28  8:59 ` [PATCH v2] " Punit Agrawal
  0 siblings, 1 reply; 6+ messages in thread
From: Punit Agrawal @ 2021-09-28  8:17 UTC (permalink / raw)
  To: jkacur; +Cc: linux-rt-users, daniel.sangorrin, Punit Agrawal, Suresh Hegde

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

On systems where cpu idle is disabled to reduce latencies,
"/dev/cpu_dma_latency" does not exist and leads to the following
exception report from python when using hwlatdetect.py -

    FileNotFoundError: [Errno 2] No such file or directory: '/dev/cpu_dma_latency

Update hwlatdetect to check whether the file exists before attemping
to write values to it.

Reported-by: Suresh Hegde <suresh1.hegde@toshiba.co.jp>
Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
 src/hwlatdetect/hwlatdetect.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/hwlatdetect/hwlatdetect.py b/src/hwlatdetect/hwlatdetect.py
index 12228f45f852..89537a6e371f 100755
--- a/src/hwlatdetect/hwlatdetect.py
+++ b/src/hwlatdetect/hwlatdetect.py
@@ -216,8 +216,11 @@ class Detector(object):
     # use c_states_on() to close the file descriptor and re-enable c-states
     #
     def c_states_off(self):
-        self.dma_latency_handle = os.open("/dev/cpu_dma_latency", os.O_WRONLY)
-        os.write(self.dma_latency_handle, b'\x00\x00\x00\x00')
+        if os.path.exists("/dev/cpu_dma_latency"):
+            self.dma_latency_handle = os.open("/dev/cpu_dma_latency", os.O_WRONLY)
+            os.write(self.dma_latency_handle, b'\x00\x00\x00\x00')
+        else:
+            self.dma_latency_handle = None
         debug("c-states disabled")
 
     def c_states_on(self):
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2] rt-tests: hwlatdetect: Gracefully handle lack of /dev/cpu_dma_latency
  2021-09-28  8:17 [PATCH] rt-tests: hwlatdetect: Gracefully handle lack of /dev/cpu_dma_latency Punit Agrawal
@ 2021-09-28  8:59 ` Punit Agrawal
  2021-09-28 14:34   ` John Kacur
  0 siblings, 1 reply; 6+ messages in thread
From: Punit Agrawal @ 2021-09-28  8:59 UTC (permalink / raw)
  To: jkacur; +Cc: linux-rt-users, daniel.sangorrin, Punit Agrawal, Suresh Hegde

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

On systems where cpu idle is disabled to reduce latencies,
"/dev/cpu_dma_latency" does not exist and leads to the following
exception report from python when using hwlatdetect.py -

    FileNotFoundError: [Errno 2] No such file or directory: '/dev/cpu_dma_latency

Update hwlatdetect to check whether the file exists before attemping
to write values to it.

Reported-by: Suresh Hegde <suresh.c11@toshiba-tsip.com>
Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
v1 -> v2:
* Correct Suresh's email

 src/hwlatdetect/hwlatdetect.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/hwlatdetect/hwlatdetect.py b/src/hwlatdetect/hwlatdetect.py
index 12228f45f852..89537a6e371f 100755
--- a/src/hwlatdetect/hwlatdetect.py
+++ b/src/hwlatdetect/hwlatdetect.py
@@ -216,8 +216,11 @@ class Detector(object):
     # use c_states_on() to close the file descriptor and re-enable c-states
     #
     def c_states_off(self):
-        self.dma_latency_handle = os.open("/dev/cpu_dma_latency", os.O_WRONLY)
-        os.write(self.dma_latency_handle, b'\x00\x00\x00\x00')
+        if os.path.exists("/dev/cpu_dma_latency"):
+            self.dma_latency_handle = os.open("/dev/cpu_dma_latency", os.O_WRONLY)
+            os.write(self.dma_latency_handle, b'\x00\x00\x00\x00')
+        else:
+            self.dma_latency_handle = None
         debug("c-states disabled")
 
     def c_states_on(self):
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] rt-tests: hwlatdetect: Gracefully handle lack of /dev/cpu_dma_latency
  2021-09-28  8:59 ` [PATCH v2] " Punit Agrawal
@ 2021-09-28 14:34   ` John Kacur
  2021-09-28 22:48     ` Punit Agrawal
  0 siblings, 1 reply; 6+ messages in thread
From: John Kacur @ 2021-09-28 14:34 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-rt-users, daniel.sangorrin, Punit Agrawal, Suresh Hegde



On Tue, 28 Sep 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> On systems where cpu idle is disabled to reduce latencies,
> "/dev/cpu_dma_latency" does not exist and leads to the following
> exception report from python when using hwlatdetect.py -
> 
>     FileNotFoundError: [Errno 2] No such file or directory: '/dev/cpu_dma_latency
> 
> Update hwlatdetect to check whether the file exists before attemping
> to write values to it.
> 
> Reported-by: Suresh Hegde <suresh.c11@toshiba-tsip.com>
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
> v1 -> v2:
> * Correct Suresh's email
> 
>  src/hwlatdetect/hwlatdetect.py | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/hwlatdetect/hwlatdetect.py b/src/hwlatdetect/hwlatdetect.py
> index 12228f45f852..89537a6e371f 100755
> --- a/src/hwlatdetect/hwlatdetect.py
> +++ b/src/hwlatdetect/hwlatdetect.py
> @@ -216,8 +216,11 @@ class Detector(object):
>      # use c_states_on() to close the file descriptor and re-enable c-states
>      #
>      def c_states_off(self):
> -        self.dma_latency_handle = os.open("/dev/cpu_dma_latency", os.O_WRONLY)
> -        os.write(self.dma_latency_handle, b'\x00\x00\x00\x00')
> +        if os.path.exists("/dev/cpu_dma_latency"):
> +            self.dma_latency_handle = os.open("/dev/cpu_dma_latency", os.O_WRONLY)
> +            os.write(self.dma_latency_handle, b'\x00\x00\x00\x00')
> +        else:
> +            self.dma_latency_handle = None
>          debug("c-states disabled")
>  
>      def c_states_on(self):
> -- 
> 2.32.0
> 
> 

Thanks! I have a couple of nit-picky suggestions.

If you initialize self.dma_latency_handle = None in the init method of 
class Detector, then you can drop the else part of your check to see if 
the file exists.

Also, in the situation you describe, the debug messages should probably be 
hoisted up under the "if" part, and that goes for method c_states_on() 
too, as they are not relevant.

You might as well fix the comments above the code too,
openinging -> opening
writeing -> writing
And edit that one line not to over flow 80 chars.

Thanks!

John


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] rt-tests: hwlatdetect: Gracefully handle lack of /dev/cpu_dma_latency
  2021-09-28 14:34   ` John Kacur
@ 2021-09-28 22:48     ` Punit Agrawal
  2021-09-28 23:25       ` John Kacur
  0 siblings, 1 reply; 6+ messages in thread
From: Punit Agrawal @ 2021-09-28 22:48 UTC (permalink / raw)
  To: John Kacur; +Cc: linux-rt-users, daniel.sangorrin, Punit Agrawal, Suresh Hegde

Hi John,

Thanks for having a look.

John Kacur <jkacur@redhat.com> writes:

> On Tue, 28 Sep 2021, Punit Agrawal wrote:
>
>> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> 
>> On systems where cpu idle is disabled to reduce latencies,
>> "/dev/cpu_dma_latency" does not exist and leads to the following
>> exception report from python when using hwlatdetect.py -
>> 
>>     FileNotFoundError: [Errno 2] No such file or directory: '/dev/cpu_dma_latency
>> 
>> Update hwlatdetect to check whether the file exists before attemping
>> to write values to it.
>> 
>> Reported-by: Suresh Hegde <suresh.c11@toshiba-tsip.com>
>> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> ---
>> v1 -> v2:
>> * Correct Suresh's email
>> 
>>  src/hwlatdetect/hwlatdetect.py | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/src/hwlatdetect/hwlatdetect.py b/src/hwlatdetect/hwlatdetect.py
>> index 12228f45f852..89537a6e371f 100755
>> --- a/src/hwlatdetect/hwlatdetect.py
>> +++ b/src/hwlatdetect/hwlatdetect.py
>> @@ -216,8 +216,11 @@ class Detector(object):
>>      # use c_states_on() to close the file descriptor and re-enable c-states
>>      #
>>      def c_states_off(self):
>> -        self.dma_latency_handle = os.open("/dev/cpu_dma_latency", os.O_WRONLY)
>> -        os.write(self.dma_latency_handle, b'\x00\x00\x00\x00')
>> +        if os.path.exists("/dev/cpu_dma_latency"):
>> +            self.dma_latency_handle = os.open("/dev/cpu_dma_latency", os.O_WRONLY)
>> +            os.write(self.dma_latency_handle, b'\x00\x00\x00\x00')
>> +        else:
>> +            self.dma_latency_handle = None
>>          debug("c-states disabled")
>>  
>>      def c_states_on(self):
>> -- 
>> 2.32.0
>> 
>> 
>
> Thanks! I have a couple of nit-picky suggestions.
>
> If you initialize self.dma_latency_handle = None in the init method of 
> class Detector, then you can drop the else part of your check to see if 
> the file exists.

I can do that - but then I wonder if there is any harm in initialising
the file handle in the init method itself too. That way
c_states_[off|on]() can focus on doing their thing after checking the
handle is not null.

I'll send a patch in sometime addressing this and below comments.

>
> Also, in the situation you describe, the debug messages should probably be 
> hoisted up under the "if" part, and that goes for method c_states_on() 
> too, as they are not relevant.
>
> You might as well fix the comments above the code too,
> openinging -> opening
> writeing -> writing
> And edit that one line not to over flow 80 chars.
>
> Thanks!
>
> John

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] rt-tests: hwlatdetect: Gracefully handle lack of /dev/cpu_dma_latency
  2021-09-28 22:48     ` Punit Agrawal
@ 2021-09-28 23:25       ` John Kacur
  2021-09-29  0:10         ` Punit Agrawal
  0 siblings, 1 reply; 6+ messages in thread
From: John Kacur @ 2021-09-28 23:25 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: linux-rt-users, daniel.sangorrin, Punit Agrawal, Suresh Hegde



On Wed, 29 Sep 2021, Punit Agrawal wrote:

> Hi John,
> 
> Thanks for having a look.
> 
> John Kacur <jkacur@redhat.com> writes:
> 
> > On Tue, 28 Sep 2021, Punit Agrawal wrote:
> >
> >> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> >> 
> >> On systems where cpu idle is disabled to reduce latencies,
> >> "/dev/cpu_dma_latency" does not exist and leads to the following
> >> exception report from python when using hwlatdetect.py -
> >> 
> >>     FileNotFoundError: [Errno 2] No such file or directory: '/dev/cpu_dma_latency
> >> 
> >> Update hwlatdetect to check whether the file exists before attemping
> >> to write values to it.
> >> 
> >> Reported-by: Suresh Hegde <suresh.c11@toshiba-tsip.com>
> >> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> >> ---
> >> v1 -> v2:
> >> * Correct Suresh's email
> >> 
> >>  src/hwlatdetect/hwlatdetect.py | 7 +++++--
> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/src/hwlatdetect/hwlatdetect.py b/src/hwlatdetect/hwlatdetect.py
> >> index 12228f45f852..89537a6e371f 100755
> >> --- a/src/hwlatdetect/hwlatdetect.py
> >> +++ b/src/hwlatdetect/hwlatdetect.py
> >> @@ -216,8 +216,11 @@ class Detector(object):
> >>      # use c_states_on() to close the file descriptor and re-enable c-states
> >>      #
> >>      def c_states_off(self):
> >> -        self.dma_latency_handle = os.open("/dev/cpu_dma_latency", os.O_WRONLY)
> >> -        os.write(self.dma_latency_handle, b'\x00\x00\x00\x00')
> >> +        if os.path.exists("/dev/cpu_dma_latency"):
> >> +            self.dma_latency_handle = os.open("/dev/cpu_dma_latency", os.O_WRONLY)
> >> +            os.write(self.dma_latency_handle, b'\x00\x00\x00\x00')
> >> +        else:
> >> +            self.dma_latency_handle = None
> >>          debug("c-states disabled")
> >>  
> >>      def c_states_on(self):
> >> -- 
> >> 2.32.0
> >> 
> >> 
> >
> > Thanks! I have a couple of nit-picky suggestions.
> >
> > If you initialize self.dma_latency_handle = None in the init method of 
> > class Detector, then you can drop the else part of your check to see if 
> > the file exists.
> 
> I can do that - but then I wonder if there is any harm in initialising
> the file handle in the init method itself too. That way
> c_states_[off|on]() can focus on doing their thing after checking the
> handle is not null.

I mean I suppose it would work, but I wouldn't consider it great style, 
I think of the init as a place for basic declarations and initializations.

Anyway, I'll have a look at whatever way you decide to go.

> 
> I'll send a patch in sometime addressing this and below comments.
> 
> >
> > Also, in the situation you describe, the debug messages should probably be 
> > hoisted up under the "if" part, and that goes for method c_states_on() 
> > too, as they are not relevant.
> >
> > You might as well fix the comments above the code too,
> > openinging -> opening
> > writeing -> writing
> > And edit that one line not to over flow 80 chars.
> >
> > Thanks!
> >
> > John
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] rt-tests: hwlatdetect: Gracefully handle lack of /dev/cpu_dma_latency
  2021-09-28 23:25       ` John Kacur
@ 2021-09-29  0:10         ` Punit Agrawal
  0 siblings, 0 replies; 6+ messages in thread
From: Punit Agrawal @ 2021-09-29  0:10 UTC (permalink / raw)
  To: John Kacur; +Cc: linux-rt-users, daniel.sangorrin, Punit Agrawal, Suresh Hegde

John Kacur <jkacur@redhat.com> writes:

> On Wed, 29 Sep 2021, Punit Agrawal wrote:
>

[...]

>> > Thanks! I have a couple of nit-picky suggestions.
>> >
>> > If you initialize self.dma_latency_handle = None in the init method of 
>> > class Detector, then you can drop the else part of your check to see if 
>> > the file exists.
>> 
>> I can do that - but then I wonder if there is any harm in initialising
>> the file handle in the init method itself too. That way
>> c_states_[off|on]() can focus on doing their thing after checking the
>> handle is not null.
>
> I mean I suppose it would work, but I wouldn't consider it great style, 
> I think of the init as a place for basic declarations and
> initializations.

I don't have a strong preference here - so I'll go with your suggestion.

Thanks.

> Anyway, I'll have a look at whatever way you decide to go.
>

[...]


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-29  0:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28  8:17 [PATCH] rt-tests: hwlatdetect: Gracefully handle lack of /dev/cpu_dma_latency Punit Agrawal
2021-09-28  8:59 ` [PATCH v2] " Punit Agrawal
2021-09-28 14:34   ` John Kacur
2021-09-28 22:48     ` Punit Agrawal
2021-09-28 23:25       ` John Kacur
2021-09-29  0:10         ` Punit Agrawal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.