<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 Fri, Jun 5, 2020 at 6:14 PM Jan Stancek <<a href="mailto:jstancek@redhat.com" target="_blank">jstancek@redhat.com</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">On Wed, Jun 03, 2020 at 08:51:37PM +0800, Li Wang wrote:<br>
>> I don't get why global variables are necessary.<br>
>><br>
><br>
>The only reason to export them as global variables is to make the legacy<br>
>read/write_cpuse_files() happy. So that I said it is a compromise.<br>
><br>
>$ git grep tst_cgroup_new_path<br>
>cpuset/cpuset01.c:      write_cpuset_files(tst_cgroup_new_path, "cpus",<br>
>buf);<br>
>cpuset/cpuset01.c:      write_cpuset_files(tst_cgroup_new_path, "mems",<br>
>mems);<br>
>cpuset/cpuset01.c:      write_cpuset_files(tst_cgroup_new_path, "mems",<br>
>buf);<br>
>cpuset/cpuset01.c:      write_cpuset_files(tst_cgroup_new_path, "mems",<br>
>buf);<br>
>lib/mem.c:      write_cpuset_files(tst_cgroup_new_path, "mems", buf);<br>
>lib/mem.c:              write_cpuset_files(tst_cgroup_new_path, "cpus",<br>
>cpus);<br>
>lib/mem.c:              write_cpuset_files(tst_cgroup_new_path, "cpus",<br>
>"0");<br>
>oom/oom04.c:            write_cpuset_files(tst_cgroup_new_path,<br>
>"memory_migrate", "1");<br>
>oom/oom05.c:            write_cpuset_files(tst_cgroup_new_path,<br>
>"memory_migrate", "1");<br>
<br>
What if we provided access to it via API? Would we still need these<br>
global variables?<br>
<br>
  char *tst_cgroup_get_path(const char *cgroup_mnt)<br>
      // return ptr to tst_cgroup_paths->new_path<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">The series of list operating function are hiding in the library. My thought is</div><div class="gmail_default" style="font-size:small">to make the list transparent to users. </div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">In your method, we have to export the tst_cgroup_get_path() as an external</div><div class="gmail_default" style="font-size:small">function, it stills needs an extra local pointer in testcase to store the got new_path,</div><div class="gmail_default" style="font-size:small">it doesn't seem tidier too.</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
mount path is always known to test, because it passes it to tst_cgroup_mount(),<br>
so it just needs to find out "new path".<br>
<br>
Would that satisfy the need of this legacy test?</blockquote><div><br></div><div><span class="gmail_default" style="font-size:small">How about</span> moving the cpuset legacy code to the library as part of APIs<span class="gmail_default" style="font-size:small">? That'd</span></div><div><span class="gmail_default" style="font-size:small">help to capsulate all the operation of cgroup control in lib, and people just need</span></div><div><span class="gmail_default" style="font-size:small">to invoke the related function as what he/she wants.</span></div><div class="gmail_default" style="font-size:small"><br></div></div><div class="gmail_default" style="font-size:small">+void tst_cgroup_cpuset_read_files(const char *cgroup_dir, const char *filename, char *buf);</div><span class="gmail_default" style="font-size:small">+</span>void tst_cgroup_cpuset_write_files(const char *cgroup_dir, const char *filename, const char *buf);<br clear="all"><div><br></div><div><div class="gmail_default" style="font-size:small">Then 'tst_cgroup_new_path' searching work will all located internally. And 'tst_cgroup_ctl_knob' can</div><div class="gmail_default" style="font-size:small">be local variable too.</div></div><div><br></div><div><div class="gmail_default" style="font-size:small">Any other comments? (attach the v3.1)</div></div><div><br></div>-- <br><div dir="ltr"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>