From: Arve Hjønnevåg on
2010/6/9 <david(a)lang.hm>:
> On Wed, 9 Jun 2010, Arve Hj?nnev?g wrote:
>
>>
>> The power may not see the event, the process that reads the event will
>> always see it. If the power manager is not in the poll call when the
>> event happens, the process that reads the event can read the event
>> before the power manager calls poll.
>>
>
>>
>> All input events that can wake the system are handled by one
>> user-space suspend blocker. Input devices come and go so we would need
>> to add and remove the fds dynamically.
>
>
>>
>> For that to work the wakeup events would have to be reported to the
>> power manager in a reliable way in the first place. Passing the file
>> descriptor that the app uses to the power manager does not work for
>> this, since the app could read the event while the power manager was
>> not in the poll call and the power manager would never see it. Also,
>> existing apps don't pass their file descriptors to the power manager,
>> so it has the get the event from somewhere else.
>>
>
> why could the suspend blocker process see all events, but the power manager
> process not see the events?
>

Because in this proposal the power manager only looks for the events
(on the same queue) when no user space suspend blockers are active.

> have the userspace talk to the power manager the way it does to the suspend
> blocker now and what's the difference?
>
> effectivly think s/suspend blocker/power manager/ (with the power manager
> doing all the other things that are proposed instead of grabbing the
> wakelock), the difference should be hidden to the rest of userspace.
>
> what am I missing here?
>

The current user space interface does not require that clients
register the file descriptors that they get wakeup events from with
another process.

--
Arve Hj�nnev�g
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Neil Brown on
On Wed, 9 Jun 2010 21:51:38 -0700
Arve Hjønnevåg <arve(a)android.com> wrote:

> The current user space interface does not require that clients
> register the file descriptors that they get wakeup events from with
> another process.
>

However I believe they *do* register these file descriptors with the kernel,
via some sort of ioctl (I think you have said that is the case for input
devices at least).
Can you confirm that?

If that is the case, is it really such a big change to register with another
process instead of with the kernel?

Thanks,
NeilBrown


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Rafael J. Wysocki on
On Thursday, June 10, 2010, Neil Brown wrote:
> On Wed, 9 Jun 2010 11:40:27 +0200
> "Rafael J. Wysocki" <rjw(a)sisk.pl> wrote:
>
> > On Wednesday 09 June 2010, Felipe Contreras wrote:
> > > On Wed, Jun 9, 2010 at 6:46 AM, Linus Torvalds
> > > <torvalds(a)linux-foundation.org> wrote:
> > > > On Tue, 8 Jun 2010, david(a)lang.hm wrote:
> > > >>
> > > >> having suspend blockers inside the kernel adds significant complexity, it's
> > > >> worth it only if the complexity buys you enough. In this case the question is
> > > >> if the suspend blockers would extend the sleep time enough more to matter. As
> > > >> per my other e-mail, this is an area with rapidly diminishing returns as the
> > > >> sleep times get longer.
> > > >
> > > > Well, the counter-argument that nobody seems to have brought up is that
> > > > suspend blockers exist, are real code, and end up being shipped in a lot
> > > > of machines.
> > > >
> > > > That's a _big_ argument in favour of them. Certainly much bigger than
> > > > arguing against them based on some complexity-arguments for an alternative
> > > > that hasn't seen any testing at all.
> > > >
> > > > IOW, I would seriously hope that this discussion was more about real code
> > > > that _exists_ and does what people need. It seems to have degenerated into
> > > > something else.
> > > >
> > > > Because in the end, "code talks, bullshit walks". People can complain and
> > > > suggest alternatives all they want, but you can't just argue. At some
> > > > point you need to show the code that actually solves the problem.
> > >
> > > That's assuming there is an actual problem, which according to all the
> > > embedded people except android, there is not.
> >
> > Yes, there is, but they've decided to ignore it.
> >
> > > And if there is indeed such a problem (probably not big), it might be
> > > solved properly by the time suspend blockers are merged, or few
> > > releases after.
> >
> > Not quite. Have you followed all of the discussion, actually?
> >
> > > Whatever the solution (or workaround) is, it would be nice if it could
> > > be used by more than just android people, and it would also be nice to
> > > do it without introducing user-space API that *nobody* likes and might
> > > be quickly deprecated.
> >
> > I agree with Linus and I don't have that much of a problem with the API that
> > people seem to have. In fact the much-hated user space API is just a char
> > device driver with 3 ioctls (that can be extended in future if need be) and
> > the kernel API is acceptable to me.
>
> I think there is a little bit more to it than that. It seems there is a new
> ioctl for input/event devices to say "Any events queued here should be
> treated as wake-up events". There may be similar additions to other devices,
> but I know of no details.

That's not in the patchset as in my pull request.

It is used on Android, though, and it would have been submitted separately, had
the first patchset been merged.

> I wonder if we can get a complete statement of changes to the user-space
> API...
>
> > Yes, there is some overlap between it
> > and PM QoS, but IMhO that overlap may be reduced over time (eg. by
> > using PM QoS requirements to implement suspend blockers).
> >
> > To me, the question boils down to whether or not we're able to persuade the
> > Android people to use any other approach (eg. by demonstrating that something
> > else is actually better), because even if we invent a brilliant new approach,
> > but Android ends up using its old one anyway, the net result will be as though
> > we haven't done anything useful.
>
> Yes. There is no point unless we can meet somewhere in the middle. I think
> that would have to include a full suspend that freezes all processes.
> Solutions which reject that - while quite clever - would require too much
> change to Android user-space to be acceptable.

Moreover, having thought a bit more about the "power manager in user space"
concept I'm not sure if it really is that better than the original wakelocks
idea. Namely, it only repaces a kernel-based mechanism with a user space
task doing basically the same thing, but the communication between that task
and the other cooperating user space tasks is arguably more complicated (it
also uses the kernel resources, although indirectly).

So, for a phone-like system, where you'd generally want to simplify user space,
having a "power manager" in the kernel seems to make sense to me.

Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Neil Brown on
On Thu, 10 Jun 2010 10:59:43 +0200
"Rafael J. Wysocki" <rjw(a)sisk.pl> wrote:

> On Thursday, June 10, 2010, Neil Brown wrote:
> > On Wed, 9 Jun 2010 11:40:27 +0200
> > "Rafael J. Wysocki" <rjw(a)sisk.pl> wrote:
> >
> > > On Wednesday 09 June 2010, Felipe Contreras wrote:
> > > > On Wed, Jun 9, 2010 at 6:46 AM, Linus Torvalds
> > > > <torvalds(a)linux-foundation.org> wrote:
> > > > > On Tue, 8 Jun 2010, david(a)lang.hm wrote:
> > > > >>
> > > > >> having suspend blockers inside the kernel adds significant complexity, it's
> > > > >> worth it only if the complexity buys you enough. In this case the question is
> > > > >> if the suspend blockers would extend the sleep time enough more to matter. As
> > > > >> per my other e-mail, this is an area with rapidly diminishing returns as the
> > > > >> sleep times get longer.
> > > > >
> > > > > Well, the counter-argument that nobody seems to have brought up is that
> > > > > suspend blockers exist, are real code, and end up being shipped in a lot
> > > > > of machines.
> > > > >
> > > > > That's a _big_ argument in favour of them. Certainly much bigger than
> > > > > arguing against them based on some complexity-arguments for an alternative
> > > > > that hasn't seen any testing at all.
> > > > >
> > > > > IOW, I would seriously hope that this discussion was more about real code
> > > > > that _exists_ and does what people need. It seems to have degenerated into
> > > > > something else.
> > > > >
> > > > > Because in the end, "code talks, bullshit walks". People can complain and
> > > > > suggest alternatives all they want, but you can't just argue. At some
> > > > > point you need to show the code that actually solves the problem.
> > > >
> > > > That's assuming there is an actual problem, which according to all the
> > > > embedded people except android, there is not.
> > >
> > > Yes, there is, but they've decided to ignore it.
> > >
> > > > And if there is indeed such a problem (probably not big), it might be
> > > > solved properly by the time suspend blockers are merged, or few
> > > > releases after.
> > >
> > > Not quite. Have you followed all of the discussion, actually?
> > >
> > > > Whatever the solution (or workaround) is, it would be nice if it could
> > > > be used by more than just android people, and it would also be nice to
> > > > do it without introducing user-space API that *nobody* likes and might
> > > > be quickly deprecated.
> > >
> > > I agree with Linus and I don't have that much of a problem with the API that
> > > people seem to have. In fact the much-hated user space API is just a char
> > > device driver with 3 ioctls (that can be extended in future if need be) and
> > > the kernel API is acceptable to me.
> >
> > I think there is a little bit more to it than that. It seems there is a new
> > ioctl for input/event devices to say "Any events queued here should be
> > treated as wake-up events". There may be similar additions to other devices,
> > but I know of no details.
>
> That's not in the patchset as in my pull request.
>
> It is used on Android, though, and it would have been submitted separately, had
> the first patchset been merged.

Very true. But as the one cannot be used without the other, they really need
to be considered as a package.


>
> > I wonder if we can get a complete statement of changes to the user-space
> > API...
> >
> > > Yes, there is some overlap between it
> > > and PM QoS, but IMhO that overlap may be reduced over time (eg. by
> > > using PM QoS requirements to implement suspend blockers).
> > >
> > > To me, the question boils down to whether or not we're able to persuade the
> > > Android people to use any other approach (eg. by demonstrating that something
> > > else is actually better), because even if we invent a brilliant new approach,
> > > but Android ends up using its old one anyway, the net result will be as though
> > > we haven't done anything useful.
> >
> > Yes. There is no point unless we can meet somewhere in the middle. I think
> > that would have to include a full suspend that freezes all processes.
> > Solutions which reject that - while quite clever - would require too much
> > change to Android user-space to be acceptable.
>
> Moreover, having thought a bit more about the "power manager in user space"
> concept I'm not sure if it really is that better than the original wakelocks
> idea. Namely, it only repaces a kernel-based mechanism with a user space
> task doing basically the same thing, but the communication between that task
> and the other cooperating user space tasks is arguably more complicated (it
> also uses the kernel resources, although indirectly).
>
> So, for a phone-like system, where you'd generally want to simplify user space,
> having a "power manager" in the kernel seems to make sense to me.
>

Following that logic would we end up putting everything in the kernel?

To my mind the advantage of having something in user-space is flexibility -
you can refine the interfaces and behaviours without bothering the kernel.
The reasons for putting things in the kernel are:
- tight integration with VM or processes (the two main abstractions that the
kernel has to manage)
- privileged access to devices
- arbitration between processes with different privilege levels

Need-for-speed, on the other hand, is not necessarily a justification for
going in the kernel - experience shows that we can make user-space
interactions quite fast enough.


The "power manager" itself is quite trivial. It just needs something like:
while true
wait for all wake-locks to be dropped
activate suspend
and for 'activate suspend' to be interruptible by something taking out a
wake-lock.

To my mind, the most interesting part of this is interruptibility. There is
essentially only one way to do that in Unix/Linux: signals.
So it makes sense to have a task - with a pid - performing this loop so that
it can be sent a signal.
And we already have mechanisms for sending signals on all sorts of different
events, both kernel events (fcntl(F_OWNER)), file events (FNOTIFY), and
normal process operations (kill).

If you are going to have a process (which I think you should) it makes sense
for it to spend some time in user-space to use ipc-of-your-choice to
communicate with other processes, and spend some time in kernel-space to be
able to wait for in-kernel locks to be dropped, and to use signals to get out
of kernel-space, and system calls to get into kernel space.

Yes, the proposed user-space interface for suspend blocks may not be all that
horrible. But the fact is that you can get all the required functionality by
just using currently existing interfaces. Against that background, adding
anything new should be avoided.

NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Pavel Machek on
Hi!

> >> We started here because it's possibly the only api level change we have --
> >> almost everything else is driver or subarch type work or controversial but
> >> entirely self-contained (like the binder, which I would be shocked to see
> >> ever hit mainline). [...]
> >
> > So why arent those bits mainline? It's a 1000 times easier to get drivers and
> > small improvements and non-ABI changes upstream.
> >
> > After basically two years of growing your fork (and some attempts to get your
> > drivers into drivers/staging/ - from where they have meanwhile dropped out
> > again) you re-started with the worst possible thing to merge: a big and
> > difficult kernel feature affecting many subsystems. Why?
>
> Because a large number of our drivers depend on it.

The dependencies are trivial. Last time I checked, you had about
150KLoC drivers, with about 100 lines depending on wakelock support --
I know, I cleaned it up from staging.

The changes required for merging 150KLoC will be definitely much
bigger than 100 lines...

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/