linux-laptop.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

From: Nishanth Aravamudan <nacc@us.ibm.com>
To: sfr@canb.auug.org.au
Cc: linux-laptop@vger.kernel.org
Subject: apm_mainloop() waitqueue usage
Date: Thu, 16 Dec 2004 12:06:19 -0800	[thread overview]
Message-ID: <20041216200619.GD2565@us.ibm.com> (raw)

Hello,

I am working on a Kernel Janitors TODO entry dealing with the use of
schedule_timeout(). In the process, I came across
arch/i386/kernel/apm.c::apm_mainloop(), which uses waitqueues &
schedule_timeout() to check for events at a regular interval, depending on
waitqueue events occurring.

A few considerations:

Ideally, I would like to convert as many waitqueue users as possible to the
wait_event* family of macros. For reference, here is the code in 2.6.10-rc3:


static void apm_mainloop(void)
{
	DECLARE_WAITQUEUE(wait, current);

	add_wait_queue(&apm_waitqueue, &wait);
	set_current_state(TASK_INTERRUPTIBLE);
	for (;;) {
		schedule_timeout(APM_CHECK_TIMEOUT);
		if (exit_kapmd)
			break;
		/*
		 * Ok, check all events, check for idle (and mark us sleeping
		 * so as not to count towards the load average)..
		 */
		set_current_state(TASK_INTERRUPTIBLE);
		apm_event_handler();
	}
	remove_wait_queue(&apm_waitqueue, &wait);
}

I have already easily changed the pre-loop initialization to:

	DECLARE_WAITQUEUE(wait, current);

	prepare_to_wait(&apm_waitqueue, &wait, TASK_UNINTERRUPTIBLE);

First, I changed the state to TASK_UNINTERRUPTIBLE. The code as it is does not
deal with signals in any way (that I see); I believe that is incorrect for
TASK_INTERRUPTIBLE sleeps, as the code must deal with both potential waitqueue
events & signals as being the cause for early wakeups. Please correct me if I
misunderstood.

Second, if the call to apm_event_handler() were not there, the change to
wait_event_timeout() would be very straightforward, as the code would be
identical... However, the call is there. So, I have a few questions:

Do you think it would be possible to perhaps change the structuring of the
function? I don't think it is, but it can't hurt to ask.

Second, if I added another macro to the wait_event family which allows the
invocation of a function at an interval (effectively exactly what you do in
apm_mainloop), would you be willing to convert to this macro? -- presuming of
course, the community agreed to merge the new macro to mainline.

Third, if neither of these options appeals to you, I may just push a patch
which does the prepare_to_wait() change.

Thanks,
Nish

                 reply	other threads:[~2004-12-16 20:06 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20041216200619.GD2565@us.ibm.com \
    --to=nacc@us.ibm.com \
    --cc=linux-laptop@vger.kernel.org \
    --cc=sfr@canb.auug.org.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).