[linux-next:master 12891/13017] mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation

[linux-next:master 12891/13017] mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation

[kbuild test robot]

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

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   2bba65ab5f9f1cebd21d95c410b96952851f58b3
commit: e191357c4c31d02eb30736a49327ef32407fab47 [12891/13017] slub: create new ___slab_alloc function that can be called with irqs disabled
config: s390-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout e191357c4c31d02eb30736a49327ef32407fab47
        # save the attached .config to linux build tree
        make.cross ARCH=s390 

All warnings (new ones prefixed by >>):

   mm/slub.c: In function 'unfreeze_partials.isra.42':
   mm/slub.c:2019:1: warning: 'unfreeze_partials.isra.42' uses dynamic stack allocation
    }
    ^
   mm/slub.c: In function 'get_partial_node.isra.43':
   mm/slub.c:1654:1: warning: 'get_partial_node.isra.43' uses dynamic stack allocation
    }
    ^
   mm/slub.c: In function 'deactivate_slab':
   mm/slub.c:1951:1: warning: 'deactivate_slab' uses dynamic stack allocation
    }
    ^
   mm/slub.c: In function '__slab_free':
   mm/slub.c:2696:1: warning: '__slab_free' uses dynamic stack allocation
    }
    ^
   mm/slub.c: In function '___slab_alloc':
>> mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation
    }
    ^

vim +/___slab_alloc +2396 mm/slub.c

71c7a06f Christoph Lameter 2008-02-14  2380  		return NULL;
497b66f2 Christoph Lameter 2011-08-09  2381  	}
2cfb7455 Christoph Lameter 2011-06-01  2382  
f6e7def7 Christoph Lameter 2012-05-09  2383  	page = c->page;
5091b74a Christoph Lameter 2012-07-31  2384  	if (likely(!kmem_cache_debug(s) && pfmemalloc_match(page, gfpflags)))
497b66f2 Christoph Lameter 2011-08-09  2385  		goto load_freelist;
497b66f2 Christoph Lameter 2011-08-09  2386  
497b66f2 Christoph Lameter 2011-08-09  2387  	/* Only entered in the debug case */
d0e0ac97 Chen Gang         2013-07-15  2388  	if (kmem_cache_debug(s) &&
d0e0ac97 Chen Gang         2013-07-15  2389  			!alloc_debug_processing(s, page, freelist, addr))
497b66f2 Christoph Lameter 2011-08-09  2390  		goto new_slab;	/* Slab failed checks. Next slab needed */
894b8788 Christoph Lameter 2007-05-10  2391  
f6e7def7 Christoph Lameter 2012-05-09  2392  	deactivate_slab(s, page, get_freepointer(s, freelist));
c17dda40 Christoph Lameter 2012-05-09  2393  	c->page = NULL;
c17dda40 Christoph Lameter 2012-05-09  2394  	c->freelist = NULL;
6faa6833 Christoph Lameter 2012-05-09  2395  	return freelist;
894b8788 Christoph Lameter 2007-05-10 @2396  }
894b8788 Christoph Lameter 2007-05-10  2397  
894b8788 Christoph Lameter 2007-05-10  2398  /*
e191357c Christoph Lameter 2015-11-11  2399   * Another one that disabled interrupt and compensates for possible
e191357c Christoph Lameter 2015-11-11  2400   * cpu changes by refetching the per cpu area pointer.
e191357c Christoph Lameter 2015-11-11  2401   */
e191357c Christoph Lameter 2015-11-11  2402  static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
e191357c Christoph Lameter 2015-11-11  2403  			  unsigned long addr, struct kmem_cache_cpu *c)
e191357c Christoph Lameter 2015-11-11  2404  {

:::::: The code at line 2396 was first introduced by commit
:::::: 894b8788d7f265eb7c6f75a9a77cedeb48f51586 slub: support concurrent local and remote frees and allocs on a slab

:::::: TO: Christoph Lameter <clameter@sgi.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/octet-stream, Size: 39189 bytes --]

Re: [linux-next:master 12891/13017] mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation

[Andrew Morton]

On Wed, 11 Nov 2015 14:34:19 +0800 kbuild test robot <fengguang.wu@intel.com> wrote:

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head:   2bba65ab5f9f1cebd21d95c410b96952851f58b3
> commit: e191357c4c31d02eb30736a49327ef32407fab47 [12891/13017] slub: create new ___slab_alloc function that can be called with irqs disabled
> config: s390-allmodconfig (attached as .config)
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         git checkout e191357c4c31d02eb30736a49327ef32407fab47
>         # save the attached .config to linux build tree
>         make.cross ARCH=s390 
> 
> All warnings (new ones prefixed by >>):
> 
>    mm/slub.c: In function 'unfreeze_partials.isra.42':
>    mm/slub.c:2019:1: warning: 'unfreeze_partials.isra.42' uses dynamic stack allocation
>     }
>     ^
>    mm/slub.c: In function 'get_partial_node.isra.43':
>    mm/slub.c:1654:1: warning: 'get_partial_node.isra.43' uses dynamic stack allocation
>     }
>     ^
>    mm/slub.c: In function 'deactivate_slab':
>    mm/slub.c:1951:1: warning: 'deactivate_slab' uses dynamic stack allocation
>     }
>     ^
>    mm/slub.c: In function '__slab_free':
>    mm/slub.c:2696:1: warning: '__slab_free' uses dynamic stack allocation
>     }
>     ^
>    mm/slub.c: In function '___slab_alloc':
> >> mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation
>     }
>     ^

This patch doesn't add any dynamic stack allocations.  The fact that
slub.c already had a bunch of these warnings makes me suspect that it's
happening in one of the s390 headers?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [linux-next:master 12891/13017] mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation

[Martin Schwidefsky]

On Wed, 11 Nov 2015 12:41:08 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 11 Nov 2015 14:34:19 +0800 kbuild test robot <fengguang.wu@intel.com> wrote:
> 
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > head:   2bba65ab5f9f1cebd21d95c410b96952851f58b3
> > commit: e191357c4c31d02eb30736a49327ef32407fab47 [12891/13017] slub: create new ___slab_alloc function that can be called with irqs disabled
> > config: s390-allmodconfig (attached as .config)
> > reproduce:
> >         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         git checkout e191357c4c31d02eb30736a49327ef32407fab47
> >         # save the attached .config to linux build tree
> >         make.cross ARCH=s390 
> > 
> > All warnings (new ones prefixed by >>):
> > 
> >    mm/slub.c: In function 'unfreeze_partials.isra.42':
> >    mm/slub.c:2019:1: warning: 'unfreeze_partials.isra.42' uses dynamic stack allocation
> >     }
> >     ^
> >    mm/slub.c: In function 'get_partial_node.isra.43':
> >    mm/slub.c:1654:1: warning: 'get_partial_node.isra.43' uses dynamic stack allocation
> >     }
> >     ^
> >    mm/slub.c: In function 'deactivate_slab':
> >    mm/slub.c:1951:1: warning: 'deactivate_slab' uses dynamic stack allocation
> >     }
> >     ^
> >    mm/slub.c: In function '__slab_free':
> >    mm/slub.c:2696:1: warning: '__slab_free' uses dynamic stack allocation
> >     }
> >     ^
> >    mm/slub.c: In function '___slab_alloc':
> > >> mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation
> >     }
> >     ^
> 
> This patch doesn't add any dynamic stack allocations.  The fact that
> slub.c already had a bunch of these warnings makes me suspect that it's
> happening in one of the s390 headers?
 
That looks like a false positive to me. I can not find any function that does
a dynamic allocation and the generated code creates a stack frame with a
constant size. A bit odd is the fact that the stack frame is create in two
steps, e.g. deactivate_slab:

    a632:       b9 04 00 ef             lgr     %r14,%r15
    a636:       a7 fb ff 50             aghi    %r15,-176	# first 176 bytes
    a63a:       b9 04 00 bf             lgr     %r11,%r15
    a63e:       e3 e0 f0 98 00 24       stg     %r14,152(%r15)
    a644:       e3 10 f0 98 00 04       lg      %r1,152(%r15)
    a64a:       a7 fb ff 30             aghi    %r15,-208	# another 208 bytes
    a64e:       e3 30 b0 e8 00 24       stg     %r3,232(%r11)
    a654:       e3 40 b0 d8 00 24       stg     %r4,216(%r11)

Strange. Andreas can you make something of this?

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [linux-next:master 12891/13017] mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation

[Andreas Krebbel1]

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

mschwid2@linux.vnet.ibm.com wrote on 11/13/2015 12:52:00 PM:
> > This patch doesn't add any dynamic stack allocations.  The fact that
> > slub.c already had a bunch of these warnings makes me suspect that 
it's
> > happening in one of the s390 headers?
> 
> That looks like a false positive to me. I can not find any function that 
does
> a dynamic allocation and the generated code creates a stack frame with a
> constant size. A bit odd is the fact that the stack frame is create in 
two
> steps, e.g. deactivate_slab:
> 
>     a632:       b9 04 00 ef             lgr     %r14,%r15
>     a636:       a7 fb ff 50             aghi    %r15,-176   # first 176 
bytes
>     a63a:       b9 04 00 bf             lgr     %r11,%r15
>     a63e:       e3 e0 f0 98 00 24       stg     %r14,152(%r15)
>     a644:       e3 10 f0 98 00 04       lg      %r1,152(%r15)
>     a64a:       a7 fb ff 30             aghi    %r15,-208   # 
> another 208 bytes
>     a64e:       e3 30 b0 e8 00 24       stg     %r3,232(%r11)
>     a654:       e3 40 b0 d8 00 24       stg     %r4,216(%r11)
> 
> Strange. Andreas can you make something of this?

Hi Martin,

this appears to be the result of aligning struct page to more than 8 bytes 
and putting it onto the stack - wich is only 8 bytes aligned.  The 
compiler has to perform runtime alignment to achieve that. It allocates 
memory using *alloca* and does the math with the returned pointer. Our 
dynamic stack allocation option basically only checks if there is an 
alloca user.

We have added that -mwarn-dynamicstack option because our runtime check 
(-mstack-guard) is only able to deal with the static stack allocations. So 
with dynamic stack allocations you might still overwrite stuff without the 
runtime stack guard being able to catch it. So in one regard the warning 
is correct since there in fact is a stack allocation which will not be 
covered by the stack guard. One the other hand the additional stack 
allocation is done with a constant value so it is not really dynamic and 
the warning message is not really helpful. Perhaps we should emit warnings 
whenever there are stack allocations present which aren't covered by the 
stack guard? Or should we silently ignore such a case?

-Andreas-


[-- Attachment #2: Type: text/html, Size: 3040 bytes --]

Re: [linux-next:master 12891/13017] mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation

[Christoph Lameter]

On Fri, 13 Nov 2015, Andreas Krebbel1 wrote:

> this appears to be the result of aligning struct page to more than 8 bytes
> and putting it onto the stack - wich is only 8 bytes aligned.  The
> compiler has to perform runtime alignment to achieve that. It allocates
> memory using *alloca* and does the math with the returned pointer. Our
> dynamic stack allocation option basically only checks if there is an
> alloca user.

The slub uses of struct page only require an alignment of the page struct
on the stack to a word. So its fine.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [linux-next:master 12891/13017] mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation

[Andreas Krebbel1]

> On Fri, 13 Nov 2015, Andreas Krebbel1 wrote:
> 
> > this appears to be the result of aligning struct page to more than 8 
bytes
> > and putting it onto the stack - wich is only 8 bytes aligned.  The
> > compiler has to perform runtime alignment to achieve that. It 
allocates
> > memory using *alloca* and does the math with the returned pointer. Our
> > dynamic stack allocation option basically only checks if there is an
> > alloca user.
> 
> The slub uses of struct page only require an alignment of the page 
struct
> on the stack to a word. So its fine.

Our compare and swap double hardware instruction unfortunately requires 16 
byte alignment. That's probably the reason why this alignment has been 
picked. So I don't think that we can easily get rid of it.

-Andreas-

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [linux-next:master 12891/13017] mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation

[Christoph Lameter]

On Fri, 13 Nov 2015, Andreas Krebbel1 wrote:

> > The slub uses of struct page only require an alignment of the page
> struct
> > on the stack to a word. So its fine.
>
> Our compare and swap double hardware instruction unfortunately requires 16
> byte alignment. That's probably the reason why this alignment has been
> picked. So I don't think that we can easily get rid of it.

The cmpxchg double is not run on the page struct on the stack. Its just
used because I wanted to duplicate the counter layout.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [linux-next:master 12891/13017] mm/slub.c:2396:1: warning: '___slab_alloc' uses dynamic stack allocation

[Martin Schwidefsky]

On Fri, 13 Nov 2015 16:13:46 +0100
"Andreas Krebbel1" <Andreas.Krebbel@de.ibm.com> wrote:

> this appears to be the result of aligning struct page to more than 8 bytes 
> and putting it onto the stack - wich is only 8 bytes aligned.  The 
> compiler has to perform runtime alignment to achieve that. It allocates 
> memory using *alloca* and does the math with the returned pointer. Our 
> dynamic stack allocation option basically only checks if there is an 
> alloca user.

I can confirm that this is caused by the struct page alignment, if I
force HAVE_ALIGNED_STRUCT_PAGE=n the warning vanishes.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>