[KJ] Re: 12/34: cdrom/cdu31a: replace interruptible_sleep_on() with

[KJ] Re: 12/34: cdrom/cdu31a: replace interruptible_sleep_on() with

[Domen Puncer]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

On 25/01/05 15:14 -0800, Nishanth Aravamudan wrote:
> Hi,
> 
> Please consider applying.
> 
> Description: Use wait_event_interruptible() instead of the deprecated
> interruptible_sleep_on(). The patch is straight-forward as the macros should 
> result in the same execution. Patch is compile-tested (still throws out warnings
> regarding {save,restore}_flags()).

It also indirectly calls schedule() with interrupts disabled... this
can't be right?


	Domen

> 
>  	cli();
> -	while (sony_inuse) {
> -		interruptible_sleep_on(&sony_wait);
> -		if (signal_pending(current)) {
> -			restore_flags(flags);
> -			return -EAGAIN;
> -		}
> +	wait_event_interruptible(sony_wait, sony_inuse);
> +	if (signal_pending(current)) {
> +		restore_flags(flags);
> +		return -EAGAIN;

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] Re: 12/34: cdrom/cdu31a: replace interruptible_sleep_on()

[Nish Aravamudan]

[-- Attachment #1: Type: text/plain, Size: 1214 bytes --]

On Sun, 30 Jan 2005 13:10:27 +0100, Domen Puncer <domen@coderock.org> wrote:
> On 25/01/05 15:14 -0800, Nishanth Aravamudan wrote:
> > Hi,
> >
> > Please consider applying.
> >
> > Description: Use wait_event_interruptible() instead of the deprecated
> > interruptible_sleep_on(). The patch is straight-forward as the macros should
> > result in the same execution. Patch is compile-tested (still throws out warnings
> > regarding {save,restore}_flags()).
> 
> It also indirectly calls schedule() with interrupts disabled... this
> can't be right?
> 
> >       cli();
> > -     while (sony_inuse) {
> > -             interruptible_sleep_on(&sony_wait);
> > -             if (signal_pending(current)) {
> > -                     restore_flags(flags);
> > -                     return -EAGAIN;
> > -             }
> > +     wait_event_interruptible(sony_wait, sony_inuse);
> > +     if (signal_pending(current)) {
> > +             restore_flags(flags);
> > +             return -EAGAIN;

Yes, pretty much any driver that still calls cli()/sti() is broken in
one way or another. I think you are right, though, that the schedule()
with interrupts cleared makes little sense. I'm not sure how to fix it
though.

-Nish

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors