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]
Date:	Sat, 6 Apr 2013 13:05:07 +0200
From:	Richard Cochran <richardcochran@...il.com>
To:	Pawel Moll <pawel.moll@....com>
Cc:	John Stultz <john.stultz@...aro.org>,
	Peter Zijlstra <peterz@...radead.org>,
	David Ahern <dsahern@...il.com>,
	Stephane Eranian <eranian@...gle.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>,
	"mingo@...e.hu" <mingo@...e.hu>, Paul Mackerras <paulus@...ba.org>,
	Anton Blanchard <anton@...ba.org>,
	Will Deacon <Will.Deacon@....com>,
	"ak@...ux.intel.com" <ak@...ux.intel.com>,
	Pekka Enberg <penberg@...il.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Robert Richter <robert.richter@....com>
Subject: Re: [RFC] perf: need to expose sched_clock to correlate user
 samples with kernel samples

On Fri, Apr 05, 2013 at 07:16:53PM +0100, Pawel Moll wrote:
 
> Ok, so how about the code below? Disclaimer: this is just a proposal.
> I'm not sure how welcomed would be an extra field in struct file, but
> this makes the clocks ultimately flexible - one can "attach" the clock
> to any arbitrary struct file. Alternatively we could mark a "clocked"
> file with a special flag in f_mode and have some kind of lookup.

Only a tiny minority of file instances will want to be clocks.
Therefore I think adding the extra field will be a hard sell.

The flag idea sounds harmless, but how do you perform the lookup?
 
> Also, I can't stop thinking that the posix-clock.c shouldn't actually do
> anything about the character device... The PTP core (as the model of
> using character device seems to me just one of possible choices) could
> do this on its own and have simple open/release attaching/detaching the
> clock. This would remove a lot of "generic dev" code in the
> posix-clock.c and all the optional cdev methods in struct posix_clock.
> It's just a thought, though...

Right, the chardev could be pushed into the PHC layer. The original
idea of chardev clocks did have precedents, though, like hpet and rtc.
 
> And a couple of questions to Richard... Isn't the kref_put() in
> posix_clock_unregister() a bug? I'm not 100% but it looks like a simple
> register->unregister sequence was making the ref count == -1, so the
> delete_clock() won't be called.

Well,

	posix_clock_register() -> kref_init() ->
		atomic_set(&kref->refcount, 1);

So refcount is now 1 ...

	posix_clock_unregister() -> kref_put() -> kref_sub(count=1) ->
		atomic_sub_and_test((int) count, &kref->refcount)

and refcount is now 0. Can't see how you would get -1 here.

> And was there any particular reason that the ops in struct
> posix_clock are *not* a pointer?

One less run time indirection maybe? I don't really remember why or
how we arrived at this. The whole PHC review took a year, with
something like fifteen revisions.

Thanks,
Richard

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ