[PATCH] Do not delete cgroups upon completion of fio job

[PATCH] Do not delete cgroups upon completion of fio job

[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.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 cgroup.c |    1 -
 1 file changed, 1 deletion(-)

Index: fio/cgroup.c
===================================================================
--- fio.orig/cgroup.c	2010-03-31 13:29:49.000000000 -0400
+++ fio/cgroup.c	2010-03-31 13:31:55.788002620 -0400
@@ -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);

Re: [PATCH] Do not delete cgroups upon completion of fio job

[Jens Axboe]

On Wed, Mar 31 2010, Vivek Goyal wrote:
> 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?

-- 
Jens Axboe

Re: [PATCH] Do not delete cgroups upon completion of fio job

[Vivek Goyal]

On Wed, Mar 31, 2010 at 07:26:00PM +0200, Jens Axboe wrote:
> On Wed, Mar 31 2010, Vivek Goyal wrote:
> > 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.

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

> 
> 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

Re: [PATCH] Do not delete cgroups upon completion of fio job

[Jens Axboe]

On Wed, Mar 31 2010, Vivek Goyal wrote:
> On Wed, Mar 31, 2010 at 07:26:00PM +0200, Jens Axboe wrote:
> > On Wed, Mar 31 2010, Vivek Goyal wrote:
> > > 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.
> 
> Sorry, I did not check properly. Upon retest, I see cgroups are not being 
> deleted if these were not created by fio.
> 
> > 
> > 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.

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

-- 
Jens Axboe