linux-embedded.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Move an assert under DEBUG_KERNEL.
@ 2011-01-06  8:13 Rob Landley
  2011-01-06 23:41 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Rob Landley @ 2011-01-06  8:13 UTC (permalink / raw)
  To: trivial, linux-kernel, linux-embedded

From: Rob Landley <rlandley@parallels.com>

Move an assert under DEBUG_KERNEL.

Signed-off-by: Rob Landley <rlandley@parallels.com>
---

Saves about 3k from x86-64 defconfig according to scripts/bloat-o-meter.

  include/linux/rtnetlink.h |    4 ++++
  1 file changed, 4 insertions(+)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index bbad657..28c4025 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -782,6 +782,7 @@ extern struct netdev_queue 
*dev_ingress_queue_create(struct net_device *dev);
  extern void rtnetlink_init(void);
  extern void __rtnl_unlock(void);

+#ifdef CONFIG_DEBUG_KERNEL
  #define ASSERT_RTNL() do { \
  	if (unlikely(!rtnl_is_locked())) { \
  		printk(KERN_ERR "RTNL: assertion failed at %s (%d)\n", \
@@ -789,6 +790,9 @@ extern void __rtnl_unlock(void);
  		dump_stack(); \
  	} \
  } while(0)
+#else
+#define ASSERT_RTNL()
+#endif

  static inline u32 rtm_get_table(struct rtattr **rta, u8 table)
  {

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Move an assert under DEBUG_KERNEL.
  2011-01-06  8:13 [PATCH] Move an assert under DEBUG_KERNEL Rob Landley
@ 2011-01-06 23:41 ` Andrew Morton
  2011-01-07  9:44   ` Rob Landley
  2011-01-10  8:35   ` [PATCH] Move an assert under DEBUG_KERNEL. (attempt 2) Rob Landley
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2011-01-06 23:41 UTC (permalink / raw)
  To: Rob Landley; +Cc: trivial, linux-kernel, linux-embedded

On Thu, 6 Jan 2011 02:13:38 -0600
Rob Landley <rlandley@parallels.com> wrote:

> From: Rob Landley <rlandley@parallels.com>
> 
> Move an assert under DEBUG_KERNEL.
> 
> Signed-off-by: Rob Landley <rlandley@parallels.com>
> ---
> 
> Saves about 3k from x86-64 defconfig according to scripts/bloat-o-meter.
> 
>   include/linux/rtnetlink.h |    4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index bbad657..28c4025 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -782,6 +782,7 @@ extern struct netdev_queue 
> *dev_ingress_queue_create(struct net_device *dev);
>   extern void rtnetlink_init(void);
>   extern void __rtnl_unlock(void);
> 
> +#ifdef CONFIG_DEBUG_KERNEL
>   #define ASSERT_RTNL() do { \
>   	if (unlikely(!rtnl_is_locked())) { \
>   		printk(KERN_ERR "RTNL: assertion failed at %s (%d)\n", \
> @@ -789,6 +790,9 @@ extern void __rtnl_unlock(void);
>   		dump_stack(); \
>   	} \
>   } while(0)
> +#else
> +#define ASSERT_RTNL()
> +#endif
> 
>   static inline u32 rtm_get_table(struct rtattr **rta, u8 table)
>   {

Probably a worthwhile thing to do, IMO.  If there's some net-specific
CONFIG_DEBUG_ setting then that wold be a better thing to use.

However the patch was a) wordwrapped, b) space-stuffed and c) not cc'ed
to the networking list.  So its prospects are dim.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Move an assert under DEBUG_KERNEL.
  2011-01-06 23:41 ` Andrew Morton
@ 2011-01-07  9:44   ` Rob Landley
  2011-01-10  8:35   ` [PATCH] Move an assert under DEBUG_KERNEL. (attempt 2) Rob Landley
  1 sibling, 0 replies; 6+ messages in thread
From: Rob Landley @ 2011-01-07  9:44 UTC (permalink / raw)
  To: Andrew Morton; +Cc: trivial, linux-kernel, linux-embedded

On 01/06/2011 05:41 PM, Andrew Morton wrote:
>> +#ifdef CONFIG_DEBUG_KERNEL
>>    #define ASSERT_RTNL() do { \
>>    	if (unlikely(!rtnl_is_locked())) { \
>>    		printk(KERN_ERR "RTNL: assertion failed at %s (%d)\n", \
>> @@ -789,6 +790,9 @@ extern void __rtnl_unlock(void);
>>    		dump_stack(); \
>>    	} \
>>    } while(0)
>> +#else
>> +#define ASSERT_RTNL()
>> +#endif
>>
>>    static inline u32 rtm_get_table(struct rtattr **rta, u8 table)
>>    {
>
> Probably a worthwhile thing to do, IMO.  If there's some net-specific
> CONFIG_DEBUG_ setting then that wold be a better thing to use.

I looked and didn't find one.  lib/Kconfig.debug has DEBUG_OBJECTS and 
PROVE_LOCKING and such but nothing quite on topic.  The only "DEBUG" in 
net/Kconfig is NETFLITER_DEBUG.  Nothing relevant in 
drivers/net/Kconfig, there isn't a Kconfig in net/core...

I thought about adding a new symbol, but CONFIG_DEBUG_KERNEL is already 
used in a few existing places:

   arch/powerpc/kernel/sysfs.c
   arch/parisc/mm/init.c
   arch/blackfin/include/asm/entry.h

So this isn't the first instance of it, but that doesn't mean those uses 
are correct. :)

> However the patch was a) wordwrapped, b) space-stuffed and c) not cc'ed
> to the networking list.  So its prospects are dim.

Sorry, finally gave up on kmail and set up thunderbird.  Still trying to 
beat the darn thing into submission.  (It looked right before I hit 
send.  And I cursored over the tabs to make sure. :)

I'll work out my email issues and then cc: the networking list on the 
resubmit.

Thanks,

Rob

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] Move an assert under DEBUG_KERNEL. (attempt 2)
  2011-01-06 23:41 ` Andrew Morton
  2011-01-07  9:44   ` Rob Landley
@ 2011-01-10  8:35   ` Rob Landley
  2011-01-10 17:15     ` Randy Dunlap
  2011-01-10 17:19     ` Randy Dunlap
  1 sibling, 2 replies; 6+ messages in thread
From: Rob Landley @ 2011-01-10  8:35 UTC (permalink / raw)
  To: Andrew Morton; +Cc: trivial, linux-net, linux-kernel, linux-embedded

On 01/06/2011 05:41 PM, Andrew Morton wrote:
> Probably a worthwhile thing to do, IMO.  If there's some net-specific
> CONFIG_DEBUG_ setting then that wold be a better thing to use.
> 
> However the patch was a) wordwrapped, b) space-stuffed and c) not cc'ed
> to the networking list.  So its prospects are dim.

Ok, either I've beaten thunderbird into submission, or I'll be submitting a patch to Documentation/email-clients.txt.  (Whether or not I need to find a different smtp server to send this through remains an open question.)

(Confirming: I looked for a more specific DEBUG symbol, but couldn't find one.  I can add one, but this seems a bit small to have its own symbol, and DEBUG_KERNEL is already used in a few *.c files.)

From: Rob Landley <rlandley@parallels.com>

Move an assert under DEBUG_KERNEL.  (Minor cleanup to save a few bytes.)

Signed-off-by: Rob Landley <rlandley@parallels.com>
---

 include/linux/rtnetlink.h |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index bbad657..28c4025 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -782,6 +782,7 @@ extern struct netdev_queue *dev_ingress_queue_create(struct net_device *dev);
 extern void rtnetlink_init(void);
 extern void __rtnl_unlock(void);
 
+#ifdef CONFIG_DEBUG_KERNEL
 #define ASSERT_RTNL() do { \
 	if (unlikely(!rtnl_is_locked())) { \
 		printk(KERN_ERR "RTNL: assertion failed at %s (%d)\n", \
@@ -789,6 +790,9 @@ extern void __rtnl_unlock(void);
 		dump_stack(); \
 	} \
 } while(0)
+#else
+#define ASSERT_RTNL()
+#endif
 
 static inline u32 rtm_get_table(struct rtattr **rta, u8 table)
 {

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Move an assert under DEBUG_KERNEL. (attempt 2)
  2011-01-10  8:35   ` [PATCH] Move an assert under DEBUG_KERNEL. (attempt 2) Rob Landley
@ 2011-01-10 17:15     ` Randy Dunlap
  2011-01-10 17:19     ` Randy Dunlap
  1 sibling, 0 replies; 6+ messages in thread
From: Randy Dunlap @ 2011-01-10 17:15 UTC (permalink / raw)
  To: Rob Landley
  Cc: Andrew Morton, trivial, linux-net, linux-kernel, linux-embedded

On Mon, 10 Jan 2011 02:35:33 -0600 Rob Landley wrote:

> On 01/06/2011 05:41 PM, Andrew Morton wrote:
> > Probably a worthwhile thing to do, IMO.  If there's some net-specific
> > CONFIG_DEBUG_ setting then that wold be a better thing to use.
> > 
> > However the patch was a) wordwrapped, b) space-stuffed and c) not cc'ed
> > to the networking list.  So its prospects are dim.
> 
> Ok, either I've beaten thunderbird into submission, or I'll be submitting a patch to Documentation/email-clients.txt.  (Whether or not I need to find a different smtp server to send this through remains an open question.)


Hi Rob,
The patch applies cleanly.  However, if you have any additions/updates to
Documentation/email-clients.txt, please send them along.

and please do hard CR/LF every 70-72 characters or so, instead of very_long_lines.


> (Confirming: I looked for a more specific DEBUG symbol, but couldn't find one.  I can add one, but this seems a bit small to have its own symbol, and DEBUG_KERNEL is already used in a few *.c files.)
> 
> From: Rob Landley <rlandley@parallels.com>
> 
> Move an assert under DEBUG_KERNEL.  (Minor cleanup to save a few bytes.)
> 
> Signed-off-by: Rob Landley <rlandley@parallels.com>
> ---
> 
>  include/linux/rtnetlink.h |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index bbad657..28c4025 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -782,6 +782,7 @@ extern struct netdev_queue *dev_ingress_queue_create(struct net_device *dev);
>  extern void rtnetlink_init(void);
>  extern void __rtnl_unlock(void);
>  
> +#ifdef CONFIG_DEBUG_KERNEL
>  #define ASSERT_RTNL() do { \
>  	if (unlikely(!rtnl_is_locked())) { \
>  		printk(KERN_ERR "RTNL: assertion failed at %s (%d)\n", \
> @@ -789,6 +790,9 @@ extern void __rtnl_unlock(void);
>  		dump_stack(); \
>  	} \
>  } while(0)
> +#else
> +#define ASSERT_RTNL()
> +#endif
>  
>  static inline u32 rtm_get_table(struct rtattr **rta, u8 table)
>  {
> --


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Move an assert under DEBUG_KERNEL. (attempt 2)
  2011-01-10  8:35   ` [PATCH] Move an assert under DEBUG_KERNEL. (attempt 2) Rob Landley
  2011-01-10 17:15     ` Randy Dunlap
@ 2011-01-10 17:19     ` Randy Dunlap
  1 sibling, 0 replies; 6+ messages in thread
From: Randy Dunlap @ 2011-01-10 17:19 UTC (permalink / raw)
  To: Rob Landley; +Cc: Andrew Morton, trivial, linux-kernel, linux-embedded, netdev

On Mon, 10 Jan 2011 02:35:33 -0600 Rob Landley wrote:

> On 01/06/2011 05:41 PM, Andrew Morton wrote:
> > Probably a worthwhile thing to do, IMO.  If there's some net-specific
> > CONFIG_DEBUG_ setting then that wold be a better thing to use.
> > 
> > However the patch was a) wordwrapped, b) space-stuffed and c) not cc'ed
> > to the networking list.  So its prospects are dim.
> 
> Ok, either I've beaten thunderbird into submission, or I'll be submitting a patch to Documentation/email-clients.txt.  (Whether or not I need to find a different smtp server to send this through remains an open question.)
> 
> (Confirming: I looked for a more specific DEBUG symbol, but couldn't find one.  I can add one, but this seems a bit small to have its own symbol, and DEBUG_KERNEL is already used in a few *.c files.)
> 
> From: Rob Landley <rlandley@parallels.com>
> 
> Move an assert under DEBUG_KERNEL.  (Minor cleanup to save a few bytes.)
> 
> Signed-off-by: Rob Landley <rlandley@parallels.com>
> ---
> 
>  include/linux/rtnetlink.h |    4 ++++
>  1 file changed, 4 insertions(+)


Hm, cc to linux-net should be to netdev instead (so I changed it).


> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index bbad657..28c4025 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -782,6 +782,7 @@ extern struct netdev_queue *dev_ingress_queue_create(struct net_device *dev);
>  extern void rtnetlink_init(void);
>  extern void __rtnl_unlock(void);
>  
> +#ifdef CONFIG_DEBUG_KERNEL
>  #define ASSERT_RTNL() do { \
>  	if (unlikely(!rtnl_is_locked())) { \
>  		printk(KERN_ERR "RTNL: assertion failed at %s (%d)\n", \
> @@ -789,6 +790,9 @@ extern void __rtnl_unlock(void);
>  		dump_stack(); \
>  	} \
>  } while(0)
> +#else
> +#define ASSERT_RTNL()
> +#endif

Empty macros in Linux usually take the (preferred) form of this one from kernel.h:

#define might_resched() do { } while (0)

although it's up to DaveM in this case.


>  
>  static inline u32 rtm_get_table(struct rtattr **rta, u8 table)
>  {
> --


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-01-10 17:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-06  8:13 [PATCH] Move an assert under DEBUG_KERNEL Rob Landley
2011-01-06 23:41 ` Andrew Morton
2011-01-07  9:44   ` Rob Landley
2011-01-10  8:35   ` [PATCH] Move an assert under DEBUG_KERNEL. (attempt 2) Rob Landley
2011-01-10 17:15     ` Randy Dunlap
2011-01-10 17:19     ` Randy Dunlap

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).