<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Dec 21, 2021 at 5:17 PM Cyril Hrubis <<a href="mailto:chrubis@suse.cz">chrubis@suse.cz</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi!<br>
> > > v2 --> v3<br>
> > > * rename to tst_disable_oom_protection<br>
> > > * support set PID as 0 to protect current process<br>
> ><br>
> > > +static void set_oom_score_adj(pid_t pid, int value)<br>
> > > +{<br>
> > > + int val;<br>
> > > + char score_path[64];<br>
> > > +<br>
> > > + if (access("/proc/self/oom_score_adj", F_OK) == -1) {<br>
> > We need to check here also /proc/PID/oom_score_adj, i.e. score_path.<br>
> ><br>
> <br>
> Good catch, I would add a 'W_OK' checking and skip the setting with<br>
> a reminder message if run without root.<br>
> <br>
> how about this?<br>
> <br>
> if (access(score_path, W_OK) == -1) {<br>
> tst_res(TINFO, "Warning: %s cannot be accessed for writing,<br>
> please check if test run with root user.",<br>
> score_path);<br>
<br>
Hmm, I guess that we should produce TINFO if the file does not exist and<br>
TWARN if we cannot write to it. Something as:<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Not exactly, if someone gives a wrong PID, that also cannot find</div><div class="gmail_default" style="font-size:small">the score_path. So we shouldn't skip OOM adjustment only</div><div class="gmail_default" style="font-size:small">with printing the TFINO.</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
if (access(score_path, F_OK)) {<br>
tst_res(TINFO,<br>
"'%s' does not exist, skipping OOM score adjustement",<br>
score_path);<br>
return;<br>
}<br>
<br>
if (access(score_path, W_OK)) {<br>
tst_res(TWARN, "'%s' not writeable, are you root?", score_path);<br>
return;<br>
}<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">As Petr points out, only root user can set a negative value to oom_score_adj,</div><div class="gmail_default" style="font-size:small">this W_OK is not enough for ordinary users.</div><br></div><div><br></div><div><div class="gmail_default" style="font-size:small">Consider about situation, I'd suggest go with non-safe macros and add</div><div class="gmail_default" style="font-size:small">additional check in the last.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">e.g.</div><div class="gmail_default" style="font-size:small"><br></div>--- a/lib/tst_memutils.c<br>+++ b/lib/tst_memutils.c<br>@@ -108,17 +108,21 @@ static void set_oom_score_adj(pid_t pid, int value)<br> else<br> sprintf(score_path, "/proc/%d/oom_score_adj", pid);<br> <br>- if (access(score_path, R_OK | W_OK) == -1) {<br>- tst_res(TINFO, "Warning: %s cannot be accessed for reading/writing,<br>- please check if test run with root user.",<br>- score_path);<br>- return<br>- }<br>-<br>- SAFE_FILE_PRINTF(score_path, "%d", value);<br>- SAFE_FILE_SCANF(score_path, "%d", &val);<br>- if (val != value)<br>+ if (access(score_path, F_OK) == -1)<br>+ tst_brk(TBROK, "%s does not exist, please check if PID is valid");<br>+<br>+ FILE_PRINTF(score_path, "%d", value);<br>+ FILE_SCANF(score_path, "%d", &val);<br>+<br>+ if (val != value) {<br>+ if (value < 0) {<br>+ tst_res(TINFO, "Warning: %s cannot be set to negative value,<br>+ please check if test run with root user.",<br>+ score_path);<br>+ return<br>+ }<br> tst_brk(TBROK, "oom_score_adj = %d, but expect %d.", val, value);<br>+ }<br> }<br><div class="gmail_default" style="font-size:small"></div><br></div></div><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>