Any comments would be, as always, appreciated.
-Nish
Reorder add_wait_queue() and set_current_state() as a
signal could be lost in between the two functions.
Signed-off-by: Nishanth Aravamudan <[email protected]>
Signed-off-by: Domen Puncer <[email protected]>
---
kj-domen/drivers/char/snsc.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff -puN drivers/char/snsc.c~reorder-state-drivers_char_snsc drivers/char/snsc.c
--- kj/drivers/char/snsc.c~reorder-state-drivers_char_snsc 2005-03-05 16:09:54.000000000 +0100
+++ kj-domen/drivers/char/snsc.c 2005-03-05 16:09:54.000000000 +0100
@@ -192,8 +192,8 @@ scdrv_read(struct file *file, char __use
}
len = CHUNKSIZE;
- add_wait_queue(&sd->sd_rq, &wait);
set_current_state(TASK_INTERRUPTIBLE);
+ add_wait_queue(&sd->sd_rq, &wait);
spin_unlock_irqrestore(&sd->sd_rlock, flags);
schedule_timeout(SCDRV_TIMEOUT);
@@ -288,8 +288,8 @@ scdrv_write(struct file *file, const cha
return -EAGAIN;
}
- add_wait_queue(&sd->sd_wq, &wait);
set_current_state(TASK_INTERRUPTIBLE);
+ add_wait_queue(&sd->sd_wq, &wait);
spin_unlock_irqrestore(&sd->sd_wlock, flags);
schedule_timeout(SCDRV_TIMEOUT);
_
On Sunday, March 6, 2005 2:36 pm, [email protected] wrote:
> Any comments would be, as always, appreciated.
I don't have a problem with this change, but the maintainer probably should
have been Cc'd. Greg, does this change look ok to you? Note that it's
already been committed to the upstream tree, so if it's a bad change we'll
need to have the cset excluded or something.
>
> -Nish
>
> Reorder add_wait_queue() and set_current_state() as a
> signal could be lost in between the two functions.
>
> Signed-off-by: Nishanth Aravamudan <[email protected]>
> Signed-off-by: Domen Puncer <[email protected]>
> ---
>
>
> kj-domen/drivers/char/snsc.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff -puN drivers/char/snsc.c~reorder-state-drivers_char_snsc
> drivers/char/snsc.c ---
> kj/drivers/char/snsc.c~reorder-state-drivers_char_snsc 2005-03-05
> 16:09:54.000000000 +0100 +++ kj-domen/drivers/char/snsc.c 2005-03-05
> 16:09:54.000000000 +0100 @@ -192,8 +192,8 @@ scdrv_read(struct file *file,
> char __use
> }
>
> len = CHUNKSIZE;
> - add_wait_queue(&sd->sd_rq, &wait);
> set_current_state(TASK_INTERRUPTIBLE);
> + add_wait_queue(&sd->sd_rq, &wait);
> spin_unlock_irqrestore(&sd->sd_rlock, flags);
>
> schedule_timeout(SCDRV_TIMEOUT);
> @@ -288,8 +288,8 @@ scdrv_write(struct file *file, const cha
> return -EAGAIN;
> }
>
> - add_wait_queue(&sd->sd_wq, &wait);
> set_current_state(TASK_INTERRUPTIBLE);
> + add_wait_queue(&sd->sd_wq, &wait);
> spin_unlock_irqrestore(&sd->sd_wlock, flags);
>
> schedule_timeout(SCDRV_TIMEOUT);
> _
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Mon, Mar 14, 2005 at 09:45:44AM -0800, Jesse Barnes wrote:
> On Sunday, March 6, 2005 2:36 pm, [email protected] wrote:
> > Any comments would be, as always, appreciated.
>
> I don't have a problem with this change, but the maintainer probably should
> have been Cc'd. Greg, does this change look ok to you? Note that it's
> already been committed to the upstream tree, so if it's a bad change we'll
> need to have the cset excluded or something.
Thanks for the feedback. I don't see a maintainer entry for Greg
anywhere in the snsc.{c,h} files or MAINTAINERS. Could someone throw a
patch upstream so that whomever should be contacted for this driver will
be in the future?
Thanks,
Nish
On Monday, March 14, 2005 9:54 am, Nishanth Aravamudan wrote:
> On Mon, Mar 14, 2005 at 09:45:44AM -0800, Jesse Barnes wrote:
> > On Sunday, March 6, 2005 2:36 pm, [email protected] wrote:
> > > Any comments would be, as always, appreciated.
> >
> > I don't have a problem with this change, but the maintainer probably
> > should have been Cc'd. Greg, does this change look ok to you? Note that
> > it's already been committed to the upstream tree, so if it's a bad change
> > we'll need to have the cset excluded or something.
>
> Thanks for the feedback. I don't see a maintainer entry for Greg
> anywhere in the snsc.{c,h} files or MAINTAINERS. Could someone throw a
> patch upstream so that whomever should be contacted for this driver will
> be in the future?
Good point :) I assumed there was a MODULE_AUTHOR entry. Here's a patch to
get Greg more spam^W^W^W^Wadd the relevant info. Look ok to you Greg?
Signed-off-by: Jesse Barnes <[email protected]>
Jesse
On Mon, 14 Mar 2005, Jesse Barnes wrote:
> On Sunday, March 6, 2005 2:36 pm, [email protected] wrote:
> > Any comments would be, as always, appreciated.
>
> I don't have a problem with this change, but the maintainer probably should
> have been Cc'd. Greg, does this change look ok to you? Note that it's
> already been committed to the upstream tree, so if it's a bad change we'll
> need to have the cset excluded or something.
I think it's safe enough. Since interrupts are off at this point,
I don't think the order of the two functions actually matters (i.e.
we couldn't have received a signal until the call to
spin_unlock_irqrestore() anyway).
Thanks - Greg
On Mon, 14 Mar 2005, Jesse Barnes wrote:
> On Monday, March 14, 2005 9:54 am, Nishanth Aravamudan wrote:
> > On Mon, Mar 14, 2005 at 09:45:44AM -0800, Jesse Barnes wrote:
> > > On Sunday, March 6, 2005 2:36 pm, [email protected] wrote:
> > > > Any comments would be, as always, appreciated.
> > >
> > > I don't have a problem with this change, but the maintainer probably
> > > should have been Cc'd. Greg, does this change look ok to you? Note that
> > > it's already been committed to the upstream tree, so if it's a bad change
> > > we'll need to have the cset excluded or something.
> >
> > Thanks for the feedback. I don't see a maintainer entry for Greg
> > anywhere in the snsc.{c,h} files or MAINTAINERS. Could someone throw a
> > patch upstream so that whomever should be contacted for this driver will
> > be in the future?
>
> Good point :) I assumed there was a MODULE_AUTHOR entry. Here's a patch to
> get Greg more spam^W^W^W^Wadd the relevant info. Look ok to you Greg?
Looks good. Apologies to Nish for dropping my
apparently-orphaned code on his doorstep. :-)
Thanks - Greg