Hi,
On 06/20/2013 11:08 AM, Sander Bosma wrote:
> When a channel got constipated, all messages in the messagequeue were
> still sent to the device. This can cause a bufferoverflow in some
> devices such as the SIM900 modem. This patch removes all pending
> messages belonging to the channel from the messagequeue when a
> channel gets constipated. Those messages are stored in a list within
> the DLCI. Once the channel is no longer constipated the stored
> messages are moved back into the global messagequeue. This
> significantly decreases the chance of bufferoverflows happening on
> receiving devices.
>
> Signed-off-by: Sander Bosma <s.bo...@interay.com>
> ---
> drivers/tty/n_gsm.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index 4a43ef5d7..0cad08fa 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
...
> @@ -931,7 +933,7 @@ static void gsm_dlci_data_sweep(struct gsm_mux *gsm)
> int len;
> /* Priority ordering: We should do priority with RR of the groups */
> int i = 1;
> -
> + struct tty_struct *tty;
Why did you remove the blank line here?
> while (i < NUM_DLCI) {
> struct gsm_dlci *dlci;
>
> @@ -946,6 +948,13 @@ static void gsm_dlci_data_sweep(struct gsm_mux *gsm)
> len = gsm_dlci_data_output(gsm, dlci);
> else
> len = gsm_dlci_data_output_framed(gsm, dlci);
> +
> + if (len >= 0) {
> + tty = tty_port_tty_get(&dlci->port);
> + if (tty)
> + tty_wakeup(dlci->port.tty);
> + }
This is incorrect. You leak a tty. And this should be
tty_port_tty_wakeup anyway.
But please also document addition of wakeup in the commit log.
> if (len < 0)
> break;
> /* DLCI empty - try the next */
> @@ -1029,7 +1038,8 @@ static void gsm_process_modem(struct tty_struct *tty,
> struct gsm_dlci *dlci,
> int mlines = 0;
> u8 brk = 0;
> int fc;
> -
> + struct gsm_msg *msg, *nmsg;
> + unsigned long flags;
Again removed blank line?
> /* The modem status command can either contain one octet (v.24 signals)
> or two octets (v.24 signals + break signals). The length field will
> either be 2 or 3 respectively. This is specified in section
> @@ -1047,8 +1057,27 @@ static void gsm_process_modem(struct tty_struct *tty,
> struct gsm_dlci *dlci,
> if (fc && !dlci->constipated) {
> /* Need to throttle our output on this device */
> dlci->constipated = 1;
> + spin_lock_irqsave(&dlci->gsm->tx_lock, flags);
> + list_for_each_entry_safe(msg, nmsg, &dlci->gsm->tx_list, list) {
> + list_del(&msg->list); /* remove from msgqueue */
> + /* add to list of messages which could not be sent
> + because of constipation */
> + list_add_tail(&msg->list, &dlci->constipated_list);
list_move_tail...
> + dlci->gsm->tx_bytes -= msg->len;
> + }
> + spin_unlock_irqrestore(&dlci->gsm->tx_lock, flags);
> +
> } else if (!fc && dlci->constipated) {
> dlci->constipated = 0;
> + spin_lock_irqsave(&dlci->gsm->tx_lock, flags);
> + list_for_each_entry_safe(msg, nmsg, &dlci->constipated_list,
> + list) {
> + list_del(&msg->list); /* remove from msgqueue */
> + /* add to global messagequeue */
> + list_add_tail(&msg->list, &dlci->gsm->tx_list);
Here too.
> + dlci->gsm->tx_bytes += msg->len;
> + }
> + spin_unlock_irqrestore(&dlci->gsm->tx_lock, flags);
> gsm_dlci_data_kick(dlci);
> }
>
...
> @@ -2956,7 +2986,7 @@ static void gsmtty_close(struct tty_struct *tty, struct
> file *filp)
> {
> struct gsm_dlci *dlci = tty->driver_data;
> struct gsm_mux *gsm;
> -
> + struct gsm_msg *msg, *nmsg;
blank.
thanks,
--
js
suse labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
