On Sunday, 16 of March 2008, Johannes Berg wrote:
> This revamps the apm-emulation code to get suspend notifications
> regardless of what way pm_suspend() was invoked, whether via the
> apm ioctl or via /sys/power/state. Also do some code cleanup and
> add comments while at it.
>
> Signed-off-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
The patch looks good to me, but it spawns checkpatch.pl warnings that are going
to trigger some nasty comments from akpm. Care to fix them?
Thanks,
Rafael
> ---
> Changelog:
> v1: added notifiers to send out apm events for suspends triggered elsewhere
> v2: rewritten apm-emulation.c to work regardless even when suspend was
> triggered within apm-emulation
> v3: changed suspend state to an enum (suggested by David Brownell)
> v4: make things static, remove superfluous local variable (thanks to sparse)
> v5: no longer export pm_chain_head, rebased on a later tree
>
> drivers/char/apm-emulation.c | 346 +++++++++++++++++++++++++------------------
> 1 file changed, 207 insertions(+), 139 deletions(-)
>
> --- everything.orig/drivers/char/apm-emulation.c 2008-03-15 15:19:41.000000000 +0100
> +++ everything/drivers/char/apm-emulation.c 2008-03-16 11:08:53.000000000 +0100
> @@ -57,6 +57,55 @@ struct apm_queue {
> };
>
> /*
> + * thread states (for threads using a writable /dev/apm_bios fd):
> + *
> + * SUSPEND_NONE: nothing happening
> + * SUSPEND_PENDING: suspend event queued for thread and pending to be read
> + * SUSPEND_READ: suspend event read, pending acknowledgement
> + * SUSPEND_ACKED: acknowledgement received from thread (via ioctl),
> + * waiting for resume
> + * SUSPEND_ACKTO: acknowledgement timeout
> + * SUSPEND_DONE: thread had acked suspend and is now notified of
> + * resume
> + *
> + * SUSPEND_WAIT: this thread invoked suspend and is waiting for resume
> + *
> + * A thread migrates in one of three paths:
> + * NONE -1-> PENDING -2-> READ -3-> ACKED -4-> DONE -5-> NONE
> + * -6-> ACKTO -7-> NONE
> + * NONE -8-> WAIT -9-> NONE
> + *
> + * While in PENDING or READ, the thread is accounted for in the
> + * suspend_acks_pending counter.
> + *
> + * The transitions are invoked as follows:
> + * 1: suspend event is signalled from the core PM code
> + * 2: the suspend event is read from the fd by the userspace thread
> + * 3: userspace thread issues the APM_IOC_SUSPEND ioctl (as ack)
> + * 4: core PM code signals that we have resumed
> + * 5: APM_IOC_SUSPEND ioctl returns
> + *
> + * 6: the notifier invoked from the core PM code timed out waiting
> + * for all relevant threds to enter ACKED state and puts those
> + * that haven't into ACKTO
> + * 7: those threads issue APM_IOC_SUSPEND ioctl too late,
> + * get an error
> + *
> + * 8: userspace thread issues the APM_IOC_SUSPEND ioctl (to suspend),
> + * ioctl code invokes pm_suspend()
> + * 9: pm_suspend() returns indicating resume
> + */
> +enum apm_suspend_state {
> + SUSPEND_NONE,
> + SUSPEND_PENDING,
> + SUSPEND_READ,
> + SUSPEND_ACKED,
> + SUSPEND_ACKTO,
> + SUSPEND_WAIT,
> + SUSPEND_DONE,
> +};
> +
> +/*
> * The per-file APM data
> */
> struct apm_user {
> @@ -67,13 +116,7 @@ struct apm_user {
> unsigned int reader: 1;
>
> int suspend_result;
> - unsigned int suspend_state;
> -#define SUSPEND_NONE 0 /* no suspend pending */
> -#define SUSPEND_PENDING 1 /* suspend pending read */
> -#define SUSPEND_READ 2 /* suspend read, pending ack */
> -#define SUSPEND_ACKED 3 /* suspend acked */
> -#define SUSPEND_WAIT 4 /* waiting for suspend */
> -#define SUSPEND_DONE 5 /* suspend completed */
> + enum apm_suspend_state suspend_state;
>
> struct apm_queue queue;
> };
> @@ -81,7 +124,8 @@ struct apm_user {
> /*
> * Local variables
> */
> -static int suspends_pending;
> +static atomic_t suspend_acks_pending = ATOMIC_INIT(0);
> +static atomic_t userspace_notification_inhibit = ATOMIC_INIT(0);
> static int apm_disabled;
> static struct task_struct *kapmd_tsk;
>
> @@ -164,78 +208,6 @@ static void queue_event(apm_event_t even
> wake_up_interruptible(&apm_waitqueue);
> }
>
> -/*
> - * queue_suspend_event - queue an APM suspend event.
> - *
> - * Check that we're in a state where we can suspend. If not,
> - * return -EBUSY. Otherwise, queue an event to all "writer"
> - * users. If there are no "writer" users, return '1' to
> - * indicate that we can immediately suspend.
> - */
> -static int queue_suspend_event(apm_event_t event, struct apm_user *sender)
> -{
> - struct apm_user *as;
> - int ret = 1;
> -
> - mutex_lock(&state_lock);
> - down_read(&user_list_lock);
> -
> - /*
> - * If a thread is still processing, we can't suspend, so reject
> - * the request.
> - */
> - list_for_each_entry(as, &apm_user_list, list) {
> - if (as != sender && as->reader && as->writer && as->suser &&
> - as->suspend_state != SUSPEND_NONE) {
> - ret = -EBUSY;
> - goto out;
> - }
> - }
> -
> - list_for_each_entry(as, &apm_user_list, list) {
> - if (as != sender && as->reader && as->writer && as->suser) {
> - as->suspend_state = SUSPEND_PENDING;
> - suspends_pending++;
> - queue_add_event(&as->queue, event);
> - ret = 0;
> - }
> - }
> - out:
> - up_read(&user_list_lock);
> - mutex_unlock(&state_lock);
> - wake_up_interruptible(&apm_waitqueue);
> - return ret;
> -}
> -
> -static void apm_suspend(void)
> -{
> - struct apm_user *as;
> - int err = pm_suspend(PM_SUSPEND_MEM);
> -
> - /*
> - * Anyone on the APM queues will think we're still suspended.
> - * Send a message so everyone knows we're now awake again.
> - */
> - queue_event(APM_NORMAL_RESUME);
> -
> - /*
> - * Finally, wake up anyone who is sleeping on the suspend.
> - */
> - mutex_lock(&state_lock);
> - down_read(&user_list_lock);
> - list_for_each_entry(as, &apm_user_list, list) {
> - if (as->suspend_state == SUSPEND_WAIT ||
> - as->suspend_state == SUSPEND_ACKED) {
> - as->suspend_result = err;
> - as->suspend_state = SUSPEND_DONE;
> - }
> - }
> - up_read(&user_list_lock);
> - mutex_unlock(&state_lock);
> -
> - wake_up(&apm_suspend_waitqueue);
> -}
> -
> static ssize_t apm_read(struct file *fp, char __user *buf, size_t count, loff_t *ppos)
> {
> struct apm_user *as = fp->private_data;
> @@ -306,25 +278,22 @@ apm_ioctl(struct inode * inode, struct f
>
> as->suspend_result = -EINTR;
>
> - if (as->suspend_state == SUSPEND_READ) {
> - int pending;
> -
> + switch (as->suspend_state) {
> + case SUSPEND_READ:
> /*
> * If we read a suspend command from /dev/apm_bios,
> * then the corresponding APM_IOC_SUSPEND ioctl is
> * interpreted as an acknowledge.
> */
> as->suspend_state = SUSPEND_ACKED;
> - suspends_pending--;
> - pending = suspends_pending == 0;
> + atomic_dec(&suspend_acks_pending);
> mutex_unlock(&state_lock);
>
> /*
> - * If there are no further acknowledges required,
> - * suspend the system.
> + * suspend_acks_pending changed, the notifier needs to be
> + * woken up for this
> */
> - if (pending)
> - apm_suspend();
> + wake_up(&apm_suspend_waitqueue);
>
> /*
> * Wait for the suspend/resume to complete. If there
> @@ -340,35 +309,21 @@ apm_ioctl(struct inode * inode, struct f
> * try_to_freeze() in freezer_count() will not trigger
> */
> freezer_count();
> - } else {
> + break;
> + case SUSPEND_ACKTO:
> + as->suspend_result = -ETIMEDOUT;
> + mutex_unlock(&state_lock);
> + break;
> + default:
> as->suspend_state = SUSPEND_WAIT;
> mutex_unlock(&state_lock);
>
> /*
> * Otherwise it is a request to suspend the system.
> - * Queue an event for all readers, and expect an
> - * acknowledge from all writers who haven't already
> - * acknowledged.
> - */
> - err = queue_suspend_event(APM_USER_SUSPEND, as);
> - if (err < 0) {
> - /*
> - * Avoid taking the lock here - this
> - * should be fine.
> - */
> - as->suspend_state = SUSPEND_NONE;
> - break;
> - }
> -
> - if (err > 0)
> - apm_suspend();
> -
> - /*
> - * Wait for the suspend/resume to complete. If there
> - * are pending acknowledges, we wait here for them.
> + * Just invoke pm_suspend(), we'll handle it from
> + * there via the notifier.
> */
> - wait_event_freezable(apm_suspend_waitqueue,
> - as->suspend_state == SUSPEND_DONE);
> + as->suspend_result = pm_suspend(PM_SUSPEND_MEM);
> }
>
> mutex_lock(&state_lock);
> @@ -384,7 +339,6 @@ apm_ioctl(struct inode * inode, struct f
> static int apm_release(struct inode * inode, struct file * filp)
> {
> struct apm_user *as = filp->private_data;
> - int pending = 0;
>
> filp->private_data = NULL;
>
> @@ -394,18 +348,15 @@ static int apm_release(struct inode * in
>
> /*
> * We are now unhooked from the chain. As far as new
> - * events are concerned, we no longer exist. However, we
> - * need to balance suspends_pending, which means the
> - * possibility of sleeping.
> + * events are concerned, we no longer exist.
> */
> mutex_lock(&state_lock);
> - if (as->suspend_state != SUSPEND_NONE) {
> - suspends_pending -= 1;
> - pending = suspends_pending == 0;
> - }
> + if (as->suspend_state == SUSPEND_PENDING ||
> + as->suspend_state == SUSPEND_READ)
> + atomic_dec(&suspend_acks_pending);
> mutex_unlock(&state_lock);
> - if (pending)
> - apm_suspend();
> +
> + wake_up(&apm_suspend_waitqueue);
>
> kfree(as);
> return 0;
> @@ -529,7 +480,6 @@ static int kapmd(void *arg)
> {
> do {
> apm_event_t event;
> - int ret;
>
> wait_event_interruptible(kapmd_wait,
> !queue_empty(&kapmd_queue) || kthread_should_stop());
> @@ -554,20 +504,13 @@ static int kapmd(void *arg)
>
> case APM_USER_SUSPEND:
> case APM_SYS_SUSPEND:
> - ret = queue_suspend_event(event, NULL);
> - if (ret < 0) {
> - /*
> - * We were busy. Try again in 50ms.
> - */
> - queue_add_event(&kapmd_queue, event);
> - msleep(50);
> - }
> - if (ret > 0)
> - apm_suspend();
> + pm_suspend(PM_SUSPEND_MEM);
> break;
>
> case APM_CRITICAL_SUSPEND:
> - apm_suspend();
> + atomic_inc(&userspace_notification_inhibit);
> + pm_suspend(PM_SUSPEND_MEM);
> + atomic_dec(&userspace_notification_inhibit);
> break;
> }
> } while (1);
> @@ -575,6 +518,120 @@ static int kapmd(void *arg)
> return 0;
> }
>
> +static int apm_suspend_notifier(struct notifier_block *nb,
> + unsigned long event,
> + void *dummy)
> +{
> + struct apm_user *as;
> + int err;
> +
> + /* short-cut emergency suspends */
> + if (atomic_read(&userspace_notification_inhibit))
> + return NOTIFY_DONE;
> +
> + switch (event) {
> + case PM_SUSPEND_PREPARE:
> + /*
> + * Queue an event to all "writer" users that we want
> + * to suspend and need their ack.
> + */
> + mutex_lock(&state_lock);
> + down_read(&user_list_lock);
> +
> + list_for_each_entry(as, &apm_user_list, list) {
> + if (as->suspend_state != SUSPEND_WAIT && as->reader &&
> + as->writer && as->suser) {
> + as->suspend_state = SUSPEND_PENDING;
> + atomic_inc(&suspend_acks_pending);
> + queue_add_event(&as->queue, APM_USER_SUSPEND);
> + }
> + }
> +
> + up_read(&user_list_lock);
> + mutex_unlock(&state_lock);
> + wake_up_interruptible(&apm_waitqueue);
> +
> + /*
> + * Wait for the the suspend_acks_pending variable to drop to zero,
> + * meaning everybody acked the suspend event (or the process
> + * was killed.)
> + *
> + * If the app won't answer within a short while we assume it
> + * locked up and ignore it.
> + */
> + err = wait_event_interruptible_timeout(
> + apm_suspend_waitqueue,
> + atomic_read(&suspend_acks_pending) == 0,
> + 5*HZ);
> +
> + /* timed out */
> + if (err == 0) {
> + /*
> + * Move anybody who timed out to "ack timeout" state.
> + *
> + * We could time out and the userspace does the ACK
> + * right after we time out but before we enter the
> + * locked section here, but that's fine.
> + */
> + mutex_lock(&state_lock);
> + down_read(&user_list_lock);
> + list_for_each_entry(as, &apm_user_list, list) {
> + if (as->suspend_state == SUSPEND_PENDING ||
> + as->suspend_state == SUSPEND_READ) {
> + as->suspend_state = SUSPEND_ACKTO;
> + atomic_dec(&suspend_acks_pending);
> + }
> + }
> + up_read(&user_list_lock);
> + mutex_unlock(&state_lock);
> + }
> +
> + /* let suspend proceed */
> + if (err >= 0)
> + return NOTIFY_OK;
> +
> + /* interrupted by signal */
> + return NOTIFY_BAD;
> +
> + case PM_POST_SUSPEND:
> + /*
> + * Anyone on the APM queues will think we're still suspended.
> + * Send a message so everyone knows we're now awake again.
> + */
> + queue_event(APM_NORMAL_RESUME);
> +
> + /*
> + * Finally, wake up anyone who is sleeping on the suspend.
> + */
> + mutex_lock(&state_lock);
> + down_read(&user_list_lock);
> + list_for_each_entry(as, &apm_user_list, list) {
> + if (as->suspend_state == SUSPEND_ACKED) {
> + /*
> + * TODO: maybe grab error code, needs core
> + * changes to push the error to the notifier
> + * chain (could use the second parameter if
> + * implemented)
> + */
> + as->suspend_result = 0;
> + as->suspend_state = SUSPEND_DONE;
> + }
> + }
> + up_read(&user_list_lock);
> + mutex_unlock(&state_lock);
> +
> + wake_up(&apm_suspend_waitqueue);
> + return NOTIFY_OK;
> +
> + default:
> + return NOTIFY_DONE;
> + }
> +}
> +
> +static struct notifier_block apm_notif_block = {
> + .notifier_call = apm_suspend_notifier,
> +};
> +
> static int __init apm_init(void)
> {
> int ret;
> @@ -588,7 +645,7 @@ static int __init apm_init(void)
> if (IS_ERR(kapmd_tsk)) {
> ret = PTR_ERR(kapmd_tsk);
> kapmd_tsk = NULL;
> - return ret;
> + goto out;
> }
> wake_up_process(kapmd_tsk);
>
> @@ -597,16 +654,27 @@ static int __init apm_init(void)
> #endif
>
> ret = misc_register(&apm_device);
> - if (ret != 0) {
> - remove_proc_entry("apm", NULL);
> - kthread_stop(kapmd_tsk);
> - }
> + if (ret)
> + goto out_stop;
>
> + ret = register_pm_notifier(&apm_notif_block);
> + if (ret)
> + goto out_unregister;
> +
> + return 0;
> +
> + out_unregister:
> + misc_deregister(&apm_device);
> + out_stop:
> + remove_proc_entry("apm", NULL);
> + kthread_stop(kapmd_tsk);
> + out:
> return ret;
> }
>
> static void __exit apm_exit(void)
> {
> + unregister_pm_notifier(&apm_notif_block);
> misc_deregister(&apm_device);
> remove_proc_entry("apm", NULL);
>
>
>
>
>
--
"Premature optimization is the root of all evil." - Donald Knuth
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
