linux-kernel - Re: [patch v2 4/4] taskstats: Export "cdata

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20101208203938.GB1804@redhat.com>
Date:	Wed, 8 Dec 2010 21:39:38 +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>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	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>, Valdis.Kletnieks@...edu,
	linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org
Subject: Re: [patch v2 4/4] taskstats: Export "cdata_wait" CPU times with
	taskstats

On 12/07, Michael Holzheu wrote:
>
> On Wed, 2010-12-01 at 19:51 +0100, Oleg Nesterov wrote:
> > On 11/29, Michael Holzheu wrote:
> > >
> > > * I left the struct signal locking in the patch, because I think
> > >   that in order to get consistent data this is necessary.
> >
> > OK, I guess you mean that we want to read utime/stime "atomically",
> > and thus we need ->siglock to prevent the race with __exit_signal().
>
> Yes.
>
> But I changed my mind. If I understand the code right, currently no
> locking is done for reading the tasks statistics in both the taskstats
> and also in the procfs interface. So e.g. it is not guaranteed to get a
> consistent set of data when reading /proc/<pid>/stat, because of the
> race with account_user/system/whatever_time() and other accounting
> functions.

Not sure, I think account_user/system/whatever_time() is a bit different.
And, say, do_task_stat() takes ->siglock for thread_group_times(). Btw,
I tried to remove this lock_task_sighand(), and I still hope we can
do this ;) But the patch wasn't acked (iiuc) exactly because it can
make top unhappy..

> Because you said that since 2.6.35 accessing the signal_struct is save,
> I think now also for cdata it is not necessary to lock anything. As a
> result of this and the discussion regarding the group leader and cdata I
> would propose the following patch:

To me, this certainly makes sense. I do not really understand why
it is so important to prevent the case when, say, bacct_add_tsk()
sees the updated .utime and !updated .stime.

If we can race with the exiting tasks, then these numbers are not
"final" anyway.

But again, this all is up to you.

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/