Re: [PATCH v3 3/3] iova: defer maple tree erase on GFP_ATOMIC failure

[Jason Gunthorpe <jgg@ziepe.ca>]

On Thu, Jun 18, 2026 at 01:27:02PM -0400, Liam R. Howlett wrote:
> On 26/06/18 12:24PM, Jason Gunthorpe wrote:
> > On Thu, Jun 18, 2026 at 10:50:56AM -0400, Liam R. Howlett wrote:
> > 
> > > > If that's the case it should be documented like this too :)
> > > 
> > > Yeah, very much should add some notes here... but also maybe stop them
> > > from doing it by adding this to mas_erase():
> > > 
> > > if (mt_external_lock(mas->tree))
> > >         might_alloc(GFP_KERNEL);
> > 
> > Yeah, and then I'd add a might_alloc() to mtree_erase() as well, it
> > can always sleep..
> 
> mtree_erase() calls mas_erase(), so I think the one should be enough.

I was thinking an unconditional one before locking:

@@ -5919,6 +5919,8 @@ void *mtree_erase(struct maple_tree *mt, unsigned long index)
        MA_STATE(mas, mt, index, index);
        trace_ma_op(TP_FCT, &mas);
 
+       might_alloc(GFP_KERNEL);
+
        mtree_lock(mt);

Since with internal locking this can still sleep, and it has to be
called in a sleepable context.

The one in mas_erase() is conditional on external locking and is
basically asserting the external lock is non-atomic.

> > Okay, so to summarize:
> > 
> > - mas_erase, mtree_erase cannot fail with ENOMEM. The check for ENOMEM
> >   inside should be changed to a WARN_ON to document this.
> >   Rational: in a GFP_KERNEL context it will sleep forever until it
> >   gets memory "too small to fail"
> 
> The ENOMEM check inside mas_nomem() is still possible because we try
> NOWAIT first, so it is possible that we hit a retry.
> 
> The return from mas_nomem() without allocations should be impossible.
> So if (!mas->sheaf && !mas->alloc) should be a WARN_ON_ONCE().
> 
> I believe this is what you meant anyways?

Yeah, probably, or maybe this:

	if (mas_is_err(mas))
		goto out;

Can it ever be anything but -ENOMEM within mas_erase()?
 
> So cleaning them up should be handled sooner rather than later.  I'm
> trying to figure out the best place to do this as I do not like the
> retry with the timer idea.

I would clean them up when gap searching to allocate a new thing. That
is a safe place to fail and ensures the gaps are correct before
checking them.

Jason