0

I have a couple of questions on this piece of code, running on a jetson nano:

#include "stdio.h"
#include "unistd.h"
#include "stdlib.h"

float gputemp = 0;
float cputemp = 0;
int count = 0;

int main() {
    char* cpu;
    char* gpu;
    cpu = (char*)malloc(sizeof(char)*6);
    gpu = (char*)malloc(sizeof(char)*6);
    

    while (1) {
        FILE* fcputemp = fopen("/sys/devices/virtual/thermal/thermal_zone1/temp", "r");
        FILE* fgputemp = fopen("/sys/devices/virtual/thermal/thermal_zone2/temp","r");
        if (!fcputemp || !fgputemp ) {
            printf("Something went wrong\n");
            exit(EXIT_FAILURE);
        }
        
        cputemp = atoi(fgets(cpu, 6, fcputemp))/1000;
        gputemp = atoi(fgets(gpu, 6, fgputemp))/1000;
        
        printf("\rCpu : %.2f, Gpu : %.2f. Elapsed time : %d", cputemp, gputemp, count);
        fflush(stdout);
        
        fclose(fcputemp);
        fclose(fgputemp);
        
        count++;
        sleep(1);
    }
}

Here I have to open, get the temperatures, and then close the file each loop iteration in order to get valid data (and dont segfault). My concern here is the number of (expensive) kernel switches needed to do this.

I know that premature optimization is evil, but there is another way (or maybe the RIGHT way) to do that, opening the file only once?

And why the sensor interface (the file) cant update itself if I have it open?

P.S: Yes, I know, I didnt free cpu nor gpu variables, this is only "demo" code (just watch how i measure the time passed lol)

Rob
  • 15
  • 6
  • Why the `malloc`? Just use a straight-up array unless these structures are too big to fit on the stack. You're just using those as scratch buffers, so `char buffer[1024]` should suffice. – tadman Jun 26 '20 at 17:49
  • @tadman Yes, i used an array but i was getting a segfault, so i changed it with a malloc but the problem wasnt there, so that is a leftover. Again, this is only a demo code, i didnt try to be as rigourus as possible. – Rob Jun 26 '20 at 18:01
  • If you're getting a segfault the first step should be to drop it into a debugger and find out why, not just randomly switch things around. – tadman Jun 26 '20 at 18:04
  • A) Why are `cputemp` and `gputemp` globals? B) Why are you using `atoi` instead of `atof` if you want floating-point values? – tadman Jun 26 '20 at 18:05
  • @tadman, going to gdb was the second thing i did, and i agree, should be step one. Actually thank you for B), changed from int to float in the process and didnt change that ( i also dont think i need floats for that, the precision of the thermometer should be in the order of degrees, i have to check) . For A),again, its DEMO code, didnt even try to make this the definition of good practices – Rob Jun 26 '20 at 18:12
  • They call them best-practices not because they take more time, but because they save you a lot of hassle when things go wrong. If you're ever stuck on a crash or debugging problem sometimes cleaning up the mess is the best thing to do if you're out of other ideas. Often that reveals the problem. – tadman Jun 26 '20 at 18:36

1 Answers1

2

I'm not sure you can do this opening the files once and once only. You could try rewinding, but sysfs isn't a "real" filesystem and those aren't real files. If you rewind you might get the same data over and over, especially when using buffered calls like fopen().

The open operation is what prepares that data for reading. Since this is all managed by the kernel it should have very little overhead, and no actual disk activity. Consider that programs like top read thousands of these every second and it's no big deal.

tadman
  • 208,517
  • 23
  • 234
  • 262
  • 1
    Actually `rewind()` does the trick, thank you very much! – Rob Jun 26 '20 at 19:50
  • @Rob That's actually interesting, thanks for testing it out! – tadman Jun 27 '20 at 00:50
  • 1
    I was actually wrong. With some deeper testing turns out that this was yelding the wrong result, so im back at opening and closing each iteration. Thank you anyway! – Rob Jun 27 '20 at 14:01