Do not delete cgroups upon completion of fio job

PATCH In review Vivek Goyal 2 participants 4 messages Mar 31, 2010

Share & tools

Vivek Goyal

author

2010-03-31 17:19 UTC #0 lore raw

PATCH 1/1 Do not delete cgroups upon completion of fio job

Currently upon fio job completion, we delete the cgroups. But in the process we also loose important debugging information presetn in cgroup files. For example time information in blkio.time, sector information in blkio.sectors and how many a times a group was dequeued blkio.dequeue. Leaving the cgroups as it is, allows one to go and inspect those files after the fio job is complete.

Secondly, cgroups might have already been created by user before fio job started. In that case it is not very right to delete cgroups after job completion.

Signed-off-by Vivek Goyal <[email protected]>

---
 cgroup.c |    1 -
 1 file changed, 1 deletion(-)

cgroup.c +0 -1

Index: fio/cgroup.c
===================================================================
@@ -65,7 +65,6 @@ void cgroup_kill(struct flist_head *clis
 
 	flist_for_each_safe(n, tmp, clist) {
 		cm = flist_entry(n, struct cgroup_member, list);
-		rmdir(cm->root);
 		flist_del(&cm->list);
 		sfree(cm->root);
 		sfree(cm);

Jens Axboe

2010-03-31 17:26 UTC #1 lore raw

Quoted 10 lines · Vivek Goyal

Currently upon fio job completion, we delete the cgroups. But in the process we also loose important debugging information presetn in cgroup files. For example time information in blkio.time, sector information in blkio.sectors and how many a times a group was dequeued blkio.dequeue. Leaving the cgroups as it is, allows one to go and inspect those files after the fio job is complete.
Secondly, cgroups might have already been created by user before fio job started. In that case it is not very right to delete cgroups after job completion.

Confused, I was pretty sure that I added code back then to only delete cgroups when they were created by fio. Checking, that is what it should do - we only add the cgroup to the kill list, if we were the one to create it. That was on purpose, since I did not want to leave them around.

I'd suggest adding a specific option to NOT kill a cgroup. Or you could have a post_exec option that tar's up the contents of that directory, perhaps even formalized?

Signature · 1 line

Jens Axboe

Vivek Goyal

author

2010-03-31 17:36 UTC #2 lore raw

Quoted 12 lines · Jens Axboe

Quoted 10 lines · Vivek Goyal
Currently upon fio job completion, we delete the cgroups. But in the process we also loose important debugging information presetn in cgroup files. For example time information in blkio.time, sector information in blkio.sectors and how many a times a group was dequeued blkio.dequeue. Leaving the cgroups as it is, allows one to go and inspect those files after the fio job is complete.
Secondly, cgroups might have already been created by user before fio job started. In that case it is not very right to delete cgroups after job completion.

Quoted 5 lines

Confused, I was pretty sure that I added code back then to only delete cgroups when they were created by fio. Checking, that is what it should do - we only add the cgroup to the kill list, if we were the one to create it. That was on purpose, since I did not want to leave them around.

Sorry, I did not check properly. Upon retest, I see cgroups are not being deleted if these were not created by fio.

Quoted 4 lines

I'd suggest adding a specific option to NOT kill a cgroup. Or you could have a post_exec option that tar's up the contents of that directory, perhaps even formalized?

I think adding an option to not kill a cgroup looks good to me as I don't have to do some more scripting to tar and then untar and read cgroup debug files.

I will send a patch for that option.

Thanks
Vivek

Jens Axboe

2010-03-31 17:38 UTC #3 lore raw

Quoted 27 lines · Vivek Goyal

Quoted 12 lines · Jens Axboe
Quoted 10 lines · Vivek Goyal
Currently upon fio job completion, we delete the cgroups. But in the process we also loose important debugging information presetn in cgroup files. For example time information in blkio.time, sector information in blkio.sectors and how many a times a group was dequeued blkio.dequeue. Leaving the cgroups as it is, allows one to go and inspect those files after the fio job is complete.
Secondly, cgroups might have already been created by user before fio job started. In that case it is not very right to delete cgroups after job completion.
Quoted 5 lines
Confused, I was pretty sure that I added code back then to only delete cgroups when they were created by fio. Checking, that is what it should do - we only add the cgroup to the kill list, if we were the one to create it. That was on purpose, since I did not want to leave them around.
Sorry, I did not check properly. Upon retest, I see cgroups are not being deleted if these were not created by fio.
Quoted 4 lines
I'd suggest adding a specific option to NOT kill a cgroup. Or you could have a post_exec option that tar's up the contents of that directory, perhaps even formalized?

Quoted 5 lines

I think adding an option to not kill a cgroup looks good to me as I don't have to do some more scripting to tar and then untar and read cgroup debug files.
I will send a patch for that option.

Agree, it's probably best. Plus it sort-of matches the unlink option applied to files created. So I'd gladly take that.

Signature · 1 line

Jens Axboe