Re: [PATCH v7 09/43] btrfs: add infrastructure for safe em freeing

[Daniel Vacek]

On Tue, 2 Jun 2026 at 04:56, Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
> On Wed, May 13, 2026 at 10:52:43AM +0200, Daniel Vacek wrote:
> > +/*
> > + * Drop a ref for the extent map in the given tree.
> > + *
> > + * @tree:    tree that the em is a part of.
> > + * @em:              the em to drop the reference to.
> > + *
> > + * Drop the reference count on @em by one, if the reference count hits 0 and
> > + * there is an object on the em that can't be safely freed in the current
> > + * context (if we are holding the extent_map_tree->lock for example), then add
> > + * it to the freed_extents list on the extent_map_tree for later processing.
> > + *
> > + * This must be followed by a btrfs_free_pending_extent_maps() to clear
> > + * the pending frees.
> > + */
> > +void btrfs_free_extent_map_safe(struct extent_map_tree *tree,
> > +                             struct extent_map *em)
>
> The "unsafe" btrfs_free_extent_map() still exists.  Assuming both
> variants will continue to be needed, shouldn't the comment above the
> "unsafe" variant be updated to document the preconditions for using it?

Actually this could be cleaned up and unified into a single function
with optional argument.

> > diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
> > index 6f685f3c9327..a962012be1c3 100644
> > --- a/fs/btrfs/extent_map.h
> > +++ b/fs/btrfs/extent_map.h
> > @@ -97,11 +97,18 @@ struct extent_map {
> >       u32 flags;
> >       refcount_t refs;
> >       struct list_head list;
> > +     struct list_head free_list;
> > +};
>
> The comment above this struct says "Keep this structure as compact as
> possible, as we can have really large amounts of allocated extent maps
> at any time."  Is there any way to avoid adding a whole list_head?

Good catch. In fact we can reuse the old list member. No need for a new one.

--nX

> - Eric