From: Arnd Bergmann <arnd@kernel.org> To: Matthew Wilcox <willy@infradead.org> Cc: kernel test robot <lkp@intel.com>, llvm@lists.linux.dev, kbuild-all@lists.01.org, linux-kernel@vger.kernel.org Subject: Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio' Date: Mon, 15 Aug 2022 15:48:43 +0200 [thread overview] Message-ID: <CAK8P3a3=XC4ez-zeGptQb_9NtX5rVfr8Qq80eGaEckzUzjAxzg@mail.gmail.com> (raw) In-Reply-To: <YvpEkYMatTeNvNZa@casper.infradead.org> On Mon, Aug 15, 2022 at 3:05 PM Matthew Wilcox <willy@infradead.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@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. 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
WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@kernel.org> To: kbuild-all@lists.01.org Subject: Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio' Date: Mon, 15 Aug 2022 15:48:43 +0200 [thread overview] Message-ID: <CAK8P3a3=XC4ez-zeGptQb_9NtX5rVfr8Qq80eGaEckzUzjAxzg@mail.gmail.com> (raw) In-Reply-To: <YvpEkYMatTeNvNZa@casper.infradead.org> [-- Attachment #1: Type: text/plain, Size: 4992 bytes --] On Mon, Aug 15, 2022 at 3:05 PM Matthew Wilcox <willy@infradead.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@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. 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
next prev parent reply other threads:[~2022-08-15 13:49 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-08-14 0:21 fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio' kernel test robot 2022-08-15 12:29 ` Matthew Wilcox 2022-08-15 12:29 ` Matthew Wilcox 2022-08-15 12:56 ` Arnd Bergmann 2022-08-15 12:56 ` Arnd Bergmann 2022-08-15 13:05 ` Matthew Wilcox 2022-08-15 13:05 ` Matthew Wilcox 2022-08-15 13:48 ` Arnd Bergmann [this message] 2022-08-15 13:48 ` Arnd Bergmann 2022-08-15 14:37 ` Arnd Bergmann 2022-08-15 14:37 ` Arnd Bergmann 2022-08-15 17:11 ` Nathan Chancellor 2022-08-15 17:11 ` Nathan Chancellor
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAK8P3a3=XC4ez-zeGptQb_9NtX5rVfr8Qq80eGaEckzUzjAxzg@mail.gmail.com' \ --to=arnd@kernel.org \ --cc=kbuild-all@lists.01.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lkp@intel.com \ --cc=llvm@lists.linux.dev \ --cc=willy@infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.