rt-tests: hwlatdetect: Gracefully handle lack of /dev/cpu_dma_latency

From: Punit Agrawal <[email protected]>

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 <[email protected]>

Signed-off-by Punit Agrawal <[email protected]>

---
v1 -> v2:
* Correct Suresh's email

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

src/hwlatdetect/hwlatdetect.py +5 -2

index 12228f45f852..89537a6e371f 100755
@@ -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):

Signature · 1 line

2.32.0

Quoted 42 lines · Punit Agrawal

From: Punit Agrawal <[email protected]>

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 <[email protected]>

Signed-off-by Punit Agrawal <[email protected]>

---
v1 -> v2:
* Correct Suresh's email

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

src/hwlatdetect/hwlatdetect.py +5 -2

index 12228f45f852..89537a6e371f 100755
@@ -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):

Signature · 1 line

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

Hi John,

Thanks for having a look.

Quoted 47 lines · John Kacur

Quoted 42 lines · Punit Agrawal

From: Punit Agrawal <[email protected]>

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 <[email protected]>

Signed-off-by Punit Agrawal <[email protected]>

---
v1 -> v2:
* Correct Suresh's email

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

src/hwlatdetect/hwlatdetect.py +5 -2

index 12228f45f852..89537a6e371f 100755
@@ -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):

Signature · 1 line

2.32.0

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

Quoted 3 lines

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.

Quoted 13 lines

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

On Wed, 29 Sep 2021, Punit Agrawal wrote:

Quoted 6 lines

Quoted 5 lines

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.

Quoted 4 lines

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.