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-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
