Message-ID: <CAK8P3a3=XC4ez-zeGptQb_9NtX5rVfr8Qq80eGaEckzUzjAxzg@mail.gmail.com>
Date:   Mon, 15 Aug 2022 15:48:43 +0200
From:   Arnd Bergmann <arnd@...nel.org>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     kernel test robot <lkp@...el.com>, llvm@...ts.linux.dev,
        kbuild-all@...ts.01.org, linux-kernel@...r.kernel.org
Subject: Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds
 limit (1024) in 'ntfs_read_folio'

On Mon, Aug 15, 2022 at 3:05 PM Matthew Wilcox <willy@...radead.org> wrote:
> On Mon, Aug 15, 2022 at 02:56:11PM +0200, Arnd Bergmann wrote:
> > On Mon, Aug 15, 2022 at 2:29 PM Matthew Wilcox <willy@...radead.org> wrote:
> > >
> > > On Sun, Aug 14, 2022 at 08:21:36AM +0800, kernel test robot wrote:
> > > > Hi Matthew,
> > > >
> > > > FYI, the error/warning still remains.
> > >
> > > FYI, this is still not interesting.
> > > This is a hexagon 256kB PAGE_SIZE config, and so the amount of stack
> > > space is correspondingly larger.  The frame size warning should be
> > > increased to allow for this.
> > >
> > > > >> fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio' [-Wframe-larger-than]
> >
> > I don't think we should change the frame size warning for this, there is not
> > generally any correlation between page size and stack usage, so that would
> > just hide bugs elsewhere.
>
> In this specific case, there is.  It's a stack allocation of an array
> that depends on the number of 512-byte blocks per page.  With 4k pages,
> that's only 8.  With 256k pages, that's 512.  With an 8-byte pointer,
> that's a 4kB allocation, and even with a 4-byte pointer, that's a 2kB
> stack allocation, which is still going to blow the prescribed stack
> limit.
>
> This is not unique to NTFS!  An NTFS-specific "fix" is inappropriate.
> It's just that nobody's paying attention to the warnings coming from
> fs/buffer.c:
>
> include/linux/buffer_head.h:#define MAX_BUF_PER_PAGE (PAGE_SIZE / 512)
>
> int block_read_full_folio(struct folio *folio, get_block_t *get_block)
> {
> ...
>         struct buffer_head *bh, *head, *arr[MAX_BUF_PER_PAGE];
>
> I don't know why I'm not getting a nastygram about that one, but it's
> all bufferhead based filesystems.

I can confirm I see this warning with 256KB pages, in block_read_full_folio()
and others:

fs/mpage.c:131:20: error: stack frame size (4200) exceeds limit (1024)
in 'do_mpage_readpage' [-Werror,-Wframe-larger-than]
fs/mpage.c:447:12: error: stack frame size (4336) exceeds limit (1024)
in '__mpage_writepage' [-Werror,-Wframe-larger-than]
fs/buffer.c:2254:5: error: stack frame size (2152) exceeds limit
(1024) in 'block_read_full_folio' [-Werror,-Wframe-larger-than]
fs/fat/dir.c:1133:5: error: stack frame size (2104) exceeds limit
(1024) in 'fat_alloc_new_dir' [-Werror,-Wframe-larger-than]
fs/fat/fatent.c:466:5: error: stack frame size (2216) exceeds limit
(1024) in 'fat_alloc_clusters' [-Werror,-Wframe-larger-than]
fs/fat/fatent.c:554:5: error: stack frame size (2168) exceeds limit
(1024) in 'fat_free_clusters' [-Werror,-Wframe-larger-than]
fs/fat/dir.c:1281:5: error: stack frame size (2232) exceeds limit
(1024) in 'fat_add_entries' [-Werror,-Wframe-larger-than]
fs/ntfs3/fsntfs.c:738:5: error: stack frame size (2112) exceeds limit
(1024) in 'ntfs_clear_mft_tail' [-Werror,-Wframe-larger-than]
fs/ext4/move_extent.c:252:1: error: stack frame size (2272) exceeds
limit (1024) in 'move_extent_per_page' [-Werror,-Wframe-larger-than]
fs/ext4/readpage.c:223:5: error: stack frame size (4280) exceeds limit
(1024) in 'ext4_mpage_readpages' [-Werror,-Wframe-larger-than]

I still think that raising the warning limit here is not appropriate, having
a 512 element array of pointers on the stack is just not appropriate anywhere
IMHO.

> > NTFS has had problems with stack usage on 64K+ pages before, the last
> > time we addressed this using 4eec7faf6775 ("fs: ntfs: Limit NTFS_RW to
> > page sizes smaller than 64k"), but it looks like this time it affects both
> > write and read support.
>
> The reasoning there is faulty.  If you have a 64k (or 256k) page size,
> your stack is correspondingly huge and can handle these kinds of
> allocations.

I think that is only the case for VMAP stacks, which require full pages,
but configurations without that use the "thread_stack" kmem_cache
for allocating stacks when THREAD_SIZE is smaller than PAGE_SIZE.

The THREAD_SIZE on Hexagon is 4KB, so do_mpage_readpage()
with 4200 bytes would immediately overflow that. Obviously 4KB stacks
are problematic already and only supported as options on sh and m68k
otherwise, but raising it to the usual 8KB would likely still cause the same
problem.

I have no problems with a patch removing support for 256KB pages if that
helps, as Hexagon is the only architecture to support this and there are close
to zero Linux users anway. This would leave only three warnings for 64KB
pages in allmodconfig:

fs/mpage.c:131:20: error: stack frame size (1128) exceeds limit (1024)
in 'do_mpage_readpage' [-Werror,-Wframe-larger-than]
fs/mpage.c:447:12: error: stack frame size (1264) exceeds limit (1024)
in '__mpage_writepage' [-Werror,-Wframe-larger-than]
fs/ext4/readpage.c:223:5: error: stack frame size (1208) exceeds limit
(1024) in 'ext4_mpage_readpages' [-Werror,-Wframe-larger-than]

        Arnd

Powered by blists - more mailing lists