linux-kernel - Re: [RFC][PATCH v2 5/7] taskstats: Improve cumulative CPU time accounting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20101113183810.GA9021@redhat.com>
Date:	Sat, 13 Nov 2010 19:38:10 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Michael Holzheu <holzheu@...ux.vnet.ibm.com>
Cc:	Shailabh Nagar <nagar1234@...ibm.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Venkatesh Pallipadi <venki@...gle.com>,
	Suresh Siddha <suresh.b.siddha@...el.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Ingo Molnar <mingo@...e.hu>, John stultz <johnstul@...ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Balbir Singh <balbir@...ux.vnet.ibm.com>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	Roland McGrath <roland@...hat.com>,
	linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org
Subject: Re: [RFC][PATCH v2 5/7] taskstats: Improve cumulative CPU time
	accounting

First of all, let me repeat, I am not going to discuss whether we need
these changes or not, I do not know even if I understand your motivation.

My only concern is correctness, but

On 11/11, Michael Holzheu wrote:
>
> * Add cumulative dead thread CPU times to taskstats
> * Introduce parallel accounting process hierarchy (based on discussions
>   with Oleg Nesterov and Roland McGrath)

Michael, I really think this patch does too many different things.
I forgot the details of our previous discussion, and I got lost
trying to understand the new version.

I already asked you to split these changes, perhaps you can do this?
Say, bacct_add_tsk() looks overcomplicated, the change in copy_process()
shouldn't introduce the new CLONE_THREAD check, not sure I understand
why release_task() was chosen for reparenting, other nits...

But it is not easy to discuss these completely different things
looking at the single patch.

Imho, it would be much better if you make a separate patch which
adds acct_parent/etc and implements the parallel hierarchy. This
also makes sense because it touches the core kernel.

Personally I think that even "struct cdata" + __account_ctime() helper
needs a separate patch, and perhaps this change makes sense by itself
as cleanup. And this way the "trivial" changes (like the changes in
k_getrusage) won't distract from the functional changes.

The final change should introduce cdata_acct and actually implement
the complete cumulative accounting.

At least, please distill parallel accounting process hierarchy.
We do not want bugs in this area ;)

What do you think?

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/