* [PATCH 1/2] Bluetooth: Move scope of state_to_string @ 2012-02-15 9:50 Emeltchenko Andrei 2012-02-15 9:50 ` [PATCH 2/2] Bluetooth: Use symbolic name for state in debug Emeltchenko Andrei 2012-02-15 12:20 ` [PATCH 1/2] Bluetooth: Move scope of state_to_string Marcel Holtmann 0 siblings, 2 replies; 8+ messages in thread From: Emeltchenko Andrei @ 2012-02-15 9:50 UTC (permalink / raw) To: linux-bluetooth From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> Function state_to_string will be used in other files. Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> --- include/net/bluetooth/bluetooth.h | 27 +++++++++++++++++++++++++++ net/bluetooth/l2cap_core.c | 26 -------------------------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h index 4a82ca0..0a47c20 100644 --- a/include/net/bluetooth/bluetooth.h +++ b/include/net/bluetooth/bluetooth.h @@ -129,6 +129,33 @@ enum { BT_CLOSED }; +/* If unused will be removed by compiler */ +static inline char *state_to_string(int state) +{ + switch (state) { + case BT_CONNECTED: + return "BT_CONNECTED"; + case BT_OPEN: + return "BT_OPEN"; + case BT_BOUND: + return "BT_BOUND"; + case BT_LISTEN: + return "BT_LISTEN"; + case BT_CONNECT: + return "BT_CONNECT"; + case BT_CONNECT2: + return "BT_CONNECT2"; + case BT_CONFIG: + return "BT_CONFIG"; + case BT_DISCONN: + return "BT_DISCONN"; + case BT_CLOSED: + return "BT_CLOSED"; + } + + return "invalid state"; +} + /* BD Address */ typedef struct { __u8 b[6]; diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 8efac78..252a96e 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -215,32 +215,6 @@ static u16 l2cap_alloc_cid(struct l2cap_conn *conn) return 0; } -static char *state_to_string(int state) -{ - switch(state) { - case BT_CONNECTED: - return "BT_CONNECTED"; - case BT_OPEN: - return "BT_OPEN"; - case BT_BOUND: - return "BT_BOUND"; - case BT_LISTEN: - return "BT_LISTEN"; - case BT_CONNECT: - return "BT_CONNECT"; - case BT_CONNECT2: - return "BT_CONNECT2"; - case BT_CONFIG: - return "BT_CONFIG"; - case BT_DISCONN: - return "BT_DISCONN"; - case BT_CLOSED: - return "BT_CLOSED"; - } - - return "invalid state"; -} - static void l2cap_state_change(struct l2cap_chan *chan, int state) { BT_DBG("%p %s -> %s", chan, state_to_string(chan->state), -- 1.7.9 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] Bluetooth: Use symbolic name for state in debug 2012-02-15 9:50 [PATCH 1/2] Bluetooth: Move scope of state_to_string Emeltchenko Andrei @ 2012-02-15 9:50 ` Emeltchenko Andrei 2012-02-15 12:20 ` [PATCH 1/2] Bluetooth: Move scope of state_to_string Marcel Holtmann 1 sibling, 0 replies; 8+ messages in thread From: Emeltchenko Andrei @ 2012-02-15 9:50 UTC (permalink / raw) To: linux-bluetooth From: Andrei Emeltchenko <andrei.emeltchenko@intel.com> Use state_to_string function in debug Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> --- include/net/bluetooth/l2cap.h | 3 ++- net/bluetooth/hci_conn.c | 2 +- net/bluetooth/l2cap_core.c | 5 +++-- net/bluetooth/l2cap_sock.c | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index 42fdbb8..bbb0e21 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -612,7 +612,8 @@ static inline void l2cap_chan_put(struct l2cap_chan *c) static inline void l2cap_set_timer(struct l2cap_chan *chan, struct delayed_work *work, long timeout) { - BT_DBG("chan %p state %d timeout %ld", chan, chan->state, timeout); + BT_DBG("chan %p state %s timeout %ld", chan, + state_to_string(chan->state), timeout); if (!cancel_delayed_work(work)) l2cap_chan_hold(chan); diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 6d15cba..f7a090c 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -282,7 +282,7 @@ static void hci_conn_timeout(struct work_struct *work) disc_work.work); __u8 reason; - BT_DBG("conn %p state %d", conn, conn->state); + BT_DBG("conn %p state %s", conn, state_to_string(conn->state)); if (atomic_read(&conn->refcnt)) return; diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 252a96e..88968e7 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -231,7 +231,7 @@ static void l2cap_chan_timeout(struct work_struct *work) struct sock *sk = chan->sk; int reason; - BT_DBG("chan %p state %d", chan, chan->state); + BT_DBG("chan %p state %s", chan, state_to_string(chan->state)); lock_sock(sk); @@ -413,7 +413,8 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason) struct l2cap_conn *conn = chan->conn; struct sock *sk = chan->sk; - BT_DBG("chan %p state %d socket %p", chan, chan->state, sk->sk_socket); + BT_DBG("chan %p state %s sk %p", chan, + state_to_string(chan->state), sk); switch (chan->state) { case BT_LISTEN: diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index 138fe34..b48d6c1 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -783,7 +783,7 @@ static void l2cap_sock_kill(struct sock *sk) if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket) return; - BT_DBG("sk %p state %d", sk, sk->sk_state); + BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state)); /* Kill poor orphan */ -- 1.7.9 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Bluetooth: Move scope of state_to_string 2012-02-15 9:50 [PATCH 1/2] Bluetooth: Move scope of state_to_string Emeltchenko Andrei 2012-02-15 9:50 ` [PATCH 2/2] Bluetooth: Use symbolic name for state in debug Emeltchenko Andrei @ 2012-02-15 12:20 ` Marcel Holtmann 2012-02-15 12:53 ` Emeltchenko Andrei 2012-02-17 9:10 ` Emeltchenko Andrei 1 sibling, 2 replies; 8+ messages in thread From: Marcel Holtmann @ 2012-02-15 12:20 UTC (permalink / raw) To: Emeltchenko Andrei; +Cc: linux-bluetooth Hi Andrei, > Function state_to_string will be used in other files. > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > --- > include/net/bluetooth/bluetooth.h | 27 +++++++++++++++++++++++++++ > net/bluetooth/l2cap_core.c | 26 -------------------------- > 2 files changed, 27 insertions(+), 26 deletions(-) > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h > index 4a82ca0..0a47c20 100644 > --- a/include/net/bluetooth/bluetooth.h > +++ b/include/net/bluetooth/bluetooth.h > @@ -129,6 +129,33 @@ enum { > BT_CLOSED > }; > > +/* If unused will be removed by compiler */ > +static inline char *state_to_string(int state) > +{ > + switch (state) { > + case BT_CONNECTED: > + return "BT_CONNECTED"; > + case BT_OPEN: > + return "BT_OPEN"; > + case BT_BOUND: > + return "BT_BOUND"; > + case BT_LISTEN: > + return "BT_LISTEN"; > + case BT_CONNECT: > + return "BT_CONNECT"; > + case BT_CONNECT2: > + return "BT_CONNECT2"; > + case BT_CONFIG: > + return "BT_CONFIG"; > + case BT_DISCONN: > + return "BT_DISCONN"; > + case BT_CLOSED: > + return "BT_CLOSED"; > + } > + > + return "invalid state"; > +} > + so long term, we need to keep socket states inside l2cap_sock.c and L2CAP states inside l2cap_core.c. Since L2CAP core engine should use the states from the specification and not try to fit into socket states. I see a little bit of value here in having them in the debug log, but making them public like this seems a big overhead. Did you measure the size difference. And most likely some state lookup table is better. Regards Marcel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Bluetooth: Move scope of state_to_string 2012-02-15 12:20 ` [PATCH 1/2] Bluetooth: Move scope of state_to_string Marcel Holtmann @ 2012-02-15 12:53 ` Emeltchenko Andrei 2012-02-17 9:10 ` Emeltchenko Andrei 1 sibling, 0 replies; 8+ messages in thread From: Emeltchenko Andrei @ 2012-02-15 12:53 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth Hi Marcel, On Wed, Feb 15, 2012 at 01:20:14PM +0100, Marcel Holtmann wrote: > Hi Andrei, > > > Function state_to_string will be used in other files. > > > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > > --- > > include/net/bluetooth/bluetooth.h | 27 +++++++++++++++++++++++++++ > > net/bluetooth/l2cap_core.c | 26 -------------------------- > > 2 files changed, 27 insertions(+), 26 deletions(-) > > > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h > > index 4a82ca0..0a47c20 100644 > > --- a/include/net/bluetooth/bluetooth.h > > +++ b/include/net/bluetooth/bluetooth.h > > @@ -129,6 +129,33 @@ enum { > > BT_CLOSED > > }; > > > > +/* If unused will be removed by compiler */ > > +static inline char *state_to_string(int state) > > +{ > > + switch (state) { > > + case BT_CONNECTED: > > + return "BT_CONNECTED"; > > + case BT_OPEN: > > + return "BT_OPEN"; > > + case BT_BOUND: > > + return "BT_BOUND"; > > + case BT_LISTEN: > > + return "BT_LISTEN"; > > + case BT_CONNECT: > > + return "BT_CONNECT"; > > + case BT_CONNECT2: > > + return "BT_CONNECT2"; > > + case BT_CONFIG: > > + return "BT_CONFIG"; > > + case BT_DISCONN: > > + return "BT_DISCONN"; > > + case BT_CLOSED: > > + return "BT_CLOSED"; > > + } > > + > > + return "invalid state"; > > +} > > + > > so long term, we need to keep socket states inside l2cap_sock.c and > L2CAP states inside l2cap_core.c. Since L2CAP core engine should use the > states from the specification and not try to fit into socket states. > > I see a little bit of value here in having them in the debug log, but > making them public like this seems a big overhead. Did you measure the > size difference. And most likely some state lookup table is better. If unused this shall be removed by compiler or what do you mean here? Do you think l2cap.h is better place? Best regards Andrei Emeltchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Bluetooth: Move scope of state_to_string 2012-02-15 12:20 ` [PATCH 1/2] Bluetooth: Move scope of state_to_string Marcel Holtmann 2012-02-15 12:53 ` Emeltchenko Andrei @ 2012-02-17 9:10 ` Emeltchenko Andrei 2012-02-17 9:12 ` Marcel Holtmann 1 sibling, 1 reply; 8+ messages in thread From: Emeltchenko Andrei @ 2012-02-17 9:10 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth Hi Marcel, On Wed, Feb 15, 2012 at 01:20:14PM +0100, Marcel Holtmann wrote: > Hi Andrei, > > > Function state_to_string will be used in other files. > > > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > > --- > > include/net/bluetooth/bluetooth.h | 27 +++++++++++++++++++++++++++ > > net/bluetooth/l2cap_core.c | 26 -------------------------- > > 2 files changed, 27 insertions(+), 26 deletions(-) > > > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h > > index 4a82ca0..0a47c20 100644 > > --- a/include/net/bluetooth/bluetooth.h > > +++ b/include/net/bluetooth/bluetooth.h > > @@ -129,6 +129,33 @@ enum { > > BT_CLOSED > > }; > > > > +/* If unused will be removed by compiler */ > > +static inline char *state_to_string(int state) > > +{ > > + switch (state) { > > + case BT_CONNECTED: > > + return "BT_CONNECTED"; > > + case BT_OPEN: > > + return "BT_OPEN"; > > + case BT_BOUND: > > + return "BT_BOUND"; > > + case BT_LISTEN: > > + return "BT_LISTEN"; > > + case BT_CONNECT: > > + return "BT_CONNECT"; > > + case BT_CONNECT2: > > + return "BT_CONNECT2"; > > + case BT_CONFIG: > > + return "BT_CONFIG"; > > + case BT_DISCONN: > > + return "BT_DISCONN"; > > + case BT_CLOSED: > > + return "BT_CLOSED"; > > + } > > + > > + return "invalid state"; > > +} > > + > > so long term, we need to keep socket states inside l2cap_sock.c and > L2CAP states inside l2cap_core.c. Since L2CAP core engine should use the > states from the specification and not try to fit into socket states. > > I see a little bit of value here in having them in the debug log, but > making them public like this seems a big overhead. Did you measure the > size difference. size show exactly the same size after applying this patch with my config. (dynamic debug enabled) size net/bluetooth/bluetooth.ko text data bss dec hex filename 196404 12912 644 209960 33428 net/bluetooth/bluetooth.ko > And most likely some state lookup table is better. This can be done, BTW: unused functions will be removed by compiler, what about lookup table? I assume the same. Best regards Andrei Emeltchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Bluetooth: Move scope of state_to_string 2012-02-17 9:10 ` Emeltchenko Andrei @ 2012-02-17 9:12 ` Marcel Holtmann 2012-02-17 9:18 ` Emeltchenko Andrei 0 siblings, 1 reply; 8+ messages in thread From: Marcel Holtmann @ 2012-02-17 9:12 UTC (permalink / raw) To: Emeltchenko Andrei; +Cc: linux-bluetooth Hi Andrei, > > > Function state_to_string will be used in other files. > > > > > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com> > > > --- > > > include/net/bluetooth/bluetooth.h | 27 +++++++++++++++++++++++++++ > > > net/bluetooth/l2cap_core.c | 26 -------------------------- > > > 2 files changed, 27 insertions(+), 26 deletions(-) > > > > > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h > > > index 4a82ca0..0a47c20 100644 > > > --- a/include/net/bluetooth/bluetooth.h > > > +++ b/include/net/bluetooth/bluetooth.h > > > @@ -129,6 +129,33 @@ enum { > > > BT_CLOSED > > > }; > > > > > > +/* If unused will be removed by compiler */ > > > +static inline char *state_to_string(int state) > > > +{ > > > + switch (state) { > > > + case BT_CONNECTED: > > > + return "BT_CONNECTED"; > > > + case BT_OPEN: > > > + return "BT_OPEN"; > > > + case BT_BOUND: > > > + return "BT_BOUND"; > > > + case BT_LISTEN: > > > + return "BT_LISTEN"; > > > + case BT_CONNECT: > > > + return "BT_CONNECT"; > > > + case BT_CONNECT2: > > > + return "BT_CONNECT2"; > > > + case BT_CONFIG: > > > + return "BT_CONFIG"; > > > + case BT_DISCONN: > > > + return "BT_DISCONN"; > > > + case BT_CLOSED: > > > + return "BT_CLOSED"; > > > + } > > > + > > > + return "invalid state"; > > > +} > > > + > > > > so long term, we need to keep socket states inside l2cap_sock.c and > > L2CAP states inside l2cap_core.c. Since L2CAP core engine should use the > > states from the specification and not try to fit into socket states. > > > > I see a little bit of value here in having them in the debug log, but > > making them public like this seems a big overhead. Did you measure the > > size difference. > > size show exactly the same size after applying this patch with my config. > (dynamic debug enabled) > > size net/bluetooth/bluetooth.ko > text data bss dec hex filename > 196404 12912 644 209960 33428 net/bluetooth/bluetooth.ko then I am fine with this. Just send an updated patch against the latest tree. > > And most likely some state lookup table is better. > > This can be done, BTW: unused functions will be removed by compiler, what > about lookup table? I assume the same. Depends on where it is. If it needs to be a public symbol because it is used from multiple modules, then most likely not. Regards Marcel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Bluetooth: Move scope of state_to_string 2012-02-17 9:12 ` Marcel Holtmann @ 2012-02-17 9:18 ` Emeltchenko Andrei 2012-02-17 9:30 ` Marcel Holtmann 0 siblings, 1 reply; 8+ messages in thread From: Emeltchenko Andrei @ 2012-02-17 9:18 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth Hi Marcel, On Fri, Feb 17, 2012 at 10:12:10AM +0100, Marcel Holtmann wrote: > Hi Andrei, > > > > > Function state_to_string will be used in other files. > > > > > > > so long term, we need to keep socket states inside l2cap_sock.c and > > > L2CAP states inside l2cap_core.c. Since L2CAP core engine should use the > > > states from the specification and not try to fit into socket states. > > > > > > I see a little bit of value here in having them in the debug log, but > > > making them public like this seems a big overhead. Did you measure the > > > size difference. > > > > size show exactly the same size after applying this patch with my config. > > (dynamic debug enabled) > > > > size net/bluetooth/bluetooth.ko > > text data bss dec hex filename > > 196404 12912 644 209960 33428 net/bluetooth/bluetooth.ko > > then I am fine with this. Just send an updated patch against the latest > tree. If we don't want it to be in bluetooth.h I can export only function declaration or put it in l2cap.h. What is better? Best regards Andrei Emeltchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Bluetooth: Move scope of state_to_string 2012-02-17 9:18 ` Emeltchenko Andrei @ 2012-02-17 9:30 ` Marcel Holtmann 0 siblings, 0 replies; 8+ messages in thread From: Marcel Holtmann @ 2012-02-17 9:30 UTC (permalink / raw) To: Emeltchenko Andrei; +Cc: linux-bluetooth Hi Andrei, > > > > > Function state_to_string will be used in other files. > > > > > > > > > so long term, we need to keep socket states inside l2cap_sock.c and > > > > L2CAP states inside l2cap_core.c. Since L2CAP core engine should use the > > > > states from the specification and not try to fit into socket states. > > > > > > > > I see a little bit of value here in having them in the debug log, but > > > > making them public like this seems a big overhead. Did you measure the > > > > size difference. > > > > > > size show exactly the same size after applying this patch with my config. > > > (dynamic debug enabled) > > > > > > size net/bluetooth/bluetooth.ko > > > text data bss dec hex filename > > > 196404 12912 644 209960 33428 net/bluetooth/bluetooth.ko > > > > then I am fine with this. Just send an updated patch against the latest > > tree. > > If we don't want it to be in bluetooth.h I can export only function > declaration or put it in l2cap.h. What is better? if the size is the same, then put it in bluetooth.h. Regards Marcel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-02-17 9:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-02-15 9:50 [PATCH 1/2] Bluetooth: Move scope of state_to_string Emeltchenko Andrei 2012-02-15 9:50 ` [PATCH 2/2] Bluetooth: Use symbolic name for state in debug Emeltchenko Andrei 2012-02-15 12:20 ` [PATCH 1/2] Bluetooth: Move scope of state_to_string Marcel Holtmann 2012-02-15 12:53 ` Emeltchenko Andrei 2012-02-17 9:10 ` Emeltchenko Andrei 2012-02-17 9:12 ` Marcel Holtmann 2012-02-17 9:18 ` Emeltchenko Andrei 2012-02-17 9:30 ` Marcel Holtmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).