* [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context"
@ 2022-11-23 18:19 Fedor Pchelkin
2022-11-28 14:20 ` Pavel Machek
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Fedor Pchelkin @ 2022-11-23 18:19 UTC (permalink / raw)
To: Greg Kroah-Hartman, stable
Cc: Fedor Pchelkin, linux-kernel, Daniel Starke, jirislaby,
Alexey Khoroshilov, lvc-project
From: Fedor Pchelkin <pchelkin@ispras.ru>
[ Upstream commit acdab4cb4ba7e5f94d2b422ebd7bf4bf68178fb2 ]
This reverts commit eb75efdec8dd0f01ac85c88feafa6e63b34a2521.
The above commit is reverted as the usage of tx_mutex seems not to solve
the problem described in eb75efdec8dd ("tty: n_gsm: avoid call of sleeping
functions from atomic context") and just moves the bug to another place.
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Reviewed-by: Daniel Starke <daniel.starke@siemens.com>
Link: https://lore.kernel.org/r/20221008110221.13645-2-pchelkin@ispras.ru
---
drivers/tty/n_gsm.c | 39 +++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c91a3004931f..c2212f52a603 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -235,7 +235,7 @@ struct gsm_mux {
int old_c_iflag; /* termios c_iflag value before attach */
bool constipated; /* Asked by remote to shut up */
- struct mutex tx_mutex;
+ spinlock_t tx_lock;
unsigned int tx_bytes; /* TX data outstanding */
#define TX_THRESH_HI 8192
#define TX_THRESH_LO 2048
@@ -820,14 +820,15 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg)
*
* Add data to the transmit queue and try and get stuff moving
* out of the mux tty if not already doing so. Take the
- * the gsm tx mutex and dlci lock.
+ * the gsm tx lock and dlci lock.
*/
static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg)
{
- mutex_lock(&dlci->gsm->tx_mutex);
+ unsigned long flags;
+ spin_lock_irqsave(&dlci->gsm->tx_lock, flags);
__gsm_data_queue(dlci, msg);
- mutex_unlock(&dlci->gsm->tx_mutex);
+ spin_unlock_irqrestore(&dlci->gsm->tx_lock, flags);
}
/**
@@ -839,7 +840,7 @@ static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg)
* is data. Keep to the MRU of the mux. This path handles the usual tty
* interface which is a byte stream with optional modem data.
*
- * Caller must hold the tx_mutex of the mux.
+ * Caller must hold the tx_lock of the mux.
*/
static int gsm_dlci_data_output(struct gsm_mux *gsm, struct gsm_dlci *dlci)
@@ -902,7 +903,7 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct gsm_dlci *dlci)
* is data. Keep to the MRU of the mux. This path handles framed data
* queued as skbuffs to the DLCI.
*
- * Caller must hold the tx_mutex of the mux.
+ * Caller must hold the tx_lock of the mux.
*/
static int gsm_dlci_data_output_framed(struct gsm_mux *gsm,
@@ -918,7 +919,7 @@ static int gsm_dlci_data_output_framed(struct gsm_mux *gsm,
if (dlci->adaption == 4)
overhead = 1;
- /* dlci->skb is locked by tx_mutex */
+ /* dlci->skb is locked by tx_lock */
if (dlci->skb == NULL) {
dlci->skb = skb_dequeue_tail(&dlci->skb_list);
if (dlci->skb == NULL)
@@ -1018,12 +1019,13 @@ static void gsm_dlci_data_sweep(struct gsm_mux *gsm)
static void gsm_dlci_data_kick(struct gsm_dlci *dlci)
{
+ unsigned long flags;
int sweep;
if (dlci->constipated)
return;
- mutex_lock(&dlci->gsm->tx_mutex);
+ spin_lock_irqsave(&dlci->gsm->tx_lock, flags);
/* If we have nothing running then we need to fire up */
sweep = (dlci->gsm->tx_bytes < TX_THRESH_LO);
if (dlci->gsm->tx_bytes == 0) {
@@ -1034,7 +1036,7 @@ static void gsm_dlci_data_kick(struct gsm_dlci *dlci)
}
if (sweep)
gsm_dlci_data_sweep(dlci->gsm);
- mutex_unlock(&dlci->gsm->tx_mutex);
+ spin_unlock_irqrestore(&dlci->gsm->tx_lock, flags);
}
/*
@@ -1256,6 +1258,7 @@ static void gsm_control_message(struct gsm_mux *gsm, unsigned int command,
const u8 *data, int clen)
{
u8 buf[1];
+ unsigned long flags;
switch (command) {
case CMD_CLD: {
@@ -1277,9 +1280,9 @@ static void gsm_control_message(struct gsm_mux *gsm, unsigned int command,
gsm->constipated = false;
gsm_control_reply(gsm, CMD_FCON, NULL, 0);
/* Kick the link in case it is idling */
- mutex_lock(&gsm->tx_mutex);
+ spin_lock_irqsave(&gsm->tx_lock, flags);
gsm_data_kick(gsm, NULL);
- mutex_unlock(&gsm->tx_mutex);
+ spin_unlock_irqrestore(&gsm->tx_lock, flags);
break;
case CMD_FCOFF:
/* Modem wants us to STFU */
@@ -2225,7 +2228,6 @@ static void gsm_free_mux(struct gsm_mux *gsm)
break;
}
}
- mutex_destroy(&gsm->tx_mutex);
mutex_destroy(&gsm->mutex);
kfree(gsm->txframe);
kfree(gsm->buf);
@@ -2297,12 +2299,12 @@ static struct gsm_mux *gsm_alloc_mux(void)
}
spin_lock_init(&gsm->lock);
mutex_init(&gsm->mutex);
- mutex_init(&gsm->tx_mutex);
kref_init(&gsm->ref);
INIT_LIST_HEAD(&gsm->tx_list);
timer_setup(&gsm->t2_timer, gsm_control_retransmit, 0);
init_waitqueue_head(&gsm->event);
spin_lock_init(&gsm->control_lock);
+ spin_lock_init(&gsm->tx_lock);
gsm->t1 = T1;
gsm->t2 = T2;
@@ -2327,7 +2329,6 @@ static struct gsm_mux *gsm_alloc_mux(void)
}
spin_unlock(&gsm_mux_lock);
if (i == MAX_MUX) {
- mutex_destroy(&gsm->tx_mutex);
mutex_destroy(&gsm->mutex);
kfree(gsm->txframe);
kfree(gsm->buf);
@@ -2652,15 +2653,16 @@ static int gsmld_open(struct tty_struct *tty)
static void gsmld_write_wakeup(struct tty_struct *tty)
{
struct gsm_mux *gsm = tty->disc_data;
+ unsigned long flags;
/* Queue poll */
clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
- mutex_lock(&gsm->tx_mutex);
+ spin_lock_irqsave(&gsm->tx_lock, flags);
gsm_data_kick(gsm, NULL);
if (gsm->tx_bytes < TX_THRESH_LO) {
gsm_dlci_data_sweep(gsm);
}
- mutex_unlock(&gsm->tx_mutex);
+ spin_unlock_irqrestore(&gsm->tx_lock, flags);
}
/**
@@ -2703,6 +2705,7 @@ static ssize_t gsmld_write(struct tty_struct *tty, struct file *file,
const unsigned char *buf, size_t nr)
{
struct gsm_mux *gsm = tty->disc_data;
+ unsigned long flags;
int space;
int ret;
@@ -2710,13 +2713,13 @@ static ssize_t gsmld_write(struct tty_struct *tty, struct file *file,
return -ENODEV;
ret = -ENOBUFS;
- mutex_lock(&gsm->tx_mutex);
+ spin_lock_irqsave(&gsm->tx_lock, flags);
space = tty_write_room(tty);
if (space >= nr)
ret = tty->ops->write(tty, buf, nr);
else
set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
- mutex_unlock(&gsm->tx_mutex);
+ spin_unlock_irqrestore(&gsm->tx_lock, flags);
return ret;
}
--
2.38.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" 2022-11-23 18:19 [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" Fedor Pchelkin @ 2022-11-28 14:20 ` Pavel Machek 2022-12-03 13:35 ` Greg Kroah-Hartman 2022-12-03 13:38 ` Greg Kroah-Hartman 2 siblings, 0 replies; 11+ messages in thread From: Pavel Machek @ 2022-11-28 14:20 UTC (permalink / raw) To: Fedor Pchelkin Cc: Greg Kroah-Hartman, stable, linux-kernel, Daniel Starke, jirislaby, Alexey Khoroshilov, lvc-project [-- Attachment #1: Type: text/plain, Size: 841 bytes --] On Wed 2022-11-23 21:19:16, Fedor Pchelkin wrote: > From: Fedor Pchelkin <pchelkin@ispras.ru> > > [ Upstream commit acdab4cb4ba7e5f94d2b422ebd7bf4bf68178fb2 ] > > This reverts commit eb75efdec8dd0f01ac85c88feafa6e63b34a2521. > > The above commit is reverted as the usage of tx_mutex seems not to solve > the problem described in eb75efdec8dd ("tty: n_gsm: avoid call of sleeping > functions from atomic context") and just moves the bug to another place. > > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> > Reviewed-by: Daniel Starke <daniel.starke@siemens.com> > Link: https://lore.kernel.org/r/20221008110221.13645-2-pchelkin@ispras.ru Reviewed-by: Pavel Machek <pavel@denx.de> -- People of Russia, stop Putin before his war on Ukraine escalates. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" 2022-11-23 18:19 [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" Fedor Pchelkin 2022-11-28 14:20 ` Pavel Machek @ 2022-12-03 13:35 ` Greg Kroah-Hartman 2022-12-03 22:35 ` [PATCH 5.19 0/2] tty: n_gsm: revert tx_mutex usage Fedor Pchelkin 2022-12-04 15:57 ` [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" Greg Kroah-Hartman 2022-12-03 13:38 ` Greg Kroah-Hartman 2 siblings, 2 replies; 11+ messages in thread From: Greg Kroah-Hartman @ 2022-12-03 13:35 UTC (permalink / raw) To: Fedor Pchelkin Cc: stable, linux-kernel, Daniel Starke, jirislaby, Alexey Khoroshilov, lvc-project On Wed, Nov 23, 2022 at 09:19:16PM +0300, Fedor Pchelkin wrote: > From: Fedor Pchelkin <pchelkin@ispras.ru> > > [ Upstream commit acdab4cb4ba7e5f94d2b422ebd7bf4bf68178fb2 ] > > This reverts commit eb75efdec8dd0f01ac85c88feafa6e63b34a2521. What about for 5.15? This should also be needed there, right? thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5.19 0/2] tty: n_gsm: revert tx_mutex usage 2022-12-03 13:35 ` Greg Kroah-Hartman @ 2022-12-03 22:35 ` Fedor Pchelkin 2022-12-03 22:35 ` [PATCH 5.19 1/2] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" Fedor Pchelkin ` (2 more replies) 2022-12-04 15:57 ` [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" Greg Kroah-Hartman 1 sibling, 3 replies; 11+ messages in thread From: Fedor Pchelkin @ 2022-12-03 22:35 UTC (permalink / raw) To: Greg Kroah-Hartman, stable Cc: Fedor Pchelkin, linux-kernel, Daniel Starke, jirislaby, Alexey Khoroshilov, lvc-project, Pavel Machek Thanks for notice, n_gsm tx_mutex patches just were added to 5.19, not 5.15, so we need to revert them. I'm sorry for the inconvenience with 5.10 revert patch, fixed that in the corresponding thread. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5.19 1/2] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" 2022-12-03 22:35 ` [PATCH 5.19 0/2] tty: n_gsm: revert tx_mutex usage Fedor Pchelkin @ 2022-12-03 22:35 ` Fedor Pchelkin 2022-12-03 22:35 ` [PATCH 5.19 2/2] Revert "tty: n_gsm: replace kicktimer with delayed_work" Fedor Pchelkin 2022-12-04 15:56 ` [PATCH 5.19 0/2] tty: n_gsm: revert tx_mutex usage Greg Kroah-Hartman 2 siblings, 0 replies; 11+ messages in thread From: Fedor Pchelkin @ 2022-12-03 22:35 UTC (permalink / raw) To: Greg Kroah-Hartman, stable Cc: Fedor Pchelkin, linux-kernel, Daniel Starke, jirislaby, Alexey Khoroshilov, lvc-project, Pavel Machek From: Fedor Pchelkin <pchelkin@ispras.ru> [ Upstream commit acdab4cb4ba7e5f94d2b422ebd7bf4bf68178fb2 ] This reverts commit 132331c1f605eb5911795a6b9115114575594d0a. The above commit is reverted as the usage of tx_mutex seems not to solve the problem described in 132331c1f605 ("tty: n_gsm: avoid call of sleeping functions from atomic context") and just moves the bug to another place. Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> Reviewed-by: Daniel Starke <daniel.starke@siemens.com> Link: https://lore.kernel.org/r/20221008110221.13645-2-pchelkin@ispras.ru Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/tty/n_gsm.c | 53 +++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 01c112e2e214..e23225aff5d9 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -248,7 +248,7 @@ struct gsm_mux { bool constipated; /* Asked by remote to shut up */ bool has_devices; /* Devices were registered */ - struct mutex tx_mutex; + spinlock_t tx_lock; unsigned int tx_bytes; /* TX data outstanding */ #define TX_THRESH_HI 8192 #define TX_THRESH_LO 2048 @@ -680,6 +680,7 @@ static int gsm_send(struct gsm_mux *gsm, int addr, int cr, int control) struct gsm_msg *msg; u8 *dp; int ocr; + unsigned long flags; msg = gsm_data_alloc(gsm, addr, 0, control); if (!msg) @@ -701,10 +702,10 @@ static int gsm_send(struct gsm_mux *gsm, int addr, int cr, int control) gsm_print_packet("Q->", addr, cr, control, NULL, 0); - mutex_lock(&gsm->tx_mutex); + spin_lock_irqsave(&gsm->tx_lock, flags); list_add_tail(&msg->list, &gsm->tx_ctrl_list); gsm->tx_bytes += msg->len; - mutex_unlock(&gsm->tx_mutex); + spin_unlock_irqrestore(&gsm->tx_lock, flags); gsmld_write_trigger(gsm); return 0; @@ -729,7 +730,7 @@ static void gsm_dlci_clear_queues(struct gsm_mux *gsm, struct gsm_dlci *dlci) spin_unlock_irqrestore(&dlci->lock, flags); /* Clear data packets in MUX write queue */ - mutex_lock(&gsm->tx_mutex); + spin_lock_irqsave(&gsm->tx_lock, flags); list_for_each_entry_safe(msg, nmsg, &gsm->tx_data_list, list) { if (msg->addr != addr) continue; @@ -737,7 +738,7 @@ static void gsm_dlci_clear_queues(struct gsm_mux *gsm, struct gsm_dlci *dlci) list_del(&msg->list); kfree(msg); } - mutex_unlock(&gsm->tx_mutex); + spin_unlock_irqrestore(&gsm->tx_lock, flags); } /** @@ -1023,9 +1024,10 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg) static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg) { - mutex_lock(&dlci->gsm->tx_mutex); + unsigned long flags; + spin_lock_irqsave(&dlci->gsm->tx_lock, flags); __gsm_data_queue(dlci, msg); - mutex_unlock(&dlci->gsm->tx_mutex); + spin_unlock_irqrestore(&dlci->gsm->tx_lock, flags); } /** @@ -1037,7 +1039,7 @@ static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg) * is data. Keep to the MRU of the mux. This path handles the usual tty * interface which is a byte stream with optional modem data. * - * Caller must hold the tx_mutex of the mux. + * Caller must hold the tx_lock of the mux. */ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct gsm_dlci *dlci) @@ -1097,7 +1099,7 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct gsm_dlci *dlci) * is data. Keep to the MRU of the mux. This path handles framed data * queued as skbuffs to the DLCI. * - * Caller must hold the tx_mutex of the mux. + * Caller must hold the tx_lock of the mux. */ static int gsm_dlci_data_output_framed(struct gsm_mux *gsm, @@ -1113,7 +1115,7 @@ static int gsm_dlci_data_output_framed(struct gsm_mux *gsm, if (dlci->adaption == 4) overhead = 1; - /* dlci->skb is locked by tx_mutex */ + /* dlci->skb is locked by tx_lock */ if (dlci->skb == NULL) { dlci->skb = skb_dequeue_tail(&dlci->skb_list); if (dlci->skb == NULL) @@ -1167,7 +1169,7 @@ static int gsm_dlci_data_output_framed(struct gsm_mux *gsm, * Push an empty frame in to the transmit queue to update the modem status * bits and to transmit an optional break. * - * Caller must hold the tx_mutex of the mux. + * Caller must hold the tx_lock of the mux. */ static int gsm_dlci_modem_output(struct gsm_mux *gsm, struct gsm_dlci *dlci, @@ -1281,12 +1283,13 @@ static int gsm_dlci_data_sweep(struct gsm_mux *gsm) static void gsm_dlci_data_kick(struct gsm_dlci *dlci) { + unsigned long flags; int sweep; if (dlci->constipated) return; - mutex_lock(&dlci->gsm->tx_mutex); + spin_lock_irqsave(&dlci->gsm->tx_lock, flags); /* If we have nothing running then we need to fire up */ sweep = (dlci->gsm->tx_bytes < TX_THRESH_LO); if (dlci->gsm->tx_bytes == 0) { @@ -1297,7 +1300,7 @@ static void gsm_dlci_data_kick(struct gsm_dlci *dlci) } if (sweep) gsm_dlci_data_sweep(dlci->gsm); - mutex_unlock(&dlci->gsm->tx_mutex); + spin_unlock_irqrestore(&dlci->gsm->tx_lock, flags); } /* @@ -1991,13 +1994,14 @@ static void gsm_dlci_command(struct gsm_dlci *dlci, const u8 *data, int len) static void gsm_kick_timeout(struct work_struct *work) { struct gsm_mux *gsm = container_of(work, struct gsm_mux, kick_timeout.work); + unsigned long flags; int sent = 0; - mutex_lock(&gsm->tx_mutex); + spin_lock_irqsave(&gsm->tx_lock, flags); /* If we have nothing running then we need to fire up */ if (gsm->tx_bytes < TX_THRESH_LO) sent = gsm_dlci_data_sweep(gsm); - mutex_unlock(&gsm->tx_mutex); + spin_unlock_irqrestore(&gsm->tx_lock, flags); if (sent && debug & 4) pr_info("%s TX queue stalled\n", __func__); @@ -2527,7 +2531,6 @@ static void gsm_free_mux(struct gsm_mux *gsm) break; } } - mutex_destroy(&gsm->tx_mutex); mutex_destroy(&gsm->mutex); kfree(gsm->txframe); kfree(gsm->buf); @@ -2599,7 +2602,6 @@ static struct gsm_mux *gsm_alloc_mux(void) } spin_lock_init(&gsm->lock); mutex_init(&gsm->mutex); - mutex_init(&gsm->tx_mutex); kref_init(&gsm->ref); INIT_LIST_HEAD(&gsm->tx_ctrl_list); INIT_LIST_HEAD(&gsm->tx_data_list); @@ -2608,6 +2610,7 @@ static struct gsm_mux *gsm_alloc_mux(void) INIT_WORK(&gsm->tx_work, gsmld_write_task); init_waitqueue_head(&gsm->event); spin_lock_init(&gsm->control_lock); + spin_lock_init(&gsm->tx_lock); gsm->t1 = T1; gsm->t2 = T2; @@ -2632,7 +2635,6 @@ static struct gsm_mux *gsm_alloc_mux(void) } spin_unlock(&gsm_mux_lock); if (i == MAX_MUX) { - mutex_destroy(&gsm->tx_mutex); mutex_destroy(&gsm->mutex); kfree(gsm->txframe); kfree(gsm->buf); @@ -2788,16 +2790,17 @@ static void gsmld_write_trigger(struct gsm_mux *gsm) static void gsmld_write_task(struct work_struct *work) { struct gsm_mux *gsm = container_of(work, struct gsm_mux, tx_work); + unsigned long flags; int i, ret; /* All outstanding control channel and control messages and one data * frame is sent. */ ret = -ENODEV; - mutex_lock(&gsm->tx_mutex); + spin_lock_irqsave(&gsm->tx_lock, flags); if (gsm->tty) ret = gsm_data_kick(gsm); - mutex_unlock(&gsm->tx_mutex); + spin_unlock_irqrestore(&gsm->tx_lock, flags); if (ret >= 0) for (i = 0; i < NUM_DLCI; i++) @@ -3005,6 +3008,7 @@ static ssize_t gsmld_write(struct tty_struct *tty, struct file *file, const unsigned char *buf, size_t nr) { struct gsm_mux *gsm = tty->disc_data; + unsigned long flags; int space; int ret; @@ -3012,13 +3016,13 @@ static ssize_t gsmld_write(struct tty_struct *tty, struct file *file, return -ENODEV; ret = -ENOBUFS; - mutex_lock(&gsm->tx_mutex); + spin_lock_irqsave(&gsm->tx_lock, flags); space = tty_write_room(tty); if (space >= nr) ret = tty->ops->write(tty, buf, nr); else set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags); - mutex_unlock(&gsm->tx_mutex); + spin_unlock_irqrestore(&gsm->tx_lock, flags); return ret; } @@ -3315,13 +3319,14 @@ static struct tty_ldisc_ops tty_ldisc_packet = { static void gsm_modem_upd_via_data(struct gsm_dlci *dlci, u8 brk) { struct gsm_mux *gsm = dlci->gsm; + unsigned long flags; if (dlci->state != DLCI_OPEN || dlci->adaption != 2) return; - mutex_lock(&gsm->tx_mutex); + spin_lock_irqsave(&gsm->tx_lock, flags); gsm_dlci_modem_output(gsm, dlci, brk); - mutex_unlock(&gsm->tx_mutex); + spin_unlock_irqrestore(&gsm->tx_lock, flags); } /** -- 2.38.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5.19 2/2] Revert "tty: n_gsm: replace kicktimer with delayed_work" 2022-12-03 22:35 ` [PATCH 5.19 0/2] tty: n_gsm: revert tx_mutex usage Fedor Pchelkin 2022-12-03 22:35 ` [PATCH 5.19 1/2] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" Fedor Pchelkin @ 2022-12-03 22:35 ` Fedor Pchelkin 2022-12-04 15:56 ` [PATCH 5.19 0/2] tty: n_gsm: revert tx_mutex usage Greg Kroah-Hartman 2 siblings, 0 replies; 11+ messages in thread From: Fedor Pchelkin @ 2022-12-03 22:35 UTC (permalink / raw) To: Greg Kroah-Hartman, stable Cc: Fedor Pchelkin, linux-kernel, Daniel Starke, jirislaby, Alexey Khoroshilov, lvc-project, Pavel Machek From: Fedor Pchelkin <pchelkin@ispras.ru> [ Upstream commit 15743ae50e04aa907131e3ae8d66e9a2964ea232 ] This reverts commit 2af54fe4f713d5f29e1520d7780112ff9b6121be. The above commit is reverted as it was a prerequisite for tx_mutex introduction and tx_mutex has been removed as it does not correctly work in order to protect tx data. Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> Reviewed-by: Daniel Starke <daniel.starke@siemens.com> Link: https://lore.kernel.org/r/20221008110221.13645-3-pchelkin@ispras.ru Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/tty/n_gsm.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index e23225aff5d9..d6598ca3640f 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -256,7 +256,7 @@ struct gsm_mux { struct list_head tx_data_list; /* Pending data packets */ /* Control messages */ - struct delayed_work kick_timeout; /* Kick TX queuing on timeout */ + struct timer_list kick_timer; /* Kick TX queuing on timeout */ struct timer_list t2_timer; /* Retransmit timer for commands */ int cretries; /* Command retry counter */ struct gsm_control *pending_cmd;/* Our current pending command */ @@ -1009,7 +1009,7 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg) gsm->tx_bytes += msg->len; gsmld_write_trigger(gsm); - schedule_delayed_work(&gsm->kick_timeout, 10 * gsm->t1 * HZ / 100); + mod_timer(&gsm->kick_timer, jiffies + 10 * gsm->t1 * HZ / 100); } /** @@ -1984,16 +1984,16 @@ static void gsm_dlci_command(struct gsm_dlci *dlci, const u8 *data, int len) } /** - * gsm_kick_timeout - transmit if possible - * @work: work contained in our gsm object + * gsm_kick_timer - transmit if possible + * @t: timer contained in our gsm object * * Transmit data from DLCIs if the queue is empty. We can't rely on * a tty wakeup except when we filled the pipe so we need to fire off * new data ourselves in other cases. */ -static void gsm_kick_timeout(struct work_struct *work) +static void gsm_kick_timer(struct timer_list *t) { - struct gsm_mux *gsm = container_of(work, struct gsm_mux, kick_timeout.work); + struct gsm_mux *gsm = from_timer(gsm, t, kick_timer); unsigned long flags; int sent = 0; @@ -2458,7 +2458,7 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc) } /* Finish outstanding timers, making sure they are done */ - cancel_delayed_work_sync(&gsm->kick_timeout); + del_timer_sync(&gsm->kick_timer); del_timer_sync(&gsm->t2_timer); /* Finish writing to ldisc */ @@ -2605,7 +2605,7 @@ static struct gsm_mux *gsm_alloc_mux(void) kref_init(&gsm->ref); INIT_LIST_HEAD(&gsm->tx_ctrl_list); INIT_LIST_HEAD(&gsm->tx_data_list); - INIT_DELAYED_WORK(&gsm->kick_timeout, gsm_kick_timeout); + timer_setup(&gsm->kick_timer, gsm_kick_timer, 0); timer_setup(&gsm->t2_timer, gsm_control_retransmit, 0); INIT_WORK(&gsm->tx_work, gsmld_write_task); init_waitqueue_head(&gsm->event); -- 2.38.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 5.19 0/2] tty: n_gsm: revert tx_mutex usage 2022-12-03 22:35 ` [PATCH 5.19 0/2] tty: n_gsm: revert tx_mutex usage Fedor Pchelkin 2022-12-03 22:35 ` [PATCH 5.19 1/2] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" Fedor Pchelkin 2022-12-03 22:35 ` [PATCH 5.19 2/2] Revert "tty: n_gsm: replace kicktimer with delayed_work" Fedor Pchelkin @ 2022-12-04 15:56 ` Greg Kroah-Hartman 2 siblings, 0 replies; 11+ messages in thread From: Greg Kroah-Hartman @ 2022-12-04 15:56 UTC (permalink / raw) To: Fedor Pchelkin Cc: stable, linux-kernel, Daniel Starke, jirislaby, Alexey Khoroshilov, lvc-project, Pavel Machek On Sun, Dec 04, 2022 at 01:35:24AM +0300, Fedor Pchelkin wrote: > Thanks for notice, n_gsm tx_mutex patches just were added to 5.19, not > 5.15, so we need to revert them. 5.19 is long end-of-life, there is nothing that we can do about that at all. greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" 2022-12-03 13:35 ` Greg Kroah-Hartman 2022-12-03 22:35 ` [PATCH 5.19 0/2] tty: n_gsm: revert tx_mutex usage Fedor Pchelkin @ 2022-12-04 15:57 ` Greg Kroah-Hartman 1 sibling, 0 replies; 11+ messages in thread From: Greg Kroah-Hartman @ 2022-12-04 15:57 UTC (permalink / raw) To: Fedor Pchelkin Cc: stable, linux-kernel, Daniel Starke, jirislaby, Alexey Khoroshilov, lvc-project On Sat, Dec 03, 2022 at 02:35:09PM +0100, Greg Kroah-Hartman wrote: > On Wed, Nov 23, 2022 at 09:19:16PM +0300, Fedor Pchelkin wrote: > > From: Fedor Pchelkin <pchelkin@ispras.ru> > > > > [ Upstream commit acdab4cb4ba7e5f94d2b422ebd7bf4bf68178fb2 ] > > > > This reverts commit eb75efdec8dd0f01ac85c88feafa6e63b34a2521. > > What about for 5.15? This should also be needed there, right? Odd, it never got applied to 5.15, so nevermind about that, sorry for the noise. greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" 2022-11-23 18:19 [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" Fedor Pchelkin 2022-11-28 14:20 ` Pavel Machek 2022-12-03 13:35 ` Greg Kroah-Hartman @ 2022-12-03 13:38 ` Greg Kroah-Hartman 2022-12-03 21:55 ` [PATCH v2 " Fedor Pchelkin 2 siblings, 1 reply; 11+ messages in thread From: Greg Kroah-Hartman @ 2022-12-03 13:38 UTC (permalink / raw) To: Fedor Pchelkin Cc: stable, linux-kernel, Daniel Starke, jirislaby, Alexey Khoroshilov, lvc-project On Wed, Nov 23, 2022 at 09:19:16PM +0300, Fedor Pchelkin wrote: > From: Fedor Pchelkin <pchelkin@ispras.ru> > > [ Upstream commit acdab4cb4ba7e5f94d2b422ebd7bf4bf68178fb2 ] > > This reverts commit eb75efdec8dd0f01ac85c88feafa6e63b34a2521. > > The above commit is reverted as the usage of tx_mutex seems not to solve > the problem described in eb75efdec8dd ("tty: n_gsm: avoid call of sleeping > functions from atomic context") and just moves the bug to another place. > > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> > Reviewed-by: Daniel Starke <daniel.starke@siemens.com> > Link: https://lore.kernel.org/r/20221008110221.13645-2-pchelkin@ispras.ru > --- > drivers/tty/n_gsm.c | 39 +++++++++++++++++++++------------------ > 1 file changed, 21 insertions(+), 18 deletions(-) Also, what happened to my original signed-off-by here? thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" 2022-12-03 13:38 ` Greg Kroah-Hartman @ 2022-12-03 21:55 ` Fedor Pchelkin 2022-12-04 15:59 ` Greg Kroah-Hartman 0 siblings, 1 reply; 11+ messages in thread From: Fedor Pchelkin @ 2022-12-03 21:55 UTC (permalink / raw) To: Greg Kroah-Hartman, stable Cc: Fedor Pchelkin, linux-kernel, Daniel Starke, jirislaby, Alexey Khoroshilov, lvc-project, Pavel Machek From: Fedor Pchelkin <pchelkin@ispras.ru> [ Upstream commit acdab4cb4ba7e5f94d2b422ebd7bf4bf68178fb2 ] This reverts commit eb75efdec8dd0f01ac85c88feafa6e63b34a2521. The above commit is reverted as the usage of tx_mutex seems not to solve the problem described in eb75efdec8dd ("tty: n_gsm: avoid call of sleeping functions from atomic context") and just moves the bug to another place. Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> Reviewed-by: Daniel Starke <daniel.starke@siemens.com> Link: https://lore.kernel.org/r/20221008110221.13645-2-pchelkin@ispras.ru Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Reviewed-by: Pavel Machek <pavel@denx.de> --- v1->v2: added signed-off-by and reviewed-by drivers/tty/n_gsm.c | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index c91a3004931f..c2212f52a603 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -235,7 +235,7 @@ struct gsm_mux { int old_c_iflag; /* termios c_iflag value before attach */ bool constipated; /* Asked by remote to shut up */ - struct mutex tx_mutex; + spinlock_t tx_lock; unsigned int tx_bytes; /* TX data outstanding */ #define TX_THRESH_HI 8192 #define TX_THRESH_LO 2048 @@ -820,14 +820,15 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg) * * Add data to the transmit queue and try and get stuff moving * out of the mux tty if not already doing so. Take the - * the gsm tx mutex and dlci lock. + * the gsm tx lock and dlci lock. */ static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg) { - mutex_lock(&dlci->gsm->tx_mutex); + unsigned long flags; + spin_lock_irqsave(&dlci->gsm->tx_lock, flags); __gsm_data_queue(dlci, msg); - mutex_unlock(&dlci->gsm->tx_mutex); + spin_unlock_irqrestore(&dlci->gsm->tx_lock, flags); } /** @@ -839,7 +840,7 @@ static void gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg) * is data. Keep to the MRU of the mux. This path handles the usual tty * interface which is a byte stream with optional modem data. * - * Caller must hold the tx_mutex of the mux. + * Caller must hold the tx_lock of the mux. */ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct gsm_dlci *dlci) @@ -902,7 +903,7 @@ static int gsm_dlci_data_output(struct gsm_mux *gsm, struct gsm_dlci *dlci) * is data. Keep to the MRU of the mux. This path handles framed data * queued as skbuffs to the DLCI. * - * Caller must hold the tx_mutex of the mux. + * Caller must hold the tx_lock of the mux. */ static int gsm_dlci_data_output_framed(struct gsm_mux *gsm, @@ -918,7 +919,7 @@ static int gsm_dlci_data_output_framed(struct gsm_mux *gsm, if (dlci->adaption == 4) overhead = 1; - /* dlci->skb is locked by tx_mutex */ + /* dlci->skb is locked by tx_lock */ if (dlci->skb == NULL) { dlci->skb = skb_dequeue_tail(&dlci->skb_list); if (dlci->skb == NULL) @@ -1018,12 +1019,13 @@ static void gsm_dlci_data_sweep(struct gsm_mux *gsm) static void gsm_dlci_data_kick(struct gsm_dlci *dlci) { + unsigned long flags; int sweep; if (dlci->constipated) return; - mutex_lock(&dlci->gsm->tx_mutex); + spin_lock_irqsave(&dlci->gsm->tx_lock, flags); /* If we have nothing running then we need to fire up */ sweep = (dlci->gsm->tx_bytes < TX_THRESH_LO); if (dlci->gsm->tx_bytes == 0) { @@ -1034,7 +1036,7 @@ static void gsm_dlci_data_kick(struct gsm_dlci *dlci) } if (sweep) gsm_dlci_data_sweep(dlci->gsm); - mutex_unlock(&dlci->gsm->tx_mutex); + spin_unlock_irqrestore(&dlci->gsm->tx_lock, flags); } /* @@ -1256,6 +1258,7 @@ static void gsm_control_message(struct gsm_mux *gsm, unsigned int command, const u8 *data, int clen) { u8 buf[1]; + unsigned long flags; switch (command) { case CMD_CLD: { @@ -1277,9 +1280,9 @@ static void gsm_control_message(struct gsm_mux *gsm, unsigned int command, gsm->constipated = false; gsm_control_reply(gsm, CMD_FCON, NULL, 0); /* Kick the link in case it is idling */ - mutex_lock(&gsm->tx_mutex); + spin_lock_irqsave(&gsm->tx_lock, flags); gsm_data_kick(gsm, NULL); - mutex_unlock(&gsm->tx_mutex); + spin_unlock_irqrestore(&gsm->tx_lock, flags); break; case CMD_FCOFF: /* Modem wants us to STFU */ @@ -2225,7 +2228,6 @@ static void gsm_free_mux(struct gsm_mux *gsm) break; } } - mutex_destroy(&gsm->tx_mutex); mutex_destroy(&gsm->mutex); kfree(gsm->txframe); kfree(gsm->buf); @@ -2297,12 +2299,12 @@ static struct gsm_mux *gsm_alloc_mux(void) } spin_lock_init(&gsm->lock); mutex_init(&gsm->mutex); - mutex_init(&gsm->tx_mutex); kref_init(&gsm->ref); INIT_LIST_HEAD(&gsm->tx_list); timer_setup(&gsm->t2_timer, gsm_control_retransmit, 0); init_waitqueue_head(&gsm->event); spin_lock_init(&gsm->control_lock); + spin_lock_init(&gsm->tx_lock); gsm->t1 = T1; gsm->t2 = T2; @@ -2327,7 +2329,6 @@ static struct gsm_mux *gsm_alloc_mux(void) } spin_unlock(&gsm_mux_lock); if (i == MAX_MUX) { - mutex_destroy(&gsm->tx_mutex); mutex_destroy(&gsm->mutex); kfree(gsm->txframe); kfree(gsm->buf); @@ -2652,15 +2653,16 @@ static int gsmld_open(struct tty_struct *tty) static void gsmld_write_wakeup(struct tty_struct *tty) { struct gsm_mux *gsm = tty->disc_data; + unsigned long flags; /* Queue poll */ clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags); - mutex_lock(&gsm->tx_mutex); + spin_lock_irqsave(&gsm->tx_lock, flags); gsm_data_kick(gsm, NULL); if (gsm->tx_bytes < TX_THRESH_LO) { gsm_dlci_data_sweep(gsm); } - mutex_unlock(&gsm->tx_mutex); + spin_unlock_irqrestore(&gsm->tx_lock, flags); } /** @@ -2703,6 +2705,7 @@ static ssize_t gsmld_write(struct tty_struct *tty, struct file *file, const unsigned char *buf, size_t nr) { struct gsm_mux *gsm = tty->disc_data; + unsigned long flags; int space; int ret; @@ -2710,13 +2713,13 @@ static ssize_t gsmld_write(struct tty_struct *tty, struct file *file, return -ENODEV; ret = -ENOBUFS; - mutex_lock(&gsm->tx_mutex); + spin_lock_irqsave(&gsm->tx_lock, flags); space = tty_write_room(tty); if (space >= nr) ret = tty->ops->write(tty, buf, nr); else set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags); - mutex_unlock(&gsm->tx_mutex); + spin_unlock_irqrestore(&gsm->tx_lock, flags); return ret; } -- 2.38.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" 2022-12-03 21:55 ` [PATCH v2 " Fedor Pchelkin @ 2022-12-04 15:59 ` Greg Kroah-Hartman 0 siblings, 0 replies; 11+ messages in thread From: Greg Kroah-Hartman @ 2022-12-04 15:59 UTC (permalink / raw) To: Fedor Pchelkin Cc: stable, linux-kernel, Daniel Starke, jirislaby, Alexey Khoroshilov, lvc-project, Pavel Machek On Sun, Dec 04, 2022 at 12:55:18AM +0300, Fedor Pchelkin wrote: > From: Fedor Pchelkin <pchelkin@ispras.ru> > > [ Upstream commit acdab4cb4ba7e5f94d2b422ebd7bf4bf68178fb2 ] > > This reverts commit eb75efdec8dd0f01ac85c88feafa6e63b34a2521. > > The above commit is reverted as the usage of tx_mutex seems not to solve > the problem described in eb75efdec8dd ("tty: n_gsm: avoid call of sleeping > functions from atomic context") and just moves the bug to another place. > > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru> > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> > Reviewed-by: Daniel Starke <daniel.starke@siemens.com> > Link: https://lore.kernel.org/r/20221008110221.13645-2-pchelkin@ispras.ru > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Reviewed-by: Pavel Machek <pavel@denx.de> Now queued up, thanks. greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-12-04 15:59 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-23 18:19 [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" Fedor Pchelkin 2022-11-28 14:20 ` Pavel Machek 2022-12-03 13:35 ` Greg Kroah-Hartman 2022-12-03 22:35 ` [PATCH 5.19 0/2] tty: n_gsm: revert tx_mutex usage Fedor Pchelkin 2022-12-03 22:35 ` [PATCH 5.19 1/2] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" Fedor Pchelkin 2022-12-03 22:35 ` [PATCH 5.19 2/2] Revert "tty: n_gsm: replace kicktimer with delayed_work" Fedor Pchelkin 2022-12-04 15:56 ` [PATCH 5.19 0/2] tty: n_gsm: revert tx_mutex usage Greg Kroah-Hartman 2022-12-04 15:57 ` [PATCH 5.10] Revert "tty: n_gsm: avoid call of sleeping functions from atomic context" Greg Kroah-Hartman 2022-12-03 13:38 ` Greg Kroah-Hartman 2022-12-03 21:55 ` [PATCH v2 " Fedor Pchelkin 2022-12-04 15:59 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox