[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Jody McIntyre]

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

/me finally gets time to review this...

This doesn't apply to anything I can find.  Patches against 1394 svn
( svn://svn.linux1394.org/libraw1394/ ) or development bk
( http://linux-1394.bkbits.net/1394-dev ) are most useful, but I can
deal with others.


On Tue, Jan 25, 2005 at 04:10:44PM -0800, Nishanth Aravamudan wrote:
> Hi,
> 
> I mistakenly sent to the wrong address for the list. Please reply in CC to
> bcollins@debian.org and kernel-janitors@lists.osdl.org. Thanks!
> 
> Please consider applying.
> 
> Description: Use wait_event_interruptible() instead of the deprecated
> interruptible_sleep_on(). The first change is simply to clean up the code a
> little to make it clearer. The second actually does a replacement, mimicking
> exactly the first. I removed the #if 1/#else/endif logic, as it duplicated the
> same code. Patch is compile-tested.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> 
> --- 2.6.11-rc2-kj-v/drivers/ieee1394/video1394.c	2005-01-24 09:34:07.000000000 -0800
> +++ 2.6.11-rc2-kj/drivers/ieee1394/video1394.c	2005-01-24 15:06:16.000000000 -0800
> @@ -964,10 +964,9 @@ static int __video1394_ioctl(struct file
>  				}
>  			}
>  #else
> -			if (wait_event_interruptible(d->waitq,
> -						     d->buffer_status[v.buffer]
> -						     == VIDEO1394_BUFFER_READY)
> -			    == -ERESTARTSYS)
> +			wait_event_interruptible(d->waitq,
> +					(d->buffer_status[v.buffer] == VIDEO1394_BUFFER_READY));
> +			if (signal_pending(current))
>  				return -EINTR;

Fine, but that only touches the #else of an #if 1 and leaves an
interruptible_sleep_on in the part that's actually executed.

Can you change this to be more like hunk 2?

For what it's worth, the #if 1 has been there since 2002 so it's
probably safe to remove by now :)

Thanks,
Jody

>  #endif
>  			d->buffer_status[v.buffer]=VIDEO1394_BUFFER_FREE;
> @@ -1126,19 +1125,10 @@ static int __video1394_ioctl(struct file
>  			d->buffer_status[v.buffer]=VIDEO1394_BUFFER_FREE;
>  			return 0;
>  		case VIDEO1394_BUFFER_QUEUED:
> -#if 1
> -			while (d->buffer_status[v.buffer]!=
> -			      VIDEO1394_BUFFER_READY) {
> -				interruptible_sleep_on(&d->waitq);
> -				if (signal_pending(current)) return -EINTR;
> -			}
> -#else
> -			if (wait_event_interruptible(d->waitq,
> -						     d->buffer_status[v.buffer]
> -						     == VIDEO1394_BUFFER_READY)
> -			    == -ERESTARTSYS)
> +			wait_event_interruptible(d->waitq,
> +					(d->buffer_status[v.buffer] == VIDEO1394_BUFFER_READY));
> +			if (signal_pending(current))
>  				return -EINTR;
> -#endif
>  			d->buffer_status[v.buffer]=VIDEO1394_BUFFER_FREE;
>  			return 0;
>  		default:
> 
> 
> -------------------------------------------------------
> This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
> Tool for open source databases. Create drag-&-drop reports. Save time
> by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
> Download a FREE copy at http://www.intelliview.com/go/osdn_nl
> _______________________________________________
> mailing list linux1394-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux1394-devel

-- 

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

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Nishanth Aravamudan]

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

On Mon, Feb 14, 2005 at 07:20:49PM -0500, Jody McIntyre wrote:
> /me finally gets time to review this...
> 
> This doesn't apply to anything I can find.  Patches against 1394 svn
> ( svn://svn.linux1394.org/libraw1394/ ) or development bk
> ( http://linux-1394.bkbits.net/1394-dev ) are most useful, but I can
> deal with others.

OK, I will add that to my notes for ieee1394. Thanks!

> On Tue, Jan 25, 2005 at 04:10:44PM -0800, Nishanth Aravamudan wrote:
> > Hi,
> > 
> > I mistakenly sent to the wrong address for the list. Please reply in CC to
> > bcollins@debian.org and kernel-janitors@lists.osdl.org. Thanks!
> > 
> > Please consider applying.
> > 
> > Description: Use wait_event_interruptible() instead of the deprecated
> > interruptible_sleep_on(). The first change is simply to clean up the code a
> > little to make it clearer. The second actually does a replacement, mimicking
> > exactly the first. I removed the #if 1/#else/endif logic, as it duplicated the
> > same code. Patch is compile-tested.
> > 
> > Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> > 
> > --- 2.6.11-rc2-kj-v/drivers/ieee1394/video1394.c	2005-01-24 09:34:07.000000000 -0800
> > +++ 2.6.11-rc2-kj/drivers/ieee1394/video1394.c	2005-01-24 15:06:16.000000000 -0800
> > @@ -964,10 +964,9 @@ static int __video1394_ioctl(struct file
> >  				}
> >  			}
> >  #else
> > -			if (wait_event_interruptible(d->waitq,
> > -						     d->buffer_status[v.buffer]
> > -						     == VIDEO1394_BUFFER_READY)
> > -			    == -ERESTARTSYS)
> > +			wait_event_interruptible(d->waitq,
> > +					(d->buffer_status[v.buffer] == VIDEO1394_BUFFER_READY));
> > +			if (signal_pending(current))
> >  				return -EINTR;
> 
> Fine, but that only touches the #else of an #if 1 and leaves an
> interruptible_sleep_on in the part that's actually executed.
> 
> Can you change this to be more like hunk 2?
> 
> For what it's worth, the #if 1 has been there since 2002 so it's
> probably safe to remove by now :)

I will look into it, but from a cursory glance, the #if 1 case uses
locking around the sleep, which is currently not supported by
wait_event*(). I have sent a patch to LKML regarding this issue and the
need, perhaps, for alternative wait_event*() style functions, such as
ones that accept locks as parameters.

Any suggestions?

Thanks,
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

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Jody McIntyre]

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

On Mon, Feb 14, 2005 at 04:32:09PM -0800, Nishanth Aravamudan wrote:
> [...]
> 
> I will look into it, but from a cursory glance, the #if 1 case uses
> locking around the sleep, which is currently not supported by
> wait_event*(). I have sent a patch to LKML regarding this issue and the
> need, perhaps, for alternative wait_event*() style functions, such as
> ones that accept locks as parameters.

In my copy, it _unlocks_ around the sleep.  I don't see why this
couldn't be done with wait_event_interruptible (unlock, wait, lock).
Can you produce a patch with that?  I'll find a video1394 user to test
it.

Thanks,
Jody

> 
> Any suggestions?
> 
> Thanks,
> Nish
> 
> 
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from real users.
> Discover which products truly live up to the hype. Start reading now.
> http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
> _______________________________________________
> mailing list linux1394-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux1394-devel

-- 

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

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Nishanth Aravamudan]

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

On Mon, Feb 14, 2005 at 07:42:37PM -0500, Jody McIntyre wrote:
> On Mon, Feb 14, 2005 at 04:32:09PM -0800, Nishanth Aravamudan wrote:
> > [...]
> > 
> > I will look into it, but from a cursory glance, the #if 1 case uses
> > locking around the sleep, which is currently not supported by
> > wait_event*(). I have sent a patch to LKML regarding this issue and the
> > need, perhaps, for alternative wait_event*() style functions, such as
> > ones that accept locks as parameters.
> 
> In my copy, it _unlocks_ around the sleep.  I don't see why this
> couldn't be done with wait_event_interruptible (unlock, wait, lock).
> Can you produce a patch with that?  I'll find a video1394 user to test
> it.

Sorry for my mis-statement/lack of clarity. In my copy, it also unlocks
around the sleep... The problem is that wait_event*() should be used in
such a way that the while-loop is not at all necessary (ideally).
Instead, we'll have a while-loop around the unlock-wait_event-lock code,
which is just ugly to me, as wait_event is just a macro around another
while loop. The real solution is wait_event_interruptible_lock().
Hopefully I can get such a macro accepted into mainline sooner or
later...

I'm assuming the locking is done this way so that other variables protected by
the same lock can be changed while this one sleeps?

Thanks,
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

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Jody McIntyre]

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

On Mon, Feb 14, 2005 at 04:50:47PM -0800, Nishanth Aravamudan wrote:

> > In my copy, it _unlocks_ around the sleep.  I don't see why this
> > couldn't be done with wait_event_interruptible (unlock, wait, lock).
> > Can you produce a patch with that?  I'll find a video1394 user to test
> > it.
> 
> Sorry for my mis-statement/lack of clarity. In my copy, it also unlocks
> around the sleep... The problem is that wait_event*() should be used in
> such a way that the while-loop is not at all necessary (ideally).
> Instead, we'll have a while-loop around the unlock-wait_event-lock code,
> which is just ugly to me, as wait_event is just a macro around another
> while loop. The real solution is wait_event_interruptible_lock().
> Hopefully I can get such a macro accepted into mainline sooner or
> later...

So you want to leave tho old code in until then?  Fair enough.

> I'm assuming the locking is done this way so that other variables protected by
> the same lock can be changed while this one sleeps?

Correct.  See, for example, wakeup_dma_ir_ctx() .

Jody

> 
> Thanks,
> Nish
> 
> 
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from real users.
> Discover which products truly live up to the hype. Start reading now.
> http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
> _______________________________________________
> mailing list linux1394-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux1394-devel

-- 

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

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Nishanth Aravamudan]

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

On Mon, Feb 14, 2005 at 08:34:20PM -0500, Jody McIntyre wrote:
> On Mon, Feb 14, 2005 at 04:50:47PM -0800, Nishanth Aravamudan wrote:
> 
> > > In my copy, it _unlocks_ around the sleep.  I don't see why this
> > > couldn't be done with wait_event_interruptible (unlock, wait, lock).
> > > Can you produce a patch with that?  I'll find a video1394 user to test
> > > it.
> > 
> > Sorry for my mis-statement/lack of clarity. In my copy, it also unlocks
> > around the sleep... The problem is that wait_event*() should be used in
> > such a way that the while-loop is not at all necessary (ideally).
> > Instead, we'll have a while-loop around the unlock-wait_event-lock code,
> > which is just ugly to me, as wait_event is just a macro around another
> > while loop. The real solution is wait_event_interruptible_lock().
> > Hopefully I can get such a macro accepted into mainline sooner or
> > later...
> 
> So you want to leave tho old code in until then?  Fair enough.

Yup, I think that's the best solution for now.

> > I'm assuming the locking is done this way so that other variables protected by
> > the same lock can be changed while this one sleeps?
> 
> Correct.  See, for example, wakeup_dma_ir_ctx() .

That's what I thought, but I just wanted to get confirmation from you :)
Thanks for all the feedback! I will send an updated patch against the
bk-repo once the other wait_event*() macros go in (expect it before
2.6.12's full release).

Thanks,
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

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Nishanth Aravamudan]

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

On Mon, Feb 14, 2005 at 08:34:20PM -0500, Jody McIntyre wrote:
> On Mon, Feb 14, 2005 at 04:50:47PM -0800, Nishanth Aravamudan wrote:
> 
> > > In my copy, it _unlocks_ around the sleep.  I don't see why this
> > > couldn't be done with wait_event_interruptible (unlock, wait, lock).
> > > Can you produce a patch with that?  I'll find a video1394 user to test
> > > it.
> > 
> > Sorry for my mis-statement/lack of clarity. In my copy, it also unlocks
> > around the sleep... The problem is that wait_event*() should be used in
> > such a way that the while-loop is not at all necessary (ideally).
> > Instead, we'll have a while-loop around the unlock-wait_event-lock code,
> > which is just ugly to me, as wait_event is just a macro around another
> > while loop. The real solution is wait_event_interruptible_lock().
> > Hopefully I can get such a macro accepted into mainline sooner or
> > later...
> 
> So you want to leave tho old code in until then?  Fair enough.
> 
> > I'm assuming the locking is done this way so that other variables protected by
> > the same lock can be changed while this one sleeps?
> 
> Correct.  See, for example, wakeup_dma_ir_ctx() .

I am not sure if you have seen some of the discussion (under the
subject: "[RFC UPDATE PATCH] add wait_event_*_lock() functions and
comments") on LKML regarding these macros. Does Arnd's proposal to fix
video1394 seem reasonable? I can throw up a patch, if you'd like to
test.

Thanks,
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

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Jody McIntyre]

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

On Tue, Feb 15, 2005 at 10:18:11AM -0800, Nishanth Aravamudan wrote:

> I am not sure if you have seen some of the discussion (under the
> subject: "[RFC UPDATE PATCH] add wait_event_*_lock() functions and
> comments") on LKML regarding these macros. Does Arnd's proposal to fix
> video1394 seem reasonable? I can throw up a patch, if you'd like to
> test.

I just read it now.  Arnd's suggestion looks like it will work.  I
thought of something similar while reading the thread and his code looks
correct.  If you have time to make a patch, I'll find some video1394
users to test it.

Thanks,
Jody

> 
> Thanks,
> Nish
> 
> 
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from real users.
> Discover which products truly live up to the hype. Start reading now.
> http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
> _______________________________________________
> mailing list linux1394-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux1394-devel

-- 

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

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Nishanth Aravamudan]

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

On Tue, Feb 15, 2005 at 05:03:40PM -0500, Jody McIntyre wrote:
> On Tue, Feb 15, 2005 at 10:18:11AM -0800, Nishanth Aravamudan wrote:
> 
> > I am not sure if you have seen some of the discussion (under the
> > subject: "[RFC UPDATE PATCH] add wait_event_*_lock() functions and
> > comments") on LKML regarding these macros. Does Arnd's proposal to fix
> > video1394 seem reasonable? I can throw up a patch, if you'd like to
> > test.
> 
> I just read it now.  Arnd's suggestion looks like it will work.  I
> thought of something similar while reading the thread and his code looks
> correct.  If you have time to make a patch, I'll find some video1394
> users to test it.

If someone could test the following, I think it implements exactly what
Arnd suggested...

Thanks,
Nish

--- 2.6.11-rc4-v/drivers/ieee1394/video1394.c	2005-02-21 09:44:31.000000000 -0800
+++ 2.6.11-rc4/drivers/ieee1394/video1394.c	2005-02-21 09:38:37.000000000 -0800
@@ -699,6 +699,17 @@ static void initialize_dma_it_ctx(struct
 	reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, 1<<d->ctx);
 }
 
+static inline unsigned video1394_buffer_state(struct dma_iso_ctx *d,
+					      unsigned int buffer)
+{
+	unsigned long flags;
+	unsigned int ret;
+	spin_lock_irqsave(&d->lock, flags);
+	ret = d->buffer_status[buffer];
+	spin_unlock_irqrestore(&d->lock, flags);
+	return ret;
+}
+
 static int __video1394_ioctl(struct file *file,
 			     unsigned int cmd, unsigned long arg)
 {
@@ -952,24 +963,12 @@ static int __video1394_ioctl(struct file
 			    return -EINTR;
 			}
 
-#if 1
-			while (d->buffer_status[v.buffer]!=
-			      VIDEO1394_BUFFER_READY) {
-				spin_unlock_irqrestore(&d->lock, flags);
-				interruptible_sleep_on(&d->waitq);
-				spin_lock_irqsave(&d->lock, flags);
-				if (signal_pending(current)) {
-					spin_unlock_irqrestore(&d->lock,flags);
-					return -EINTR;
-				}
-			}
-#else
-			if (wait_event_interruptible(d->waitq,
-						     d->buffer_status[v.buffer]
-						     == VIDEO1394_BUFFER_READY)
-			    == -ERESTARTSYS)
+			spin_unlock_irqrestore(&d->lock, flags);
+			wait_event_interruptible(d->waitq,
+					video1394_buffer_state(d, v.buffer) == VIDEO1394_BUFFER_READY);
+			if (signal_pending(current)) {
 				return -EINTR;
-#endif
+			spin_lock_irqsave(&d->lock, flags);
 			d->buffer_status[v.buffer]=VIDEO1394_BUFFER_FREE;
 			break;
 		default:

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

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Jody McIntyre]

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

Please add a Signed-off-by: line.

(more comments below)

On Mon, Feb 21, 2005 at 09:46:23AM -0800, Nishanth Aravamudan wrote:

> --- 2.6.11-rc4-v/drivers/ieee1394/video1394.c	2005-02-21 09:44:31.000000000 -0800
> +++ 2.6.11-rc4/drivers/ieee1394/video1394.c	2005-02-21 09:38:37.000000000 -0800
> @@ -699,6 +699,17 @@ static void initialize_dma_it_ctx(struct
>  	reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, 1<<d->ctx);
>  }
>  
> +static inline unsigned video1394_buffer_state(struct dma_iso_ctx *d,
> +					      unsigned int buffer)
> +{
> +	unsigned long flags;
> +	unsigned int ret;
> +	spin_lock_irqsave(&d->lock, flags);
> +	ret = d->buffer_status[buffer];
> +	spin_unlock_irqrestore(&d->lock, flags);
> +	return ret;
> +}
> +
>  static int __video1394_ioctl(struct file *file,
>  			     unsigned int cmd, unsigned long arg)
>  {
> @@ -952,24 +963,12 @@ static int __video1394_ioctl(struct file
>  			    return -EINTR;
>  			}
>  
> -#if 1
> -			while (d->buffer_status[v.buffer]!=
> -			      VIDEO1394_BUFFER_READY) {
> -				spin_unlock_irqrestore(&d->lock, flags);
> -				interruptible_sleep_on(&d->waitq);
> -				spin_lock_irqsave(&d->lock, flags);
> -				if (signal_pending(current)) {
> -					spin_unlock_irqrestore(&d->lock,flags);
> -					return -EINTR;
> -				}
> -			}
> -#else
> -			if (wait_event_interruptible(d->waitq,
> -						     d->buffer_status[v.buffer]
> -						     == VIDEO1394_BUFFER_READY)
> -			    == -ERESTARTSYS)
> +			spin_unlock_irqrestore(&d->lock, flags);
> +			wait_event_interruptible(d->waitq,
> +					video1394_buffer_state(d, v.buffer) == VIDEO1394_BUFFER_READY);
> +			if (signal_pending(current)) {

Extra {.

>  				return -EINTR;
> -#endif
> +			spin_lock_irqsave(&d->lock, flags);
>  			d->buffer_status[v.buffer]=VIDEO1394_BUFFER_FREE;
>  			break;
>  		default:

With this fixed, it compiles.  I'll find a video1394 user to test it.

Thanks,
Jody

-- 

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

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Nishanth Aravamudan]

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

On Tue, Feb 22, 2005 at 12:29:02PM -0500, Jody McIntyre wrote:
> Please add a Signed-off-by: line.

Will do, I just wanted to get you a patch to test first. If it works,
I'll send an "official" one :)

> (more comments below)

<snip>

> > +			if (signal_pending(current)) {
> 
> Extra {.

Whoops, thanks for the catch. Let us know how the testing goes...

-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

[KJ] Re: [PATCH] 18/34: ieee1394/video1394: replace

[Jody McIntyre]

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

On Tue, Feb 22, 2005 at 09:37:33AM -0800, Nishanth Aravamudan wrote:
> On Tue, Feb 22, 2005 at 12:29:02PM -0500, Jody McIntyre wrote:
> > Please add a Signed-off-by: line.
> 
> Will do, I just wanted to get you a patch to test first. If it works,
> I'll send an "official" one :)

It's been over a week and nobody's reported any problems with your
patch.  Please send an "official" one and I will apply it.

Thanks,
Jody

> 
> > (more comments below)
> 
> <snip>
> 
> > > +			if (signal_pending(current)) {
> > 
> > Extra {.
> 
> Whoops, thanks for the catch. Let us know how the testing goes...
> 
> -Nish
> 
> 
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from real users.
> Discover which products truly live up to the hype. Start reading now.
> http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
> _______________________________________________
> mailing list linux1394-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux1394-devel

-- 

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