linux-kernel - Re: [PATCH 0/3] de_thread() should update ->real_start

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: <20130611171300.GA7920@redhat.com>
Date:	Tue, 11 Jun 2013 19:13:00 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	John Stultz <john.stultz@...aro.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Tomas Janousek <tjanouse@...hat.com>,
	Tomas Smetana <tsmetana@...hat.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/3] de_thread() should update ->real_start_time

On 06/10, John Stultz wrote:
>
> On Mon, Jun 10, 2013 at 11:33 AM, Oleg Nesterov <oleg@...hat.com> wrote:
> > 1/3 is the obvious bugfix, 2 and 3 are minor cleanups
>
> Acked-by: John Stultz <john.stultz@...aro.org> for these three patches.

Thanks!

> > simply change copy_process
> >
> >         -       do_posix_clock_monotonic_gettime(&p->start_time);
> >         +       get_monotonic_boottime(&p->start_time);
> >
> > ?
> >
> > Afaics, this will only affect do_acct_process() and bacct_add_tsk(),
> > but do we really want to exclude the suspended time in this case?
>
> So bacct_add_tsk seems easy to change, since its just:
>     do_posix_clock_monotonic_gettime(&uptime);
>     ts = timespec_sub(uptime, tsk->start_time);
>
> So grabbing the monotonic boot time for uptime would provide the same
> relative delta.

Not really, or I misunderstood monotonic/boottime interaction.

IIUC, monotonic doesn't grow during suspend, so the delta can grow if
we use get_monotonic_boottime() in copy_process() and bacct_add_tsk()
and the system was suspended in between. Right?

But perhaps this is fine and even more correct?

> > Another user of ->start_time is cgroup.c and it looks wrong... But
> > the change above should not make any difference.
>
> The cgroup usage I'm unfamiliar with. Though from the comments in the
> code, it seems like using boottime would be ok for this.

Yes, this change can't make any difference. But this code looks wrong
although I'll try to recheck later.  We can't trust started_after()
exactly because ->start_time can be changed by mt-exec. But this is
offtopic.


> An additional thought: task_struct covers quite a bit of stuff, so I
> don't know if this would be too useful, but we could further change
> start_time to be a ktime_t, allowing possible additional space savings
> in the task_struct on 64bit systems.

Agreed.

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/