All of lore.kernel.org
 help / color / mirror / Atom feed
* fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
@ 2022-08-14  0:21 kernel test robot
  2022-08-15 12:29   ` Matthew Wilcox
  0 siblings, 1 reply; 13+ messages in thread
From: kernel test robot @ 2022-08-14  0:21 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle); +Cc: llvm, kbuild-all, linux-kernel

Hi Matthew,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   f6eb0fed6a3957c0b93e3a00c1ffaad84d4ffc31
commit: 933906f8e8e4110c56db9bddd1281e4e4983a2bb ntfs: Convert ntfs to read_folio
date:   3 months ago
config: hexagon-buildonly-randconfig-r001-20220814 (https://download.01.org/0day-ci/archive/20220814/202208140835.W6F1j6Da-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 3329cec2f79185bafd678f310fafadba2a8c76d2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=933906f8e8e4110c56db9bddd1281e4e4983a2bb
        git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
        git fetch --no-tags linus master
        git checkout 933906f8e8e4110c56db9bddd1281e4e4983a2bb
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash fs/ntfs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio' [-Wframe-larger-than]
   static int ntfs_read_folio(struct file *file, struct folio *folio)
              ^
   1 warning generated.


vim +/ntfs_read_folio +378 fs/ntfs/aops.c

   359	
   360	/**
   361	 * ntfs_read_folio - fill a @folio of a @file with data from the device
   362	 * @file:	open file to which the folio @folio belongs or NULL
   363	 * @folio:	page cache folio to fill with data
   364	 *
   365	 * For non-resident attributes, ntfs_read_folio() fills the @folio of the open
   366	 * file @file by calling the ntfs version of the generic block_read_full_folio()
   367	 * function, ntfs_read_block(), which in turn creates and reads in the buffers
   368	 * associated with the folio asynchronously.
   369	 *
   370	 * For resident attributes, OTOH, ntfs_read_folio() fills @folio by copying the
   371	 * data from the mft record (which at this stage is most likely in memory) and
   372	 * fills the remainder with zeroes. Thus, in this case, I/O is synchronous, as
   373	 * even if the mft record is not cached at this point in time, we need to wait
   374	 * for it to be read in before we can do the copy.
   375	 *
   376	 * Return 0 on success and -errno on error.
   377	 */
 > 378	static int ntfs_read_folio(struct file *file, struct folio *folio)
   379	{
   380		struct page *page = &folio->page;
   381		loff_t i_size;
   382		struct inode *vi;
   383		ntfs_inode *ni, *base_ni;
   384		u8 *addr;
   385		ntfs_attr_search_ctx *ctx;
   386		MFT_RECORD *mrec;
   387		unsigned long flags;
   388		u32 attr_len;
   389		int err = 0;
   390	
   391	retry_readpage:
   392		BUG_ON(!PageLocked(page));
   393		vi = page->mapping->host;
   394		i_size = i_size_read(vi);
   395		/* Is the page fully outside i_size? (truncate in progress) */
   396		if (unlikely(page->index >= (i_size + PAGE_SIZE - 1) >>
   397				PAGE_SHIFT)) {
   398			zero_user(page, 0, PAGE_SIZE);
   399			ntfs_debug("Read outside i_size - truncated?");
   400			goto done;
   401		}
   402		/*
   403		 * This can potentially happen because we clear PageUptodate() during
   404		 * ntfs_writepage() of MstProtected() attributes.
   405		 */
   406		if (PageUptodate(page)) {
   407			unlock_page(page);
   408			return 0;
   409		}
   410		ni = NTFS_I(vi);
   411		/*
   412		 * Only $DATA attributes can be encrypted and only unnamed $DATA
   413		 * attributes can be compressed.  Index root can have the flags set but
   414		 * this means to create compressed/encrypted files, not that the
   415		 * attribute is compressed/encrypted.  Note we need to check for
   416		 * AT_INDEX_ALLOCATION since this is the type of both directory and
   417		 * index inodes.
   418		 */
   419		if (ni->type != AT_INDEX_ALLOCATION) {
   420			/* If attribute is encrypted, deny access, just like NT4. */
   421			if (NInoEncrypted(ni)) {
   422				BUG_ON(ni->type != AT_DATA);
   423				err = -EACCES;
   424				goto err_out;
   425			}
   426			/* Compressed data streams are handled in compress.c. */
   427			if (NInoNonResident(ni) && NInoCompressed(ni)) {
   428				BUG_ON(ni->type != AT_DATA);
   429				BUG_ON(ni->name_len);
   430				return ntfs_read_compressed_block(page);
   431			}
   432		}
   433		/* NInoNonResident() == NInoIndexAllocPresent() */
   434		if (NInoNonResident(ni)) {
   435			/* Normal, non-resident data stream. */
   436			return ntfs_read_block(page);
   437		}
   438		/*
   439		 * Attribute is resident, implying it is not compressed or encrypted.
   440		 * This also means the attribute is smaller than an mft record and
   441		 * hence smaller than a page, so can simply zero out any pages with
   442		 * index above 0.  Note the attribute can actually be marked compressed
   443		 * but if it is resident the actual data is not compressed so we are
   444		 * ok to ignore the compressed flag here.
   445		 */
   446		if (unlikely(page->index > 0)) {
   447			zero_user(page, 0, PAGE_SIZE);
   448			goto done;
   449		}
   450		if (!NInoAttr(ni))
   451			base_ni = ni;
   452		else
   453			base_ni = ni->ext.base_ntfs_ino;
   454		/* Map, pin, and lock the mft record. */
   455		mrec = map_mft_record(base_ni);
   456		if (IS_ERR(mrec)) {
   457			err = PTR_ERR(mrec);
   458			goto err_out;
   459		}
   460		/*
   461		 * If a parallel write made the attribute non-resident, drop the mft
   462		 * record and retry the read_folio.
   463		 */
   464		if (unlikely(NInoNonResident(ni))) {
   465			unmap_mft_record(base_ni);
   466			goto retry_readpage;
   467		}
   468		ctx = ntfs_attr_get_search_ctx(base_ni, mrec);
   469		if (unlikely(!ctx)) {
   470			err = -ENOMEM;
   471			goto unm_err_out;
   472		}
   473		err = ntfs_attr_lookup(ni->type, ni->name, ni->name_len,
   474				CASE_SENSITIVE, 0, NULL, 0, ctx);
   475		if (unlikely(err))
   476			goto put_unm_err_out;
   477		attr_len = le32_to_cpu(ctx->attr->data.resident.value_length);
   478		read_lock_irqsave(&ni->size_lock, flags);
   479		if (unlikely(attr_len > ni->initialized_size))
   480			attr_len = ni->initialized_size;
   481		i_size = i_size_read(vi);
   482		read_unlock_irqrestore(&ni->size_lock, flags);
   483		if (unlikely(attr_len > i_size)) {
   484			/* Race with shrinking truncate. */
   485			attr_len = i_size;
   486		}
   487		addr = kmap_atomic(page);
   488		/* Copy the data to the page. */
   489		memcpy(addr, (u8*)ctx->attr +
   490				le16_to_cpu(ctx->attr->data.resident.value_offset),
   491				attr_len);
   492		/* Zero the remainder of the page. */
   493		memset(addr + attr_len, 0, PAGE_SIZE - attr_len);
   494		flush_dcache_page(page);
   495		kunmap_atomic(addr);
   496	put_unm_err_out:
   497		ntfs_attr_put_search_ctx(ctx);
   498	unm_err_out:
   499		unmap_mft_record(base_ni);
   500	done:
   501		SetPageUptodate(page);
   502	err_out:
   503		unlock_page(page);
   504		return err;
   505	}
   506	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
  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
  0 siblings, 0 replies; 13+ messages in thread
From: Matthew Wilcox @ 2022-08-15 12:29 UTC (permalink / raw)
  To: kernel test robot; +Cc: llvm, kbuild-all, linux-kernel

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]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
@ 2022-08-15 12:29   ` Matthew Wilcox
  0 siblings, 0 replies; 13+ messages in thread
From: Matthew Wilcox @ 2022-08-15 12:29 UTC (permalink / raw)
  To: kbuild-all

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

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]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
  2022-08-15 12:29   ` Matthew Wilcox
@ 2022-08-15 12:56     ` Arnd Bergmann
  -1 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2022-08-15 12:56 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: kernel test robot, llvm, kbuild-all, linux-kernel

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.

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.

I checked hexagon ntfs with 64KB pages, and that stays below the 1024 byte
limit, so we could add another dependency

--- a/fs/ntfs/Kconfig
+++ b/fs/ntfs/Kconfig
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config NTFS_FS
        tristate "NTFS file system support"
+      depends on PAGE_SIZE_LESS_THAN_256KB
        select NLS
        help
          NTFS is the file system of Microsoft Windows NT, 2000, XP and 2003.

to rule out the broken case but still allow powerpc64 and arm64 with
64KB to use NTFS.

      Arnd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
@ 2022-08-15 12:56     ` Arnd Bergmann
  0 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2022-08-15 12:56 UTC (permalink / raw)
  To: kbuild-all

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

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.

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.

I checked hexagon ntfs with 64KB pages, and that stays below the 1024 byte
limit, so we could add another dependency

--- a/fs/ntfs/Kconfig
+++ b/fs/ntfs/Kconfig
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config NTFS_FS
        tristate "NTFS file system support"
+      depends on PAGE_SIZE_LESS_THAN_256KB
        select NLS
        help
          NTFS is the file system of Microsoft Windows NT, 2000, XP and 2003.

to rule out the broken case but still allow powerpc64 and arm64 with
64KB to use NTFS.

      Arnd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
  2022-08-15 12:56     ` Arnd Bergmann
@ 2022-08-15 13:05       ` Matthew Wilcox
  -1 siblings, 0 replies; 13+ messages in thread
From: Matthew Wilcox @ 2022-08-15 13:05 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: kernel test robot, llvm, kbuild-all, linux-kernel

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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
@ 2022-08-15 13:05       ` Matthew Wilcox
  0 siblings, 0 replies; 13+ messages in thread
From: Matthew Wilcox @ 2022-08-15 13:05 UTC (permalink / raw)
  To: kbuild-all

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

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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
  2022-08-15 13:05       ` Matthew Wilcox
@ 2022-08-15 13:48         ` Arnd Bergmann
  -1 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2022-08-15 13:48 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: kernel test robot, llvm, kbuild-all, linux-kernel

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
@ 2022-08-15 13:48         ` Arnd Bergmann
  0 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2022-08-15 13:48 UTC (permalink / raw)
  To: kbuild-all

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
  2022-08-15 13:48         ` Arnd Bergmann
@ 2022-08-15 14:37           ` Arnd Bergmann
  -1 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2022-08-15 14:37 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: kernel test robot, llvm, kbuild-all, linux-kernel

On Mon, Aug 15, 2022 at 3:48 PM Arnd Bergmann <arnd@kernel.org> wrote:

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

I looked into these a bit more and found that these are arrays of sector_t,
which could be either 32-bit or 64-bit wide before 72deb455b5ec
("block: remove CONFIG_LBDAF"), but is now always 64-bit, so having
an array of 128 of these (65536/512) adds a 1KB to the stack and will
cause a warning. It's only slightly over the limit, and there are very few
32-bit systems that allow 64KB pages to trigger that warning.

I see now that ppc440 also supports 256KB pages and has the same
problem as hexagon, but also has been broken since the start of the
git history in this regard:

fs/mpage.c:638:1: error: the frame size of 4280 bytes is larger than
2048 bytes [-Werror=frame-larger-than=]

I don't know if anyone strongly cares about 256KB pages on
ppc44x any more, but given this, I'm fairly sure that they are
not using block based file systems. So we could just make
CONFIG_BLOCK depend on PAGE_SIZE_LESS_THAN_256KB
globally instead of dropping 256KB pages everywhere.

        Arnd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
@ 2022-08-15 14:37           ` Arnd Bergmann
  0 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2022-08-15 14:37 UTC (permalink / raw)
  To: kbuild-all

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

On Mon, Aug 15, 2022 at 3:48 PM Arnd Bergmann <arnd@kernel.org> wrote:

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

I looked into these a bit more and found that these are arrays of sector_t,
which could be either 32-bit or 64-bit wide before 72deb455b5ec
("block: remove CONFIG_LBDAF"), but is now always 64-bit, so having
an array of 128 of these (65536/512) adds a 1KB to the stack and will
cause a warning. It's only slightly over the limit, and there are very few
32-bit systems that allow 64KB pages to trigger that warning.

I see now that ppc440 also supports 256KB pages and has the same
problem as hexagon, but also has been broken since the start of the
git history in this regard:

fs/mpage.c:638:1: error: the frame size of 4280 bytes is larger than
2048 bytes [-Werror=frame-larger-than=]

I don't know if anyone strongly cares about 256KB pages on
ppc44x any more, but given this, I'm fairly sure that they are
not using block based file systems. So we could just make
CONFIG_BLOCK depend on PAGE_SIZE_LESS_THAN_256KB
globally instead of dropping 256KB pages everywhere.

        Arnd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
  2022-08-15 14:37           ` Arnd Bergmann
@ 2022-08-15 17:11             ` Nathan Chancellor
  -1 siblings, 0 replies; 13+ messages in thread
From: Nathan Chancellor @ 2022-08-15 17:11 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Matthew Wilcox, kernel test robot, llvm, kbuild-all, linux-kernel

On Mon, Aug 15, 2022 at 04:37:09PM +0200, Arnd Bergmann wrote:
> On Mon, Aug 15, 2022 at 3:48 PM Arnd Bergmann <arnd@kernel.org> wrote:
> 
> > 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

Right, I had brought up at least adjusting the dependencies of 256KB
pages so that it could not be selected with CONFIG_COMPILE_TEST to
reduce the number of warnings that would appear in randconfigs.

https://lore.kernel.org/YoAlvnyjEbYV4T1L@dev-arch.thelio-3990X/

I suspect removing it outright would be fine too.

> > 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]
> 
> I looked into these a bit more and found that these are arrays of sector_t,
> which could be either 32-bit or 64-bit wide before 72deb455b5ec
> ("block: remove CONFIG_LBDAF"), but is now always 64-bit, so having
> an array of 128 of these (65536/512) adds a 1KB to the stack and will
> cause a warning. It's only slightly over the limit, and there are very few
> 32-bit systems that allow 64KB pages to trigger that warning.
> 
> I see now that ppc440 also supports 256KB pages and has the same
> problem as hexagon, but also has been broken since the start of the
> git history in this regard:
> 
> fs/mpage.c:638:1: error: the frame size of 4280 bytes is larger than
> 2048 bytes [-Werror=frame-larger-than=]
> 
> I don't know if anyone strongly cares about 256KB pages on
> ppc44x any more, but given this, I'm fairly sure that they are
> not using block based file systems. So we could just make
> CONFIG_BLOCK depend on PAGE_SIZE_LESS_THAN_256KB
> globally instead of dropping 256KB pages everywhere.

That doesn't sound like an unreasonable solution.

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: fs/ntfs/aops.c:378:12: warning: stack frame size (2216) exceeds limit (1024) in 'ntfs_read_folio'
@ 2022-08-15 17:11             ` Nathan Chancellor
  0 siblings, 0 replies; 13+ messages in thread
From: Nathan Chancellor @ 2022-08-15 17:11 UTC (permalink / raw)
  To: kbuild-all

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

On Mon, Aug 15, 2022 at 04:37:09PM +0200, Arnd Bergmann wrote:
> On Mon, Aug 15, 2022 at 3:48 PM Arnd Bergmann <arnd@kernel.org> wrote:
> 
> > 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

Right, I had brought up at least adjusting the dependencies of 256KB
pages so that it could not be selected with CONFIG_COMPILE_TEST to
reduce the number of warnings that would appear in randconfigs.

https://lore.kernel.org/YoAlvnyjEbYV4T1L(a)dev-arch.thelio-3990X/

I suspect removing it outright would be fine too.

> > 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]
> 
> I looked into these a bit more and found that these are arrays of sector_t,
> which could be either 32-bit or 64-bit wide before 72deb455b5ec
> ("block: remove CONFIG_LBDAF"), but is now always 64-bit, so having
> an array of 128 of these (65536/512) adds a 1KB to the stack and will
> cause a warning. It's only slightly over the limit, and there are very few
> 32-bit systems that allow 64KB pages to trigger that warning.
> 
> I see now that ppc440 also supports 256KB pages and has the same
> problem as hexagon, but also has been broken since the start of the
> git history in this regard:
> 
> fs/mpage.c:638:1: error: the frame size of 4280 bytes is larger than
> 2048 bytes [-Werror=frame-larger-than=]
> 
> I don't know if anyone strongly cares about 256KB pages on
> ppc44x any more, but given this, I'm fairly sure that they are
> not using block based file systems. So we could just make
> CONFIG_BLOCK depend on PAGE_SIZE_LESS_THAN_256KB
> globally instead of dropping 256KB pages everywhere.

That doesn't sound like an unreasonable solution.

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-08-15 17:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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.