Re: [PATCH v3 1/6] btrfs: use an on-stack path in btrfs_insert_orphan_item()

[David Sterba]

On Thu, Aug 20, 2026 at 08:49:52AM -0400, Jeff Layton wrote:
> On Thu, 2026-08-20 at 14:04 +0200, David Sterba wrote:
> > On Tue, Aug 11, 2026 at 02:14:54PM -0400, Jeff Layton wrote:
> > > btrfs_insert_orphan_item() allocated a btrfs_path with btrfs_alloc_path()
> > > which returns -ENOMEM on failure. It is called from btrfs_orphan_add(),
> > > so a path allocation failure there turns a recoverable error into a
> > > transaction abort.
> > > 
> > > btrfs_path is only ~112 bytes, so allocate it on the stack instead.
> > 
> > 112 is too much for on-stack, we've avoided that for btrfs_path in
> > particular, except some justified cases. This means in general the
> > beginning of call stack like ioctl, syscall handler and such. Otherwise
> > we assume there are other layers in the IO stack, like block device
> > drivers (DM), NFS, encoding layers or networking (iscsi), and obviously
> > the lowest level device drivers.
> > 
> > The trade off with possible allocation failure vs stack consumption
> > needs to be argued in the changelog, "is just 112" is not sufficient.
> >
> > Getting back the consumed stack space is painful, we've been reducing
> > unneeded or redundant parameters of functions for years. The gains are
> > like -8 bytes here and -8 bytes there, allocation of +112 wipes that out.
> > 
> > If the place of allocation is critical we can consider that but we have
> > too many of them, anywhere during the transaction commit path or
> > irreversible metadata changes. Possibly using __GFP_HIGH could work, but
> > I haven't explored that.
> > 
> > Qu added the patches to for-next but I had no chance to look closely at
> > this patchset yet and am hesitant to leave it like that.
> 
> Fair critique. These days I think we're mostly ~16k stacks, but it can
> certainly get deep given the right layering.
> 
> In this case, failing this allocation can lead to aborting the fs, so
> it seemed justified. This is a GFP_NOFS allocation too, so it can
> easily fail, even though it's small.

The MM guys say that small NOFS allocations are technically "nofail",
but with cgroup limit it can fail under normal circumstances too. We got
reports from syzbot and other tools that can trigger the memory
allocation failures in places difficult to handle (e.g. extent state
tree changes) but this practically never happens.