Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'

[Matthew Wilcox <willy@infradead.org>]

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@infradead.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.

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