Wakeup interface concerns (Was Re: changing the enum type)
hughsient at gmail.com
Tue Mar 3 01:32:08 PST 2009
On Mon, 2009-03-02 at 11:35 -0500, David Zeuthen wrote:
> The main concern is that the presence of the signals leads you to
> believe you can just connect to connect to the bus and then they get
> delivered to you with some sane frequency.
Well, you can. The kernel only updates this data every n seconds, where
n is an floating point number of seconds defined internally (in the
kernel) which can change. We don't do any clever processing of events /
timeout period, that's all done in the kernel.
> Of course this won't work because then the daemon would have to wake up
> all the time. Which coincidentally it used to do until I pointed that
We only wake up every few seconds to see if the kernel has written out
new data which is for the n seconds earlier. We certainly don't wake up
for every event!
> There's also a (very slight) concern the data carried in the signals may
> be privileged information; you may not be authorized to get this
> information depending on how your system is set up.
Sure, we can put this behind a PolicyKit action if we need to.
> The other observation to make, once you realize that signals won't work,
> is that having multiple different callers will cause measurement issues.
> They literally step on each others toes with the way this work.
No, sorry. This is exactly why DK-p is reading the kernel data, it's
difficult to get right and has to be done as root. Just from reading out
a table of data in /proc doesn't invalidate it, so there's no
stepping-on-toes problem. The kernel does the averaging over a time
period, not us.
> Another observation is that the wakeup interface is mostly just a power
> user feature, it's really only something you need if you want a GUI-ish
> powertop utility like you did in g-p-m.
Right. The QoS is probably also a power user thing, but I want to tie
that into other frameworks so it starts being used by default.
> o get rid of the signals
> o make GetData() also return
> - the data in the signals
> - amount of msec since this was last read
> in addition to the array of structs per process
There's no point returning the time since last read, as the kernel data
won't change until the next kernel-space refresh.
> o add methods to claim/release the wake up interface
> - StartReadingData (OUT string cookie)
> - StopReadingData (IN string cookie)
> and only allow a single caller to do this. Remember to
> watch for disconnects too.
I don't think it's a problem if multiple callers call this (given what I
said above), and at the moment we're just stopping the polling if
nothing reads the data for a 30 seconds.
In the end of the day, the API is tailored to what the kernel can give
us. A disk access interface might look more like what you suggested, but
that's because it's different data from the kernel. I guess the best
thing would be to do the averaging in the kernel as what we have now
with the vm dump isn't the most palatable sort of data.
More information about the devkit-devel