2006-08-01 18:06:59

by Dave Jones

[permalink] [raw]
Subject: deprecate and convert some sleep_on variants.

We've been carrying this for a dogs age in Fedora. It'd be good to get
this in -mm, so that it stands some chance of getting upstreamed at some point.

Signed-off-by: Arjan van de Ven <[email protected]>
Signed-off-by: Dave Jones <[email protected]>

diff -urNp --exclude-from=/home/davej/.exclude linux-1060/drivers/block/DAC960.c linux-1070/drivers/block/DAC960.c
--- linux-1060/drivers/block/DAC960.c
+++ linux-1070/drivers/block/DAC960.c
@@ -6132,6 +6132,9 @@ static boolean DAC960_V2_ExecuteUserComm
unsigned long flags;
unsigned char Channel, TargetID, LogicalDriveNumber;
unsigned short LogicalDeviceNumber;
+ 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);
+
while (Controller->V2.NewControllerInformation->PhysicalScanActive)
{
DAC960_ExecuteCommand(Command);
- sleep_on_timeout(&Controller->CommandWaitQueue, HZ);
+ schedule_timeout(HZ);
+ set_current_state(TASK_UNINTERRUPTIBLE);
}
+ current->state = TASK_RUNNING;
+ remove_wait_queue(&Controller->CommandWaitQueue, &__wait);
+
DAC960_UserCritical("Discovery Completed\n", Controller);
}
}
diff -urNp --exclude-from=/home/davej/.exclude linux-1060/drivers/net/tokenring/ibmtr.c linux-1070/drivers/net/tokenring/ibmtr.c
--- linux-1060/drivers/net/tokenring/ibmtr.c
+++ linux-1070/drivers/net/tokenring/ibmtr.c
@@ -850,6 +850,8 @@ static int tok_init_card(struct net_devi
struct tok_info *ti;
short PIOaddr;
unsigned long i;
+ wait_queue_t __wait;
+ init_waitqueue_entry(&__wait, current);

PIOaddr = dev->base_addr;
ti = (struct tok_info *) dev->priv;
@@ -862,13 +864,18 @@ static int tok_init_card(struct net_devi
current->state=TASK_UNINTERRUPTIBLE;
schedule_timeout(TR_RST_TIME); /* wait 50ms */

+ add_wait_queue(&ti->wait_for_reset, &__wait);
+ set_current_state(TASK_UNINTERRUPTIBLE);
outb(0, PIOaddr + ADAPTRESETREL);
#ifdef ENABLE_PAGING
if (ti->page_mask)
writeb(SRPR_ENABLE_PAGING,ti->mmio+ACA_OFFSET+ACA_RW+SRPR_EVEN);
#endif
writeb(INT_ENABLE, ti->mmio + ACA_OFFSET + ACA_SET + ISRP_EVEN);
- i = sleep_on_timeout(&ti->wait_for_reset, 4 * HZ);
+ #warning pci posting bug
+ i = schedule_timeout(4 * HZ);
+ current->state = TASK_RUNNING;
+ remove_wait_queue(&ti->wait_for_reset, &__wait);
return i? 0 : -EAGAIN;
}

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
@@ -364,10 +364,10 @@ static inline void remove_wait_queue_loc
* They are racy. DO NOT use them, use the wait_event* interfaces above.
* We plan to remove these interfaces during 2.7.
*/
-extern void FASTCALL(sleep_on(wait_queue_head_t *q));
-extern long FASTCALL(sleep_on_timeout(wait_queue_head_t *q,
+extern void __deprecated FASTCALL(sleep_on(wait_queue_head_t *q));
+extern long __deprecated FASTCALL(sleep_on_timeout(wait_queue_head_t *q,
signed long timeout));
-extern void FASTCALL(interruptible_sleep_on(wait_queue_head_t *q));
+extern void __deprecated FASTCALL(interruptible_sleep_on(wait_queue_head_t *q));
extern long FASTCALL(interruptible_sleep_on_timeout(wait_queue_head_t *q,
signed long timeout));

diff -urNp --exclude-from=/home/davej/.exclude linux-1060/kernel/sched.c linux-1070/kernel/sched.c
--- linux-1060/kernel/sched.c
+++ linux-1070/kernel/sched.c
@@ -3118,10 +3118,21 @@ EXPORT_SYMBOL(wait_for_completion_interr
__remove_wait_queue(q, &wait); \
spin_unlock_irqrestore(&q->lock, flags);

+#define SLEEP_ON_BKLCHECK \
+ if (unlikely(!kernel_locked()) && \
+ sleep_on_bkl_warnings < 10) { \
+ sleep_on_bkl_warnings++; \
+ WARN_ON(1); \
+ }
+
+static int sleep_on_bkl_warnings;
+
void fastcall __sched interruptible_sleep_on(wait_queue_head_t *q)
{
SLEEP_ON_VAR

+ SLEEP_ON_BKLCHECK
+
current->state = TASK_INTERRUPTIBLE;

SLEEP_ON_HEAD
@@ -3135,6 +3146,8 @@ long fastcall __sched interruptible_slee
{
SLEEP_ON_VAR

+ SLEEP_ON_BKLCHECK
+
current->state = TASK_INTERRUPTIBLE;

SLEEP_ON_HEAD
@@ -3759,22 +3759,12 @@ interruptible_sleep_on_timeout(wait_queu
}
EXPORT_SYMBOL(interruptible_sleep_on_timeout);

-void fastcall __sched sleep_on(wait_queue_head_t *q)
-{
- SLEEP_ON_VAR
-
- current->state = TASK_UNINTERRUPTIBLE;
-
- SLEEP_ON_HEAD
- schedule();
- SLEEP_ON_TAIL
-}
-EXPORT_SYMBOL(sleep_on);
-
long fastcall __sched sleep_on_timeout(wait_queue_head_t *q, long timeout)
{
SLEEP_ON_VAR

+ SLEEP_ON_BKLCHECK
+
current->state = TASK_UNINTERRUPTIBLE;

SLEEP_ON_HEAD

--
http://www.codemonkey.org.uk


2006-08-01 18:20:33

by Nish Aravamudan

[permalink] [raw]
Subject: Re: deprecate and convert some sleep_on variants.

On 8/1/06, Dave Jones <[email protected]> wrote:
> We've been carrying this for a dogs age in Fedora. It'd be good to get
> this in -mm, so that it stands some chance of getting upstreamed at some point.
>
> Signed-off-by: Arjan van de Ven <[email protected]>
> Signed-off-by: Dave Jones <[email protected]>
>
> diff -urNp --exclude-from=/home/davej/.exclude linux-1060/drivers/block/DAC960.c linux-1070/drivers/block/DAC960.c
> --- linux-1060/drivers/block/DAC960.c
> +++ linux-1070/drivers/block/DAC960.c
> @@ -6132,6 +6132,9 @@ static boolean DAC960_V2_ExecuteUserComm
> unsigned long flags;
> unsigned char Channel, TargetID, LogicalDriveNumber;
> unsigned short LogicalDeviceNumber;
> + 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()

> +
> 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)

> }
> + current->state = TASK_RUNNING;
> + remove_wait_queue(&Controller->CommandWaitQueue, &__wait);

and finish_wait()?

Same for ibmtr.c ?

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?

Thanks,
Nish

2006-08-01 19:04:27

by Steven Rostedt

[permalink] [raw]
Subject: Re: deprecate and convert some sleep_on variants.

On Tue, 2006-08-01 at 13:20 -0500, Nish Aravamudan wrote:
> On 8/1/06, Dave Jones <[email protected]> wrote:
> > We've been carrying this for a dogs age in Fedora. It'd be good to get
> > this in -mm, so that it stands some chance of getting upstreamed at some point.
> >
> > Signed-off-by: Arjan van de Ven <[email protected]>
> > Signed-off-by: Dave Jones <[email protected]>
> >

> 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?

As well as the changes to kernel/sched.c

-- Steve


2006-08-02 01:09:27

by Dave Jones

[permalink] [raw]
Subject: Re: deprecate and convert some sleep_on variants.

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

2006-08-02 05:57:46

by Rolf Eike Beer

[permalink] [raw]
Subject: Re: deprecate and convert some sleep_on variants.

Am Dienstag, 1. August 2006 20:06 schrieb Dave Jones:
> We've been carrying this for a dogs age in Fedora. It'd be good to get
> this in -mm, so that it stands some chance of getting upstreamed at some
> point.
>
> Signed-off-by: Arjan van de Ven <[email protected]>
> Signed-off-by: Dave Jones <[email protected]>
>
> diff -urNp --exclude-from=/home/davej/.exclude
> linux-1060/drivers/block/DAC960.c linux-1070/drivers/block/DAC960.c ---
> linux-1060/drivers/block/DAC960.c
> +++ linux-1070/drivers/block/DAC960.c
> @@ -6132,6 +6132,9 @@ static boolean DAC960_V2_ExecuteUserComm
> unsigned long flags;
> unsigned char Channel, TargetID, LogicalDriveNumber;
> unsigned short LogicalDeviceNumber;
> + wait_queue_t __wait;
> +
> + init_waitqueue_entry(&__wait, current);

Is this only my opinion or is this really a bad choice for the queues name?

Eike


Attachments:
(No filename) (865.00 B)
(No filename) (189.00 B)
Download all attachments