diff mbox

[tty-next,15/22] isdn: tty: Use private flag for ASYNC_CLOSING

Message ID 1402924639-5164-16-git-send-email-peter@hurleysoftware.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Peter Hurley June 16, 2014, 1:17 p.m. UTC
ASYNC_CLOSING is no longer used in the tty core; use private flag
info->closing as substitute.

CC: Karsten Keil <isdn@linux-pingi.de>
CC: netdev@vger.kernel.org
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/isdn/i4l/isdn_tty.c | 14 +++++++-------
 include/linux/isdn.h        |  1 +
 2 files changed, 8 insertions(+), 7 deletions(-)

Comments

David Laight June 16, 2014, 3:37 p.m. UTC | #1
From: Of Peter Hurley
> ASYNC_CLOSING is no longer used in the tty core; use private flag
> info->closing as substitute.
...
> @@ -311,6 +311,7 @@ typedef struct atemu {
>  typedef struct modem_info {
>    int			magic;
>    struct tty_port	port;
> +  int			closing:1;	 /* port count has dropped to 0    */
>    int			x_char;		 /* xon/xoff character             */
>    int			mcr;		 /* Modem control register         */
>    int                   msr;             /* Modem status register          */

That should probably be a bool and set to true/false.
You are probably adding a load of padding.

	David



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Hurley June 16, 2014, 9:01 p.m. UTC | #2
Hi David,

On 06/16/2014 11:37 AM, David Laight wrote:
> From: Of Peter Hurley
>> ASYNC_CLOSING is no longer used in the tty core; use private flag
>> info->closing as substitute.
> ...
>> @@ -311,6 +311,7 @@ typedef struct atemu {
>>   typedef struct modem_info {
>>     int			magic;
>>     struct tty_port	port;
>> +  int			closing:1;	 /* port count has dropped to 0    */
>>     int			x_char;		 /* xon/xoff character             */
>>     int			mcr;		 /* Modem control register         */
>>     int                   msr;             /* Modem status register          */
>
> That should probably be a bool and set to true/false.
> You are probably adding a load of padding.

struct modem_info is over 1K, with several existing int-as-bool fields.
An array of 64 struct modem_info are statically allocated with every isdn device.

It doesn't look like memory consumption has been a consideration with the isdn driver.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Cox June 17, 2014, 11:58 a.m. UTC | #3
On Mon, 16 Jun 2014 09:17:12 -0400
Peter Hurley <peter@hurleysoftware.com> wrote:

> ASYNC_CLOSING is no longer used in the tty core; use private flag
> info->closing as substitute.
> 
> CC: Karsten Keil <isdn@linux-pingi.de>
> CC: netdev@vger.kernel.org
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>  drivers/isdn/i4l/isdn_tty.c | 14 +++++++-------
>  include/linux/isdn.h        |  1 +
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
> index 732f68a..5310932 100644
> --- a/drivers/isdn/i4l/isdn_tty.c
> +++ b/drivers/isdn/i4l/isdn_tty.c
> @@ -1577,8 +1577,7 @@ isdn_tty_close(struct tty_struct *tty, struct file *filp)
>  #endif
>  		return;
>  	}
> -	port->flags |= ASYNC_CLOSING;
> -
> +	info->closing = 1;

This is not sane C because

> +  int			closing:1;	 /* port count has dropped to 0    */

has the values 0 and -1.

Using a bool would let the compiler figure out what it wanted to do and
do the right thing. It'll probably generate identical code for most
processors but it gives it the freedom to do better.

Alan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index 732f68a..5310932 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -1577,8 +1577,7 @@  isdn_tty_close(struct tty_struct *tty, struct file *filp)
 #endif
 		return;
 	}
-	port->flags |= ASYNC_CLOSING;
-
+	info->closing = 1;
 	tty->closing = 1;
 	/*
 	 * At this point we stop accepting input.  To do this, we
@@ -1606,6 +1605,7 @@  isdn_tty_close(struct tty_struct *tty, struct file *filp)
 	tty_ldisc_flush(tty);
 	port->tty = NULL;
 	info->ncarrier = 0;
+	info->closing = 0;
 
 	tty_port_close_end(port, tty);
 #ifdef ISDN_DEBUG_MODEM_OPEN
@@ -1806,6 +1806,7 @@  isdn_tty_modem_init(void)
 		spin_lock_init(&info->readlock);
 		sprintf(info->last_cause, "0000");
 		sprintf(info->last_num, "none");
+		info->closing = 0;
 		info->last_dir = 0;
 		info->last_lhup = 1;
 		info->last_l2 = -1;
@@ -2241,7 +2242,7 @@  isdn_tty_at_cout(char *msg, modem_info *info)
 	l = strlen(msg);
 
 	spin_lock_irqsave(&info->readlock, flags);
-	if (port->flags & ASYNC_CLOSING) {
+	if (info->closing) {
 		spin_unlock_irqrestore(&info->readlock, flags);
 		return;
 	}
@@ -2391,13 +2392,12 @@  isdn_tty_modem_result(int code, modem_info *info)
 	case RESULT_NO_CARRIER:
 #ifdef ISDN_DEBUG_MODEM_HUP
 		printk(KERN_DEBUG "modem_result: NO CARRIER %d %d\n",
-		       (info->port.flags & ASYNC_CLOSING),
-		       (!info->port.tty));
+		       info->closing, (!info->port.tty));
 #endif
 		m->mdmreg[REG_RINGCNT] = 0;
 		del_timer(&info->nc_timer);
 		info->ncarrier = 0;
-		if ((info->port.flags & ASYNC_CLOSING) || (!info->port.tty))
+		if (info->closing || (!info->port.tty))
 			return;
 
 #ifdef CONFIG_ISDN_AUDIO
@@ -2530,7 +2530,7 @@  isdn_tty_modem_result(int code, modem_info *info)
 		}
 	}
 	if (code == RESULT_NO_CARRIER) {
-		if ((info->port.flags & ASYNC_CLOSING) || (!info->port.tty))
+		if (info->closing || (!info->port.tty))
 			return;
 
 		if (info->port.flags & ASYNC_CHECK_CD)
diff --git a/include/linux/isdn.h b/include/linux/isdn.h
index 1e9a0f2..fe80475 100644
--- a/include/linux/isdn.h
+++ b/include/linux/isdn.h
@@ -311,6 +311,7 @@  typedef struct atemu {
 typedef struct modem_info {
   int			magic;
   struct tty_port	port;
+  int			closing:1;	 /* port count has dropped to 0    */
   int			x_char;		 /* xon/xoff character             */
   int			mcr;		 /* Modem control register         */
   int                   msr;             /* Modem status register          */