* [PATCH] n_gsm.c: add tx_lock in gsm_send
@ 2012-12-19 5:16 Xiao Jin
2012-12-20 13:08 ` Alan Cox
0 siblings, 1 reply; 6+ messages in thread
From: Xiao Jin @ 2012-12-19 5:16 UTC (permalink / raw)
To: gregkh, jslaby, linux-kernel, akpm, mingo, a.p.zijlstra, rusty,
william.douglas, sboyd
Cc: vincentx.pillet
From: xiaojin <jin.xiao@intel.com>
Date: Wed, 19 Dec 2012 11:53:43 +0800
Subject: [PATCH] n_gsm.c: add tx_lock in gsm_send
All the call to gsm->output should be in the tx_lock,
that could avoid potential race from MUX level. But
we have no tx_lock in gsm_send.
This patch is to add tx_lock in gsm_send.
Signed-off-by: xiaojin <jin.xiao@intel.com>
---
drivers/tty/n_gsm.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index dcc0430..4572117 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -573,6 +573,7 @@ static void gsm_send(struct gsm_mux *gsm, int addr,
int cr, int control)
int len;
u8 cbuf[10];
u8 ibuf[3];
+ unsigned long flags;
switch (gsm->encoding) {
case 0:
@@ -602,7 +603,9 @@ static void gsm_send(struct gsm_mux *gsm, int addr,
int cr, int control)
WARN_ON(1);
return;
}
+ spin_lock_irqsave(&gsm->tx_lock, flags);
gsm->output(gsm, cbuf, len);
+ spin_unlock_irqrestore(&gsm->tx_lock, flags);
gsm_print_packet("-->", addr, cr, control, NULL, 0);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] n_gsm.c: add tx_lock in gsm_send
2012-12-19 5:16 [PATCH] n_gsm.c: add tx_lock in gsm_send Xiao Jin
@ 2012-12-20 13:08 ` Alan Cox
0 siblings, 0 replies; 6+ messages in thread
From: Alan Cox @ 2012-12-20 13:08 UTC (permalink / raw)
To: Xiao Jin
Cc: gregkh, jslaby, linux-kernel, akpm, mingo, a.p.zijlstra, rusty,
william.douglas, sboyd, vincentx.pillet
On Wed, 19 Dec 2012 13:16:10 +0800
Xiao Jin <jin.xiao@intel.com> wrote:
> From: xiaojin <jin.xiao@intel.com>
> Date: Wed, 19 Dec 2012 11:53:43 +0800
> Subject: [PATCH] n_gsm.c: add tx_lock in gsm_send
>
> All the call to gsm->output should be in the tx_lock,
> that could avoid potential race from MUX level. But
> we have no tx_lock in gsm_send.
>
> This patch is to add tx_lock in gsm_send.
>
> Signed-off-by: xiaojin <jin.xiao@intel.com>
Acked-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] n_gsm.c: add tx_lock in gsm_send
@ 2012-12-20 0:38 Xiao Jin
2012-12-20 11:44 ` Alan Cox
0 siblings, 1 reply; 6+ messages in thread
From: Xiao Jin @ 2012-12-20 0:38 UTC (permalink / raw)
To: akpm, linux-kernel, mingo, a.p.zijlstra, rusty, william.douglas,
sboyd, gregkh, jslaby
Cc: vincentx.pillet
From: xiaojin <jin.xiao@intel.com>
Date: Wed, 19 Dec 2012 11:53:43 +0800
Subject: [PATCH] n_gsm.c: add tx_lock in gsm_send
All the call to gsm->output should be in the tx_lock,
that could avoid potential race from MUX level. But
we have no tx_lock in gsm_send.
This patch is to add tx_lock in gsm_send.
Signed-off-by: xiaojin <jin.xiao@intel.com>
---
drivers/tty/n_gsm.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index dcc0430..4572117 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -573,6 +573,7 @@ static void gsm_send(struct gsm_mux *gsm, int addr,
int cr, int control)
int len;
u8 cbuf[10];
u8 ibuf[3];
+ unsigned long flags;
switch (gsm->encoding) {
case 0:
@@ -602,7 +603,9 @@ static void gsm_send(struct gsm_mux *gsm, int addr,
int cr, int control)
WARN_ON(1);
return;
}
+ spin_lock_irqsave(&gsm->tx_lock, flags);
gsm->output(gsm, cbuf, len);
+ spin_unlock_irqrestore(&gsm->tx_lock, flags);
gsm_print_packet("-->", addr, cr, control, NULL, 0);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] n_gsm.c: add tx_lock in gsm_send
2012-12-20 0:38 Xiao Jin
@ 2012-12-20 11:44 ` Alan Cox
2012-12-20 12:13 ` Xiao, Jin
0 siblings, 1 reply; 6+ messages in thread
From: Alan Cox @ 2012-12-20 11:44 UTC (permalink / raw)
To: Xiao Jin
Cc: akpm, linux-kernel, mingo, a.p.zijlstra, rusty, william.douglas,
sboyd, gregkh, jslaby, vincentx.pillet
On Thu, 20 Dec 2012 08:38:40 +0800
Xiao Jin <jin.xiao@intel.com> wrote:
> From: xiaojin <jin.xiao@intel.com>
> Date: Wed, 19 Dec 2012 11:53:43 +0800
> Subject: [PATCH] n_gsm.c: add tx_lock in gsm_send
>
> All the call to gsm->output should be in the tx_lock,
> that could avoid potential race from MUX level. But
> we have no tx_lock in gsm_send.
>
> This patch is to add tx_lock in gsm_send.
gsm->output calls the transmit method of the underlying tty driver. We
can't do that with interrupts off as some drivers expect to be able to
sleep in their output paths.
Alan
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] n_gsm.c: add tx_lock in gsm_send
2012-12-20 11:44 ` Alan Cox
@ 2012-12-20 12:13 ` Xiao, Jin
2012-12-20 13:06 ` Alan Cox
0 siblings, 1 reply; 6+ messages in thread
From: Xiao, Jin @ 2012-12-20 12:13 UTC (permalink / raw)
To: 'Alan Cox'
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
mingo@elte.hu, a.p.zijlstra@chello.nl, rusty@rustcorp.com.au,
Douglas, William, sboyd@codeaurora.org,
gregkh@linuxfoundation.org, jslaby@suse.cz, Pillet, VincentX
Alan,
Thanks. But the comment makes me confused. As we see, gsm->output is called by gsm_data_kick too, and it's in the tx_lock...
Best regards,
Jin Xiao
> From: xiaojin <jin.xiao@intel.com>
> Date: Wed, 19 Dec 2012 11:53:43 +0800
> Subject: [PATCH] n_gsm.c: add tx_lock in gsm_send
>
> All the call to gsm->output should be in the tx_lock, that could avoid
> potential race from MUX level. But we have no tx_lock in gsm_send.
>
> This patch is to add tx_lock in gsm_send.
gsm->output calls the transmit method of the underlying tty driver. We
can't do that with interrupts off as some drivers expect to be able to sleep in their output paths.
Alan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] n_gsm.c: add tx_lock in gsm_send
2012-12-20 12:13 ` Xiao, Jin
@ 2012-12-20 13:06 ` Alan Cox
0 siblings, 0 replies; 6+ messages in thread
From: Alan Cox @ 2012-12-20 13:06 UTC (permalink / raw)
To: Xiao, Jin
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
mingo@elte.hu, a.p.zijlstra@chello.nl, rusty@rustcorp.com.au,
Douglas, William, sboyd@codeaurora.org,
gregkh@linuxfoundation.org, jslaby@suse.cz, Pillet, VincentX
On Thu, 20 Dec 2012 12:13:07 +0000
"Xiao, Jin" <jin.xiao@intel.com> wrote:
> Alan,
>
> Thanks. But the comment makes me confused. As we see, gsm->output is called by gsm_data_kick too, and it's in the tx_lock...
That would be a bug too or I guess we could finally give in on trying to
keep tty write paths not using spin_lock_irq and fix any oddments that
blow up. That might actually be the right thing to do as its caused
problems in other places too, and the USB tty drivers are now well
behaved.
Your patch doesn't make it any worse however so I agree it might as well
go in.
Alan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-12-20 13:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-19 5:16 [PATCH] n_gsm.c: add tx_lock in gsm_send Xiao Jin
2012-12-20 13:08 ` Alan Cox
-- strict thread matches above, loose matches on Subject: below --
2012-12-20 0:38 Xiao Jin
2012-12-20 11:44 ` Alan Cox
2012-12-20 12:13 ` Xiao, Jin
2012-12-20 13:06 ` Alan Cox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox