Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lmsensors: Add lib/ to explicit search path to .so loader #82

Merged

Conversation

anustuvicl
Copy link
Contributor

@anustuvicl anustuvicl commented Sep 7, 2023

This behaviour observed on ICL Leconte

$ module load lm-sensors
$ export PAPI_LMSENSORS_ROOT=$ICL_LM_SENSORS_ROOT
$ ./configure --with-components="lmsensors" && make && make install
$ utils/papi_component_avail

Shows the output

...
Name:   lmsensors               Linux LMsensor statistics
   \-> Disabled: libsensors.so not found.

The problem happens because the component searches for libsensors.so in $PAPI_LMSENSORS_ROOT/lib64 while the so exists in $PAPI_LMSENSORS_ROOT/lib

This PR fixes this problem by adding lib to the search path for the so file.

After this fix the output of papi_component_avail shows

...
Name:   lmsensors               lm-sensors provides tools for monitoring the hardware health
...
Name:   lmsensors               lm-sensors provides tools for monitoring the hardware health
                                Native: 170, Preset: 0, Counters: 170

@anustuvicl anustuvicl requested a review from gcongiu September 7, 2023 13:51
@gcongiu
Copy link
Contributor

gcongiu commented Sep 7, 2023

This PR is missing any description and context. Please provide more information.

@anustuvicl anustuvicl linked an issue Sep 7, 2023 that may be closed by this pull request
@@ -399,6 +399,12 @@ link_lmsensors_libraries ()
dl1 = dlopen(path_name, RTLD_NOW | RTLD_GLOBAL); // Try to open that path.
}

// Step 4: Try another explicit install default.
if (dl1 == NULL && lmsensors_root != NULL) { // if root given, try it.
snprintf(path_name, 1024, "%s/lib/libsensors.so", lmsensors_root); // PAPI Root check.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use PATH_MAX instead of a raw value for string length

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall I update the num literal in the previous statements as well (line 398)?

Copy link
Contributor

@gcongiu gcongiu Sep 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can update it in a separate commit or even a new PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated as requested with PATH_MAX, and added commit to update the same for previous statement.

@anustuvicl anustuvicl force-pushed the 2023.09.05-lmsensors_fix_lib_so_load branch from 81f335c to 5756dea Compare September 15, 2023 15:03
@anustuvicl anustuvicl force-pushed the 2023.09.05-lmsensors_fix_lib_so_load branch from 5756dea to 03c4bef Compare September 19, 2023 01:12
@anustuvicl anustuvicl merged commit c6f43ec into icl-utk-edu:master Sep 19, 2023
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PAPI lmsensors: libsensors.so fails to load
2 participants