From: Dave Jones <davej@redhat.com>
To: Nish Aravamudan <nish.aravamudan@gmail.com>
Cc: arjan@infradead.org, Linux Kernel <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@osdl.org>
Subject: Re: deprecate and convert some sleep_on variants.
Date: Tue, 1 Aug 2006 21:09:15 -0400 [thread overview]
Message-ID: <20060802010915.GC22589@redhat.com> (raw)
In-Reply-To: <29495f1d0608011120j8103c5bwd169367ee2d67bc0@mail.gmail.com>
On Tue, Aug 01, 2006 at 01:20:28PM -0500, Nish Aravamudan wrote:
> >+ wait_queue_t __wait;
> >+
> >+ init_waitqueue_entry(&__wait, current);
> >
> > spin_lock_irqsave(&Controller->queue_lock, flags);
> > while ((Command = DAC960_AllocateCommand(Controller)) == NULL)
> >@@ -6314,11 +6317,18 @@ static boolean DAC960_V2_ExecuteUserComm
> > .SegmentByteCount =
> > CommandMailbox->ControllerInfo.DataTransferSize;
> > DAC960_ExecuteCommand(Command);
> >+ add_wait_queue(&Controller->CommandWaitQueue, &__wait);
> >+ set_current_state(TASK_UNINTERRUPTIBLE);
>
> Could this use prepare_to_wait()
Maybe, though I'd rather not do that conversion with the hardware to test it.
sidenote: prepare_to_wait() and friends could really use some kerneldoc explaining
their purpose rather than their internal workings.
> > while
> > (Controller->V2.NewControllerInformation->PhysicalScanActive)
> > {
> > DAC960_ExecuteCommand(Command);
> >- sleep_on_timeout(&Controller->CommandWaitQueue, HZ);
> >+ schedule_timeout(HZ);
> >+ set_current_state(TASK_UNINTERRUPTIBLE);
>
> and schedule_timeout_uninterruptible() (which is redundant for the
> first invocation, I suppose)
Makes sense.
> >+ current->state = TASK_RUNNING;
> >+ remove_wait_queue(&Controller->CommandWaitQueue, &__wait);
>
> and finish_wait()?
>
> Same for ibmtr.c ?
Same comments as above.
> Also, would these changes:
>
> >diff -urNp --exclude-from=/home/davej/.exclude
> >linux-1060/include/linux/wait.h linux-1070/include/linux/wait.h
> >--- linux-1060/include/linux/wait.h
> >+++ linux-1070/include/linux/wait.h
>
> Be better in a separate patch?
A split-up patchset would for sure make sense for committing upstream.
Though, at least each file touched here is a separate cset.
Dave
--
http://www.codemonkey.org.uk
next prev parent reply other threads:[~2006-08-02 1:09 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-08-01 18:06 deprecate and convert some sleep_on variants Dave Jones
2006-08-01 18:20 ` Nish Aravamudan
2006-08-01 19:03 ` Steven Rostedt
2006-08-02 1:09 ` Dave Jones [this message]
2006-08-02 6:00 ` Rolf Eike Beer
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=20060802010915.GC22589@redhat.com \
--to=davej@redhat.com \
--cc=akpm@osdl.org \
--cc=arjan@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nish.aravamudan@gmail.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox