[PATCH] Staging:android:ion:ion.c : Using WARN_ON() rather than BUG()

[PATCH] Staging:android:ion:ion.c : Using WARN_ON() rather than BUG()

[harsha]

Hi,
harsha,  this is my real name.
There is no need to call BUG() over here as this error is not very basic and BUG() tends to bring the system down so calling WARN_ON() is preferable.
Please correct me if I am wrong as this is my first contribution.
I am unable to send any direct mail from gmail( gets rejected based on some filter rule match) so trying to send it in this way.
Thanks.

[PATCH] Staging:android:ion:ion.c : Using WARN_ON() rather than BUG()

[harsha]

Fixes checkpatch.pl warning: Use WARN_ON() rather than BUG_ON() and BUG()

Signed-off-by: harsha <harshasharmaiitr@gmail.com>
---
 drivers/staging/android/ion/ion.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 93e2c90..a2d36b3 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -66,7 +66,7 @@ static void ion_buffer_add(struct ion_device *dev,
 			p = &(*p)->rb_right;
 		} else {
 			pr_err("%s: buffer already found.", __func__);
-			BUG();
+			WARN_ON();
 		}
 	}
 
-- 
1.9.1

Re: [PATCH] Staging:android:ion:ion.c : Using WARN_ON() rather than BUG()

[Dan Carpenter]

On Wed, Aug 30, 2017 at 12:51:01AM +0530, harsha wrote:
> Hi,
> harsha,  this is my real name.

That's your full name that you use to sign legal documents?  No last
name? No capital letter at the beginning?

> There is no need to call BUG() over here as this error is not very basic and BUG() tends to bring the system down so calling WARN_ON() is preferable.

In the original code the kernel will call BUG() once but in the new code
it will spam the dmesg forever until you pull the power cord.  There is
no improvement.

regards,
dan carpenter

Re: [PATCH] Staging:android:ion:ion.c : Using WARN_ON() rather than BUG()

[Greg KH]

On Wed, Aug 30, 2017 at 12:51:02AM +0530, harsha wrote:
> Fixes checkpatch.pl warning: Use WARN_ON() rather than BUG_ON() and BUG()
> 
> Signed-off-by: harsha <harshasharmaiitr@gmail.com>
> ---
>  drivers/staging/android/ion/ion.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 93e2c90..a2d36b3 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -66,7 +66,7 @@ static void ion_buffer_add(struct ion_device *dev,
>  			p = &(*p)->rb_right;
>  		} else {
>  			pr_err("%s: buffer already found.", __func__);
> -			BUG();
> +			WARN_ON();
>  		}
>  	}
>  

As you did not do anything different from the last submission where I
said this patch was not correct, I'm guessing that you don't want any
feedback.  So I'll go add you to my kill-file now and just ignore your
emails, as obviously, you are ignoring mine :(

good luck!

greg k-h

Re: [PATCH] Staging:android:ion:ion.c : Using WARN_ON() rather than BUG()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4546 bytes --]

Hi harsha,

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.13-rc7 next-20170831]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/harsha/Staging-android-ion-ion-c-Using-WARN_ON-rather-than-BUG/20170901-160600
config: s390-allmodconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=s390 

All errors (new ones prefixed by >>):

   In file included from include/linux/bug.h:4:0,
                    from include/linux/mmdebug.h:4,
                    from arch/s390/include/asm/cmpxchg.h:10,
                    from arch/s390/include/asm/atomic.h:15,
                    from include/linux/atomic.h:4,
                    from include/linux/debug_locks.h:5,
                    from include/linux/lockdep.h:25,
                    from include/linux/spinlock_types.h:18,
                    from include/linux/mutex.h:15,
                    from include/linux/kernfs.h:13,
                    from include/linux/sysfs.h:15,
                    from include/linux/kobject.h:21,
                    from include/linux/device.h:17,
                    from drivers/staging/android/ion/ion.c:18:
   drivers/staging/android/ion/ion.c: In function 'ion_buffer_add':
>> arch/s390/include/asm/bug.h:54:26: error: expected expression before ')' token
     int __ret_warn_on = !!(x);   \
                             ^
   drivers/staging/android/ion/ion.c:69:4: note: in expansion of macro 'WARN_ON'
       WARN_ON();
       ^~~~~~~
--
   In file included from include/linux/bug.h:4:0,
                    from include/linux/mmdebug.h:4,
                    from arch/s390/include/asm/cmpxchg.h:10,
                    from arch/s390/include/asm/atomic.h:15,
                    from include/linux/atomic.h:4,
                    from include/linux/debug_locks.h:5,
                    from include/linux/lockdep.h:25,
                    from include/linux/spinlock_types.h:18,
                    from include/linux/mutex.h:15,
                    from include/linux/kernfs.h:13,
                    from include/linux/sysfs.h:15,
                    from include/linux/kobject.h:21,
                    from include/linux/device.h:17,
                    from drivers/staging//android/ion/ion.c:18:
   drivers/staging//android/ion/ion.c: In function 'ion_buffer_add':
>> arch/s390/include/asm/bug.h:54:26: error: expected expression before ')' token
     int __ret_warn_on = !!(x);   \
                             ^
   drivers/staging//android/ion/ion.c:69:4: note: in expansion of macro 'WARN_ON'
       WARN_ON();
       ^~~~~~~

vim +54 arch/s390/include/asm/bug.h

a9df8e32 arch/s390/include/asm/bug.h Heiko Carstens 2010-01-13  52  
c0007f1a include/asm-s390/bug.h      Heiko Carstens 2007-04-27  53  #define WARN_ON(x) ({					\
fd0cbdd3 include/asm-s390/bug.h      Heiko Carstens 2007-08-02 @54  	int __ret_warn_on = !!(x);			\
c0007f1a include/asm-s390/bug.h      Heiko Carstens 2007-04-27  55  	if (__builtin_constant_p(__ret_warn_on)) {	\
c0007f1a include/asm-s390/bug.h      Heiko Carstens 2007-04-27  56  		if (__ret_warn_on)			\
b2be0527 arch/s390/include/asm/bug.h Ben Hutchings  2010-04-03  57  			__WARN();			\
c0007f1a include/asm-s390/bug.h      Heiko Carstens 2007-04-27  58  	} else {					\
c0007f1a include/asm-s390/bug.h      Heiko Carstens 2007-04-27  59  		if (unlikely(__ret_warn_on))		\
b2be0527 arch/s390/include/asm/bug.h Ben Hutchings  2010-04-03  60  			__WARN();			\
c0007f1a include/asm-s390/bug.h      Heiko Carstens 2007-04-27  61  	}						\
c0007f1a include/asm-s390/bug.h      Heiko Carstens 2007-04-27  62  	unlikely(__ret_warn_on);			\
c0007f1a include/asm-s390/bug.h      Heiko Carstens 2007-04-27  63  })
c0007f1a include/asm-s390/bug.h      Heiko Carstens 2007-04-27  64  

:::::: The code at line 54 was first introduced by commit
:::::: fd0cbdd378258fdf44eac5ea091256a4a665315b Fix WARN_ON() on bitfield ops for all other archs

:::::: TO: Heiko Carstens <heiko.carstens@de.ibm.com>
:::::: CC: Linus Torvalds <torvalds@woody.linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47231 bytes --]

Re: [PATCH] Staging:android:ion:ion.c : Using WARN_ON() rather than BUG()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4532 bytes --]

Hi harsha,

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.13-rc7 next-20170831]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/harsha/Staging-android-ion-ion-c-Using-WARN_ON-rather-than-BUG/20170901-160600
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All error/warnings (new ones prefixed by >>):

   In file included from ./arch/xtensa/include/generated/asm/bug.h:1:0,
                    from include/linux/bug.h:4,
                    from include/linux/thread_info.h:11,
                    from arch/xtensa/include/asm/current.h:16,
                    from include/linux/mutex.h:13,
                    from include/linux/kernfs.h:13,
                    from include/linux/sysfs.h:15,
                    from include/linux/kobject.h:21,
                    from include/linux/device.h:17,
                    from drivers/staging/android/ion/ion.c:18:
   drivers/staging/android/ion/ion.c: In function 'ion_buffer_add':
>> include/asm-generic/bug.h:107:34: error: expected expression before ')' token
     int __ret_warn_on = !!(condition);    \
                                     ^
>> drivers/staging/android/ion/ion.c:69:4: note: in expansion of macro 'WARN_ON'
       WARN_ON();
       ^
--
   In file included from ./arch/xtensa/include/generated/asm/bug.h:1:0,
                    from include/linux/bug.h:4,
                    from include/linux/thread_info.h:11,
                    from arch/xtensa/include/asm/current.h:16,
                    from include/linux/mutex.h:13,
                    from include/linux/kernfs.h:13,
                    from include/linux/sysfs.h:15,
                    from include/linux/kobject.h:21,
                    from include/linux/device.h:17,
                    from drivers/staging//android/ion/ion.c:18:
   drivers/staging//android/ion/ion.c: In function 'ion_buffer_add':
>> include/asm-generic/bug.h:107:34: error: expected expression before ')' token
     int __ret_warn_on = !!(condition);    \
                                     ^
   drivers/staging//android/ion/ion.c:69:4: note: in expansion of macro 'WARN_ON'
       WARN_ON();
       ^

vim +/WARN_ON +69 drivers/staging/android/ion/ion.c

  > 18	#include <linux/device.h>
    19	#include <linux/err.h>
    20	#include <linux/file.h>
    21	#include <linux/freezer.h>
    22	#include <linux/fs.h>
    23	#include <linux/anon_inodes.h>
    24	#include <linux/kthread.h>
    25	#include <linux/list.h>
    26	#include <linux/memblock.h>
    27	#include <linux/miscdevice.h>
    28	#include <linux/export.h>
    29	#include <linux/mm.h>
    30	#include <linux/mm_types.h>
    31	#include <linux/rbtree.h>
    32	#include <linux/slab.h>
    33	#include <linux/seq_file.h>
    34	#include <linux/uaccess.h>
    35	#include <linux/vmalloc.h>
    36	#include <linux/debugfs.h>
    37	#include <linux/dma-buf.h>
    38	#include <linux/idr.h>
    39	#include <linux/sched/task.h>
    40	
    41	#include "ion.h"
    42	
    43	static struct ion_device *internal_dev;
    44	static int heap_id;
    45	
    46	bool ion_buffer_cached(struct ion_buffer *buffer)
    47	{
    48		return !!(buffer->flags & ION_FLAG_CACHED);
    49	}
    50	
    51	/* this function should only be called while dev->lock is held */
    52	static void ion_buffer_add(struct ion_device *dev,
    53				   struct ion_buffer *buffer)
    54	{
    55		struct rb_node **p = &dev->buffers.rb_node;
    56		struct rb_node *parent = NULL;
    57		struct ion_buffer *entry;
    58	
    59		while (*p) {
    60			parent = *p;
    61			entry = rb_entry(parent, struct ion_buffer, node);
    62	
    63			if (buffer < entry) {
    64				p = &(*p)->rb_left;
    65			} else if (buffer > entry) {
    66				p = &(*p)->rb_right;
    67			} else {
    68				pr_err("%s: buffer already found.", __func__);
  > 69				WARN_ON();
    70			}
    71		}
    72	
    73		rb_link_node(&buffer->node, parent, p);
    74		rb_insert_color(&buffer->node, &dev->buffers);
    75	}
    76	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50969 bytes --]

Re: [PATCH] Staging:android:ion:ion.c : Using WARN_ON() rather than BUG()

[Greg KH]

On Tue, Aug 29, 2017 at 11:30:22PM +0530, harsha wrote:
> Fixes checkpatch.pl warning: Use WARN_ON() rather than BUG_ON() and BUG()
> 
> Signed-off-by: harsha <harshasharmaiitr@gmail.com>

I still need a real name here.

> ---
>  drivers/staging/android/ion/ion.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 93e2c90..a2d36b3 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -66,7 +66,7 @@ static void ion_buffer_add(struct ion_device *dev,
>  			p = &(*p)->rb_right;
>  		} else {
>  			pr_err("%s: buffer already found.", __func__);
> -			BUG();
> +			WARN_ON();

You can't just change code without understanding _why_ you are changing
it.  You just changed the logic here, why do you think it is ok that
BUG() is no longer called?  Are you properly cleaning up and recovering
here now that WARN_ON() is called?

checkpatch.pl is a hint, you can't just blindly do whatever it says, you
still have to think.

thanks,

greg k-h

[PATCH] Staging:android:ion:ion.c : Using WARN_ON() rather than BUG()

[harsha]

Fixes checkpatch.pl warning: Use WARN_ON() rather than BUG_ON() and BUG()

Signed-off-by: harsha <harshasharmaiitr@gmail.com>
---
 drivers/staging/android/ion/ion.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 93e2c90..a2d36b3 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -66,7 +66,7 @@ static void ion_buffer_add(struct ion_device *dev,
 			p = &(*p)->rb_right;
 		} else {
 			pr_err("%s: buffer already found.", __func__);
-			BUG();
+			WARN_ON();
 		}
 	}
 
-- 
1.9.1

Re: [PATCH] Staging:android:ion:ion.c : Using WARN_ON() rather than BUG()

[Greg KH]

On Tue, Aug 29, 2017 at 10:49:54PM +0530, harsha wrote:
> Signed-off-by: harsha <harshasharmaiitr@gmail.com>
> ---
>  drivers/staging/android/ion/ion.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 93e2c90..a2d36b3 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -66,7 +66,7 @@ static void ion_buffer_add(struct ion_device *dev,
>  			p = &(*p)->rb_right;
>  		} else {
>  			pr_err("%s: buffer already found.", __func__);
> -			BUG();
> +			WARN_ON();
>  		}
>  	}
>  
> -- 
> 1.9.1


Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

- You did not use your real name.  Signed-off-by: and From: has to have
  a real name associated with it.  Use whatever you sign legal documents
  with, no anonymous patches are allowed.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

[PATCH] Staging:android:ion:ion.c : Using WARN_ON() rather than BUG()

[harsha]

Signed-off-by: harsha <harshasharmaiitr@gmail.com>
---
 drivers/staging/android/ion/ion.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 93e2c90..a2d36b3 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -66,7 +66,7 @@ static void ion_buffer_add(struct ion_device *dev,
 			p = &(*p)->rb_right;
 		} else {
 			pr_err("%s: buffer already found.", __func__);
-			BUG();
+			WARN_ON();
 		}
 	}
 
-- 
1.9.1