Re: [PATCH] mm/zswap.c: add BUG() for default case in zswap_writeback_entry()

[Chen Gang <gang.chen.5i5j@gmail.com>]

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

On 12/09/2013 06:18 PM, James Hogan wrote:
> On 09/12/13 10:11, Chen Gang wrote:
>>> Since the metag compiler is stuck on an old version (gcc 4.2.4), which
>>> is wrong to warn in this case, and newer versions of gcc don't appear to
>>> warn about it anyway (I just checked with gcc 4.7.2 x86_64), I have no
>>> objection to this warning remaining in the metag build.
>>>
>>
>> Do you try "EXTRA_CFLAGS=-W" with gcc 4.7.2? I guess it will report the
>> warning too, I don't feel the compiler is smart enough (except it lets
>> the long function zswap_get_swap_cache_page really inline)  :-)
> 
> EXTRA_CFLAGS=-W on gcc 4.7.2 gives me plenty of pointless unused
> parameter warnings when compiling mm/zswap.o, but not the warning you're
> trying to silence.
> 

Yeah, it will generate plenty of pointless warnings, although we still
can often find valuable bugs in these warnings.

Oh, I tried gcc 4.6.3-2 rhel version, get the same result as yours (do
not report warning), but for me, it is still a compiler's bug, it
*should* report a warning for it, we can try below:

 - modify zswap_get_swap_cache_page() to let it may return another value
   (one sample modification is in attachment)

 - compile again, it doesn't report related warning, either

 - in this case, it *should* report related warning.

Could you help to try it under gcc 4.7.2, thanks?


BTW: gcc really exists some bugs about uninitialized variable, e.g.

  one known bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=18501
  kernel related: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57856


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water and life which God blessed

[-- Attachment #2: diff.patch --]
[-- Type: text/x-patch, Size: 345 bytes --]

diff --git a/mm/zswap.c b/mm/zswap.c
index 5a63f78..1853ef4 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -469,8 +469,7 @@ static int zswap_get_swap_cache_page(swp_entry_t entry,
 		 */
 		err = radix_tree_preload(GFP_KERNEL);
 		if (err)
-			break;
-
+			return -4;
 		/*
 		 * Swap entry may have been freed since our caller observed it.
 		 */