| Message ID | bbf87d62e2e8274fddc160813e64aedb0a01ffe1.1637970912.git.luke.nowakowskikrijger@canonical.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Update cgroup_fj and memcg controller tests to work under cgroup2 | expand |
Hi Luke, Thanks for looking into this. On Sat, Nov 27, 2021 at 8:05 AM Luke Nowakowski-Krijger < luke.nowakowskikrijger@canonical.com> wrote: > Update tests to be able to work when memory controller is mounted under > cgroup2 hierarchy. > I'm thinking whether we could achieve these setup functions more generic in cgroup_lib.sh, which to avoid the redundancy behavior over and over again in each cgroup sub-test. About the compatibility of cgroup V1 and V2 in test, I'd suggest following what the LTP C library did in include/tst_cgroup.h. https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L2024 The solution may be briefly as: 1. scan system mounted CGroup path, and judge the situation as one of below: * only CGroup V2 mounted * only CGroup V1 mounted * both CGroup V2 and V1 mounted * no CGroup mounted 2. make use of the system mounted CGroup V2 or TSKIP * goto step 5 3. make use of the system mounted CGroup V1 * goto step 5 4. do mount Cgroup as what we needed (V1 or V2) in test * goto step 5 5. do cleanup What do you think?
Hi Li, On Wed, Dec 1, 2021 at 1:13 AM Li Wang <liwang@redhat.com> wrote: > Hi Luke, > > Thanks for looking into this. > > On Sat, Nov 27, 2021 at 8:05 AM Luke Nowakowski-Krijger < > luke.nowakowskikrijger@canonical.com> wrote: > >> Update tests to be able to work when memory controller is mounted under >> cgroup2 hierarchy. >> > > I'm thinking whether we could achieve these setup functions > more generic in cgroup_lib.sh, which to avoid the redundancy > behavior over and over again in each cgroup sub-test. > > Yes I agree. As I was doing the same things a few times I was beginning to wonder if there was a better way. I would be willing to look further into expanding the cgroup_lib.sh and resubmitting my recent patches with those changes. > About the compatibility of cgroup V1 and V2 in test, I'd suggest > following what the LTP C library did in include/tst_cgroup.h. > > https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L2024 > > The solution may be briefly as: > > 1. scan system mounted CGroup path, and judge the situation as one of > below: > * only CGroup V2 mounted > * only CGroup V1 mounted > * both CGroup V2 and V1 mounted > * no CGroup mounted > 2. make use of the system mounted CGroup V2 or TSKIP > * goto step 5 > 3. make use of the system mounted CGroup V1 > * goto step 5 > 4. do mount Cgroup as what we needed (V1 or V2) in test > * goto step 5 > 5. do cleanup > > Yes this would be the way to go through setting up a controller and checking everything. Going through the steps you mentioned for mounting a single controller and returning that path wouldn't be too hard but it seems to get more complicated when we want some guarantee of having multiple controllers in a hierarchy (if we even would want to support something like that, which for testing purposes wouldnt seem unheard of). Maybe something mimicking the tst_cgroup_require() from the C api would be useful here? I imagine this is where we would do the checking and mounting logic that you were describing. We would just also have to include checking if the controllers we are requiring can be mounted / already exist together. For example I am imaging something mimicking the C api something like: tst_cgroup_require "cpu" tst_cgroup_require "cpuset" root_mount_point =$(tst_cgroup_get_mountpoint) or just combined them together root_mount_point = $(tst_cgroup_get_mountpoint "cpu cpuset") Again, most of the tests seem to only be testing individual controllers from what I can see so I don't know if this would be too useful. Let me know what you think. > What do you think? > > -- > Regards, > Li Wang > Best, - Luke
On Thu, Dec 2, 2021 at 6:24 AM Luke Nowakowski-Krijger < luke.nowakowskikrijger@canonical.com> wrote: > Hi Li, > > On Wed, Dec 1, 2021 at 1:13 AM Li Wang <liwang@redhat.com> wrote: > >> Hi Luke, >> >> Thanks for looking into this. >> >> On Sat, Nov 27, 2021 at 8:05 AM Luke Nowakowski-Krijger < >> luke.nowakowskikrijger@canonical.com> wrote: >> >>> Update tests to be able to work when memory controller is mounted under >>> cgroup2 hierarchy. >>> >> >> I'm thinking whether we could achieve these setup functions >> more generic in cgroup_lib.sh, which to avoid the redundancy >> behavior over and over again in each cgroup sub-test. >> >> Yes I agree. As I was doing the same things a few times I was beginning > to wonder if there was a better way. I would be willing to look further into > expanding the cgroup_lib.sh and resubmitting my recent patches with those > changes. > Thanks a lot! > > About the compatibility of cgroup V1 and V2 in test, I'd suggest >> following what the LTP C library did in include/tst_cgroup.h. >> >> https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L2024 >> >> The solution may be briefly as: >> >> > 1. scan system mounted CGroup path, and judge the situation as one of >> below: >> * only CGroup V2 mounted >> * only CGroup V1 mounted >> * both CGroup V2 and V1 mounted >> * no CGroup mounted >> 2. make use of the system mounted CGroup V2 or TSKIP >> * goto step 5 >> 3. make use of the system mounted CGroup V1 >> * goto step 5 >> 4. do mount Cgroup as what we needed (V1 or V2) in test >> * goto step 5 >> 5. do cleanup >> >> > Yes this would be the way to go through setting up a controller and > checking everything. > Going through the steps you mentioned for mounting a single controller and > returning that path wouldn't be too hard but > it seems to get more complicated when we want some guarantee of having > multiple controllers in a hierarchy (if we even > would want to support something like that, which for testing purposes > wouldnt seem unheard of). > Right, it is the complicated part and you can take a reference how the current C API handles it. TBH, I even think to skip the handling on mixed mounting with V1 and V2, that is too messy/overthinking and not suggested using way. We'd better face the future and ideally as myself hoping, V2 will replace V1 everywhere someday:). > Maybe something mimicking the tst_cgroup_require() from the C api would be > useful here? I imagine this is where we would > do the checking and mounting logic that you were describing. We would just > also have to include checking if the controllers > we are requiring can be mounted / already exist together. > > For example I am imaging something mimicking the C api something like: > tst_cgroup_require "cpu" > tst_cgroup_require "cpuset" > root_mount_point =$(tst_cgroup_get_mountpoint) > I prefer this one a bit, not only it's consistent with C API but it also can do CGroup mounting in tst_cgroup_require for a system without V1 nor V2 mounting. Then I'd suggest having tst_cgroup_cleanup to help umount that which makes things more clear to understand. Anyway, it depends on the details achieve, maybe there is a better solution we haven't found. > > or just combined them together > > root_mount_point = $(tst_cgroup_get_mountpoint "cpu cpuset") > > Again, most of the tests seem to only be testing individual controllers > from what I can see > so I don't know if this would be too useful. Let me know what you think. > Yes, the existing legacy tests based on one controller should just work well to use the new APIs you're going to achieve. But eventually, I think, those tests also need further refactoring or rewriting someday. And, the most important part is that it (CGroup shell APIs) will be recommended to be used when a new test is written in the future, so now it requires interfaces that have good scalability and compatibility. No matter the test target for CGroup itself or just as an auxiliary part, these shell APIs will play a key ingredient.
Hello Li and Luke Li Wang <liwang@redhat.com> writes: > On Thu, Dec 2, 2021 at 6:24 AM Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> wrote: > > Hi Li, > > On Wed, Dec 1, 2021 at 1:13 AM Li Wang <liwang@redhat.com> wrote: > > Hi Luke, > > Thanks for looking into this. > > On Sat, Nov 27, 2021 at 8:05 AM Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> wrote: > > Update tests to be able to work when memory controller is mounted under > cgroup2 hierarchy. > > I'm thinking whether we could achieve these setup functions > more generic in cgroup_lib.sh, which to avoid the redundancy > behavior over and over again in each cgroup sub-test. +1 this is very necessary IMO > > Yes I agree. As I was doing the same things a few times I was beginning to wonder if there was a better way. I would be willing to look further into > expanding the cgroup_lib.sh and resubmitting my recent patches with > those changes. > > Thanks a lot! > > > About the compatibility of cgroup V1 and V2 in test, I'd suggest > following what the LTP C library did in include/tst_cgroup.h. > https://github.com/linux-test-project/ltp/blob/master/doc/c-test-api.txt#L2024 > > The solution may be briefly as: > > 1. scan system mounted CGroup path, and judge the situation as one of below: > * only CGroup V2 mounted > * only CGroup V1 mounted > * both CGroup V2 and V1 mounted > * no CGroup mounted > 2. make use of the system mounted CGroup V2 or TSKIP > * goto step 5 > 3. make use of the system mounted CGroup V1 > * goto step 5 > 4. do mount Cgroup as what we needed (V1 or V2) in test > * goto step 5 > 5. do cleanup > > Yes this would be the way to go through setting up a controller and checking everything. > Going through the steps you mentioned for mounting a single controller and returning that path wouldn't be too hard but > it seems to get more complicated when we want some guarantee of having multiple controllers in a hierarchy (if we even > would want to support something like that, which for testing purposes wouldnt seem unheard of). > > Right, it is the complicated part and you can take a reference how > the current C API handles it. Actually we can use the C API. This would avoid a whole bunch of issues. It requires creating a utility which we can use from shell (e.g. tst_cgctl). We *may* have to track state somehow. Either we could run the utility as a daemon initially and communicate with a socket to execute commands. Or we could save/serialise some state to environment/shell variables. Alternatively we can probably rescan the system each time. The only state really required is the test's PID which is needed to find the correct CGroup in the LTP sub-hierarchy. Still that is probably easier than dealing with all of the corner cases twice. > > TBH, I even think to skip the handling on mixed mounting with V1 > and V2, that is too messy/overthinking and not suggested using way. > > We'd better face the future and ideally as myself hoping, > V2 will replace V1 everywhere someday:). There are still controllers/features that don't exist on V2. Meanwhile there are features that only exist on V2. So if someone wants both, then they have to mount both. Regardless, this was the default in systemd, so we are stuck with it for about 20 years and can't ignore it ;-) > > > Maybe something mimicking the tst_cgroup_require() from the C api would be useful here? I imagine this is where we would > do the checking and mounting logic that you were describing. We would just also have to include checking if the controllers > we are requiring can be mounted / already exist together. > > For example I am imaging something mimicking the C api something like: > tst_cgroup_require "cpu" > tst_cgroup_require "cpuset" > root_mount_point =$(tst_cgroup_get_mountpoint) > > I prefer this one a bit, not only it's consistent with C API but it also > can do CGroup mounting in tst_cgroup_require for a system without > V1 nor V2 mounting. Then I'd suggest having tst_cgroup_cleanup to > help umount that which makes things more clear to understand. > > Anyway, it depends on the details achieve, maybe there is a better > solution we haven't found. Yes, or if it is a utility then $ test_cpu_cg_dir = $(tst_cgroup require cpu) $ test_cpu_cg_dir = $(tst_cgroup require memory) maybe with an option to pass the PID to indetify the test. I guess there might be some existing env variable the shell API sets we could use as well. $ tst_cgroup cleanup --pid $MAIN_PID
Hi Richard and Li, On Thu, Dec 2, 2021 at 1:50 AM Richard Palethorpe <rpalethorpe@suse.de> wrote: > Hello Li and Luke > Li Wang <liwang@redhat.com> writes: > > > On Thu, Dec 2, 2021 at 6:24 AM Luke Nowakowski-Krijger < > luke.nowakowskikrijger@canonical.com> wrote: > > > > Hi Li, > > > > On Wed, Dec 1, 2021 at 1:13 AM Li Wang <liwang@redhat.com> wrote: > > > > Hi Luke, > > > > Thanks for looking into this. > > > > On Sat, Nov 27, 2021 at 8:05 AM Luke Nowakowski-Krijger < > luke.nowakowskikrijger@canonical.com> wrote: > > > > Update tests to be able to work when memory controller is mounted under > > cgroup2 hierarchy. > > > > I'm thinking whether we could achieve these setup functions > > more generic in cgroup_lib.sh, which to avoid the redundancy > > behavior over and over again in each cgroup sub-test. > > +1 this is very necessary IMO > > Actually we can use the C API. This would avoid a whole bunch of > issues. It requires creating a utility which we can use from shell > (e.g. tst_cgctl). > > We *may* have to track state somehow. Either we could run the utility as > a daemon initially and communicate with a socket to execute commands. Or > we could save/serialise some state to environment/shell > variables. Alternatively we can probably rescan the system each > time. The only state really required is the test's PID which is needed > to find the correct CGroup in the LTP sub-hierarchy. > > Still that is probably easier than dealing with all of the corner cases > twice. > > I rather like this idea. If I understand you correctly we could use it in an RPC sort of way which would make a lot of things simpler and use the existing C API which is nice. My one question would be if we would want this daemon to run as a test suite utility, like it seems you are suggesting, or as a per process utility. The nice part of having a daemon that we could fork off for every test that uses it would be that the cleanup / tracking of sub-groups would get cleaned up in the normal way when we want to close the daemon and just call tst_cgroup_cleanup(). The daemons state would be tied to the test that's issuing commands to it. We could also send out the commands via a shared buffer or pipe that we read and write to. But is a daemon per test (that uses the cgroup shell api) overkill? It seems it would spare us from having to track the test PID to sub-hierarchy like you were mentioning. Or maybe there are some other drawbacks to the per-test daemon idea that I'm not seeing? Looking forward to hearing what you think. > > > > TBH, I even think to skip the handling on mixed mounting with V1 > > and V2, that is too messy/overthinking and not suggested using way. > > > > We'd better face the future and ideally as myself hoping, > > V2 will replace V1 everywhere someday:). > > There are still controllers/features that don't exist on V2. Meanwhile > there are features that only exist on V2. So if someone wants both, then > they have to mount both. Regardless, this was the default in systemd, so > we are stuck with it for about 20 years and can't ignore it ;-) > > > > > > > Maybe something mimicking the tst_cgroup_require() from the C api would > be useful here? I imagine this is where we would > > do the checking and mounting logic that you were describing. We would > just also have to include checking if the controllers > > we are requiring can be mounted / already exist together. > > > > For example I am imaging something mimicking the C api something like: > > tst_cgroup_require "cpu" > > tst_cgroup_require "cpuset" > > root_mount_point =$(tst_cgroup_get_mountpoint) > > > > I prefer this one a bit, not only it's consistent with C API but it also > > can do CGroup mounting in tst_cgroup_require for a system without > > V1 nor V2 mounting. Then I'd suggest having tst_cgroup_cleanup to > > help umount that which makes things more clear to understand. > > > > Anyway, it depends on the details achieve, maybe there is a better > > solution we haven't found. > > Yes, or if it is a utility then > > $ test_cpu_cg_dir = $(tst_cgroup require cpu) > $ test_cpu_cg_dir = $(tst_cgroup require memory) > > maybe with an option to pass the PID to indetify the test. I guess there > might be some existing env variable the shell API sets we could use as > well. > > $ tst_cgroup cleanup --pid $MAIN_PID > > -- > Thank you, > Richard. > Thanks, - Luke
Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> wrote: > > Richard Palethorpe <rpalethorpe@suse.de> wrote: > >> >> Actually we can use the C API. This would avoid a whole bunch of >> issues. It requires creating a utility which we can use from shell >> (e.g. tst_cgctl). >> > +1 This is a good idea. > >> We *may* have to track state somehow. Either we could run the utility as >> a daemon initially and communicate with a socket to execute commands. Or >> we could save/serialise some state to environment/shell >> variables. Alternatively we can probably rescan the system each >> time. The only state really required is the test's PID which is needed >> to find the correct CGroup in the LTP sub-hierarchy. >> >> Still that is probably easier than dealing with all of the corner cases >> twice. >> >> I rather like this idea. If I understand you correctly we could use it in > an RPC sort of way which would make a lot of things simpler and use the > existing C API which is nice. > > My one question would be if we would want this daemon to run as a test > suite utility, like it seems you are suggesting, or as a per process > utility. > Hmm, why do we need that utility as a daemon in the background? Isn't it easier to execute a binary utility to get the CGroup path only when needed? Just like Richard mentioned the alternative way, rescan system each time and only distinguish correct CGroup path via the test PID. > > > The nice part of having a daemon that we could fork off for every test > that uses it would be that the cleanup / tracking of sub-groups would get > cleaned up in the normal way when we want to close the daemon and just call > tst_cgroup_cleanup(). The daemons state would be tied to the test that's > issuing commands to it. We could also send out the commands via a shared > buffer or pipe that we read and write to. > > But is a daemon per test (that uses the cgroup shell api) overkill? It > seems it would spare us from having to track the test PID to sub-hierarchy > like you were mentioning. Or maybe there are some other drawbacks to the > per-test daemon idea that I'm not seeing? > I think yes, starting a daemon per test is not wise. Another drawback I can think of is that will definitely affect paralleling things, we must guarantee the CGroup mounted by testA shouldn't be scanned/used by testB, otherwise, it will fail in the cleanup phase. But, we can make the LTP test mounted CGroup path is transparent to others just by adding a special string like "ltp-cgroup".
Hi Richard and Li, Hmm, why do we need that utility as a daemon in the background? > Isn't it easier to execute a binary utility to get the CGroup path > only when needed? Just like Richard mentioned the alternative > way, rescan system each time and only distinguish correct CGroup > path via the test PID. > > Yes this would be easier if we only wanted to get access to the path. I was getting ahead of myself and thinking about using the C-api to keep track of creating sub-cgroups and all the state that comes with that. Which we wouldn't want to do for a shell utility because creation/cleanup of subgroups is for the test to take care of and that is more trivial in a shell environment. > > >> >> >> The nice part of having a daemon that we could fork off for every test >> that uses it would be that the cleanup / tracking of sub-groups would get >> cleaned up in the normal way when we want to close the daemon and just call >> tst_cgroup_cleanup(). The daemons state would be tied to the test that's >> issuing commands to it. We could also send out the commands via a shared >> buffer or pipe that we read and write to. >> >> But is a daemon per test (that uses the cgroup shell api) overkill? It >> seems it would spare us from having to track the test PID to sub-hierarchy >> like you were mentioning. Or maybe there are some other drawbacks to the >> per-test daemon idea that I'm not seeing? >> > > I think yes, starting a daemon per test is not wise. > > Another drawback I can think of is that will definitely affect paralleling > things, > we must guarantee the CGroup mounted by testA shouldn't be scanned/used > by testB, otherwise, it will fail in the cleanup phase. But, we can make > the LTP > test mounted CGroup path is transparent to others just by adding a special > string > like "ltp-cgroup". > > If I understand it correctly, all the cgroup tests are mounted and created like "controller_mount_point/ltp/test-$PID" where every test shares the mount_point and ltp dir. And when tst_cgroup_cleanup() gets called it only cleans up its own test_dir and removes the ltp dir and unmounts the mount point if it was the first test to do it. So none of the tests should be touching each other's directories and so what you're describing should already be taken care of. Maybe I'm not understanding you correctly. I think the only problem with the binary utility approach where we rescan every time we execute it is that 1) The test-$PID dir that the test would be created with the PID of the program which if we were executing the utility could be different between calls. This could be easily solved by adding an arg to tst_cgroup_opts for a specific PID, which would be the test that is calling it. 2) We lose the reference between calls to the root->test_dir that is filled in when we call tst_cgroup_require(), which is what does the cleanup of the test specific dir. This is where I believe Richard was mentioning passing the PID as "tst_cgroup cleanup --pid $MAIN_PID". Which if we wanted to use the C api for this we would have to expose it to knowing about specific PIDs? Or for the root->test_dir to be reset somewhere? > -- > Regards, > Li Wang > Let me know if this makes sense and what you think about it, I might be getting confused somewhere. But if I understand you correctly I believe that the binary utility approach where we rescan and call tst_cgroup_require() or tst_cgroup_cleanup() is a good approach. Best, - Luke
Hello Luke and Li, Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> writes: > Hi Richard and Li, > > Hmm, why do we need that utility as a daemon in the background? > Isn't it easier to execute a binary utility to get the CGroup path > only when needed? Just like Richard mentioned the alternative > way, rescan system each time and only distinguish correct CGroup > path via the test PID. > > Yes this would be easier if we only wanted to get access to the path. I was getting ahead of myself and thinking about using the C-api to keep track of creating sub-cgroups and all the state that comes with that. Which we wouldn't want to do for a shell utility because creation/cleanup of subgroups is for the test to take care of and > that is more trivial in a shell environment. Yes; tests which require complicated hierarchies have to cleanup themselves. I think it would be a waste of time to add a generic sub-directory cleanup mechanism until we have a number of tests which would benefit from it. Especially because some controllers might place special restrictions on directory and process manipulation. (cpuset for example?). > > > > The nice part of having a daemon that we could fork off for every test that uses it would be that the cleanup / tracking of sub-groups would get cleaned up in the normal way when we want to close the daemon and just call tst_cgroup_cleanup(). The daemons state would be tied to the test that's issuing commands to it. > We could also send out the commands via a shared buffer or pipe that > we read and write to. > > But is a daemon per test (that uses the cgroup shell api) overkill? It seems it would spare us from having to track the test PID to sub-hierarchy like you were mentioning. Or maybe there are some other drawbacks to the per-test daemon idea that I'm not seeing? > > I think yes, starting a daemon per test is not wise. > > Another drawback I can think of is that will definitely affect paralleling things, > we must guarantee the CGroup mounted by testA shouldn't be scanned/used > by testB, otherwise, it will fail in the cleanup phase. But, we can make the LTP > test mounted CGroup path is transparent to others just by adding a special string > like "ltp-cgroup". > > If I understand it correctly, all the cgroup tests are mounted and created like "controller_mount_point/ltp/test-$PID" where every test shares the mount_point and ltp dir. And when tst_cgroup_cleanup() gets called it only cleans up its own test_dir and removes the ltp dir and unmounts the mount point if it was the first test to do it. So > none of the tests should be touching each other's directories and so > what you're describing should already be taken care of. Maybe I'm not > understanding you correctly. I think maybe you are talking about two different things. However IMO daemons should be avoided in general. There is no hard requirement here to have a long running process. > > I think the only problem with the binary utility approach where we rescan every time we execute it is that > 1) The test-$PID dir that the test would be created with the PID of the program which if we were executing the utility could be different between calls. This could be easily solved by adding an arg to tst_cgroup_opts for a specific PID, which would be the test that is calling it. > 2) We lose the reference between calls to the root->test_dir that is filled in when we call tst_cgroup_require(), which is what does the cleanup of the test specific dir. This is where I believe Richard was mentioning passing the PID as "tst_cgroup cleanup --pid $MAIN_PID". Which if we wanted to use the C api for this we would have to > expose it to knowing about specific PIDs? Or for the root->test_dir to > be reset somewhere? I suppose we also need to remember if the current test created the ltp subdirectory or mounted any controllers. We could do this by just printing the required command line options to stdout when the utility exits. Then save this to a variable for the next use. > > -- > Regards, > Li Wang > > Let me know if this makes sense and what you think about it, I might be getting confused somewhere. But if I understand you correctly I believe that the binary utility approach where we rescan and call tst_cgroup_require() or tst_cgroup_cleanup() is a good approach. > > Best, > - Luke Yes, sounds good.
Hi Richard and Li, On Mon, Dec 6, 2021 at 1:24 AM Richard Palethorpe <rpalethorpe@suse.de> wrote: > I suppose we also need to remember if the current test created the ltp > subdirectory or mounted any controllers. > > We could do this by just printing the required command line options to > stdout when the utility exits. Then save this to a variable for the next > use. > > Yes, we would have to remember what we mounted. I think the part I am most curious about is how we would generate that state i.e what we mounted, because the Cgroup library does not expose any of this as far as I'm aware. If we want to use the tst_cgroup C lib to cleanup as well we would have to find a way to reintroduce test state to the lib that we are losing between calls of the utility, which the only way I could think of is introducing a way to export and import test state within the lib. e.g. tst_cgroup_print_test_state() tst_cgroup_load_test_state(), which doesn't feel good as it exposes some of the nice API you have going on. This is the easiest way to tell if we are mounting things because we can just print what we mounted, what the test dir of the test is, and reload that state. This could have further applications to not just this scenario but also to scenarios where if a test dies its state can be reloaded, etc, almost in a checkpoint way. Not saying its common but adds some flexibility to the API and I could see it having applications outside of this utility. Alternatively we could inspect what we created and generate state that way, i.e. make a call to tst_cgroup_require() and see if new things were mounted. Then we would have to manually be freeing things. I don't like this approach because it goes against the whole point of this which was code reuse. But the cleanup of things isnt the most difficult part so it wouldn't be the biggest deal to redo the logic. Yes, sounds good. > > Let me know what you think. I wouldn't want to add anything huge to the API without your blessing :) > -- > Thank you, > Richard. > Thanks, - Luke
Hi! > Yes, we would have to remember what we mounted. I think the part I am most > curious about is how we would generate that state i.e what we mounted, > because the Cgroup library does not expose any of this as far as I'm aware. > > If we want to use the tst_cgroup C lib to cleanup as well we would have to > find a way to reintroduce test state to the lib that we are losing between > calls of the utility, which the only way I could think of is introducing a > way to export and import test state within the lib. e.g. > tst_cgroup_print_test_state() tst_cgroup_load_test_state(), which doesn't > feel good as it exposes some of the nice API you have going on. This is the > easiest way to tell if we are mounting things because we can just print > what we mounted, what the test dir of the test is, and reload that state. > This could have further applications to not just this scenario but also to > scenarios where if a test dies its state can be reloaded, etc, almost in a > checkpoint way. Not saying its common but adds some flexibility to the API > and I could see it having applications outside of this utility. > > Alternatively we could inspect what we created and generate state that way, > i.e. make a call to tst_cgroup_require() and see if new things were > mounted. Then we would have to manually be freeing things. I don't like > this approach because it goes against the whole point of this which was > code reuse. But the cleanup of things isnt the most difficult part so it > wouldn't be the biggest deal to redo the logic. > > Yes, sounds good. > > > > > Let me know what you think. I wouldn't want to add anything huge to the API > without your blessing :) Wouldn't it be easier to rewrite these test to the C then? I think that error handling in shell CGroup tests would always be more difficuilt than in C and given that we have a nice library for C it actually sounds like a better solution.
Hi! > > Let me know what you think. I wouldn't want to add anything huge to the API > > without your blessing :) > > Wouldn't it be easier to rewrite these test to the C then? I think that > error handling in shell CGroup tests would always be more difficuilt > than in C and given that we have a nice library for C it actually sounds > like a better solution. Also not to mention other problems, in shell you actually need at least one fork to do anything which isn't ideal when you are stress testing memory, which means that the fork() may fail. While in C you just call a syscall which is less likely to fail or be stalled due to the load...
Hello, Cyril Hrubis <chrubis@suse.cz> writes: > Hi! >> Yes, we would have to remember what we mounted. I think the part I am most >> curious about is how we would generate that state i.e what we mounted, >> because the Cgroup library does not expose any of this as far as I'm aware. >> >> If we want to use the tst_cgroup C lib to cleanup as well we would have to >> find a way to reintroduce test state to the lib that we are losing between >> calls of the utility, which the only way I could think of is introducing a >> way to export and import test state within the lib. e.g. >> tst_cgroup_print_test_state() tst_cgroup_load_test_state(), which doesn't >> feel good as it exposes some of the nice API you have going on. This >> is the struct tst_cgroup_opts and tst_cgroup_print_config could be extended. These are presently only used for debugging IIRC. BTW don't worry about the purity of API too much or how it's implemented to begin with. The current API is nice, but that is only after a lengthy review process. >> easiest way to tell if we are mounting things because we can just print >> what we mounted, what the test dir of the test is, and reload that state. >> This could have further applications to not just this scenario but also to >> scenarios where if a test dies its state can be reloaded, etc, almost in a >> checkpoint way. Not saying its common but adds some flexibility to the API >> and I could see it having applications outside of this utility. Perhaps, but if a test dies then we usually assume a kernel bug happened and abort. In the future we want tests to be able to restart the system, but we don't have concrete information about such scenarios. >> >> Alternatively we could inspect what we created and generate state that way, >> i.e. make a call to tst_cgroup_require() and see if new things were >> mounted. Then we would have to manually be freeing things. I don't like >> this approach because it goes against the whole point of this which was >> code reuse. But the cleanup of things isnt the most difficult part so it >> wouldn't be the biggest deal to redo the logic. We have to save the PID (or some kind of test ID) that is used to identify the CGroup. So may as well save what CGroups we created as well. >> >> Yes, sounds good. >> > >> > >> Let me know what you think. I wouldn't want to add anything huge to the API >> without your blessing :) > > Wouldn't it be easier to rewrite these test to the C then? I think that > error handling in shell CGroup tests would always be more difficuilt > than in C and given that we have a nice library for C it actually sounds > like a better solution. Yeah, I would prefer C in general. I'm not sure what is the path of least resistance though.
diff --git a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh index c43d72116..ba7c8f386 100755 --- a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh +++ b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh @@ -21,12 +21,62 @@ TST_TIMEOUT=2100 . cgroup_lib.sh +setup_cgroupv2() +{ + mount_point=$(grep -w cgroup2 /proc/mounts | cut -f 2 | cut -d " " -f2) + if ! grep -q memory "$mount_point"/cgroup.controllers; then + tst_res TINFO "memory controller not supported on cgroup v2." + return + fi + + task_list="cgroup.procs" + cgroup_v="v2" +} + +setup_cgroupv1() +{ + exist=$(grep -w memory /proc/cgroups | cut -f1); + if [ "$exist" = "" ]; then + tst_brk TCONF NULL "memory controller not supported" + fi + + mount_point=$(grep -w memory /proc/mounts | cut -f 2 | cut -d " " -f2) + if [ "$mount_point" = "" ]; then + cgroup_mounted=0 + mount_point="/dev/memcg" + fi + + if [ "$cgroup_mounted" -eq "0" ]; then + ROD mkdir -p $mount_point + ROD mount -t cgroup -o memory none $mount_point + fi + + task_list="tasks" + cgroup_v="v1" +} + setup() { if ! is_cgroup_subsystem_available_and_enabled "memory"; then tst_brk TCONF "Either kernel does not support Memory Resource Controller or feature not enabled" fi + if tst_kvcmp -lt "2.6.30"; then + tst_brk TBROK "Test should be run with kernel 2.6.30 or newer" + fi + + cgroup_mounted=1 + + if grep -q cgroup2 /proc/mounts; then + setup_cgroupv2 + fi + + if [ -z "$cgroup_v" ]; then + setup_cgroupv1 + fi + + tst_res TINFO "test starts with cgroup $cgroup_v" + echo 3 > /proc/sys/vm/drop_caches sleep 2 local mem_free=`cat /proc/meminfo | grep MemFree | awk '{ print $2 }'` @@ -43,20 +93,12 @@ setup() cleanup() { - if [ -e /dev/memcg ]; then - EXPECT_PASS umount /dev/memcg - EXPECT_PASS rmdir /dev/memcg + if [ $cgroup_mounted -eq "0" ]; then + EXPECT_PASS umount $mount_point + EXPECT_PASS rmdir $mount_point fi } -do_mount() -{ - cleanup - - EXPECT_PASS mkdir /dev/memcg - EXPECT_PASS mount -t cgroup -omemory memcg /dev/memcg -} - # $1 Number of cgroups # $2 Allocated MB memory in one process # $3 The interval to touch memory in a process @@ -71,13 +113,11 @@ run_stress() tst_res TINFO "Testing $cgroups cgroups, using $mem_size MB, interval $interval" - do_mount - tst_res TINFO "Starting cgroups" for i in $(seq 0 $(($cgroups-1))); do - mkdir /dev/memcg/$i 2> /dev/null + mkdir "$mount_point/$i" 2> /dev/null memcg_process_stress $mem_size $interval & - echo $! > /dev/memcg/$i/tasks + echo $! > "$mount_point/$i/$task_list" pids="$pids $!" done @@ -93,12 +133,11 @@ run_stress() for pid in $pids; do kill -KILL $pid 2> /dev/null wait $pid 2> /dev/null - rmdir /dev/memcg/$i 2> /dev/null + rmdir "$mount_point/$i" 2> /dev/null i=$((i+1)) done tst_res TPASS "Test passed" - cleanup } test1()
Update tests to be able to work when memory controller is mounted under cgroup2 hierarchy. Remove redundant mounts so that it mounts once for both tests to keep the logic a bit simpler. Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> --- .../memcg/stress/memcg_stress_test.sh | 73 ++++++++++++++----- 1 file changed, 56 insertions(+), 17 deletions(-)