[bug report] xfs: use vmalloc instead of vm_map_area for buffer backing memory

[bug report] xfs: use vmalloc instead of vm_map_area for buffer backing memory

[Dan Carpenter]

Hello Christoph Hellwig,

Commit e2874632a621 ("xfs: use vmalloc instead of vm_map_area for
buffer backing memory") from Mar 10, 2025 (linux-next), leads to the
following Smatch static checker warning:

	fs/xfs/xfs_buf.c:210 xfs_buf_free()
	warn: sleeping in atomic context

fs/xfs/xfs_buf.c
    196 static void
    197 xfs_buf_free(
    198         struct xfs_buf                *bp)
    199 {
    200         unsigned int                size = BBTOB(bp->b_length);
    201 
    202         trace_xfs_buf_free(bp, _RET_IP_);
    203 
    204         ASSERT(list_empty(&bp->b_lru));
    205 
    206         if (!xfs_buftarg_is_mem(bp->b_target) && size >= PAGE_SIZE)
    207                 mm_account_reclaimed_pages(howmany(size, PAGE_SHIFT));
    208 
    209         if (is_vmalloc_addr(bp->b_addr))
--> 210                 vfree(bp->b_addr);

vfree() can sleep.  Although it's fine to call it in interrupt context.

    211         else if (bp->b_flags & _XBF_KMEM)
    212                 kfree(bp->b_addr);
    213         else
    214                 folio_put(virt_to_folio(bp->b_addr));
    215 
    216         call_rcu(&bp->b_rcu, xfs_buf_free_callback);
    217 }

These warnings tend to have a lot of false positives because the call
tree is long.  There are two functions which call xfs_clear_li_failed()
while holding a spinlock.  These are the call trees.

xfs_trans_ail_delete() <- disables preempt
xfs_qm_dqflush_done() <- disables preempt
-> xfs_clear_li_failed()
   -> xfs_buf_rele()
      -> xfs_buf_rele_uncached()
         -> xfs_buf_free()

regards,
dan carpenter

Re: [bug report] xfs: use vmalloc instead of vm_map_area for buffer backing memory

[Christoph Hellwig]

Hi Dan,

thanks for the report!

Even before we really would not want to call xfs_buf_rele with
locks held (or other preemption disabling), so I'll look into fixing
that.

Re: [bug report] xfs: use vmalloc instead of vm_map_area for buffer backing memory

[Christoph Hellwig]

On Tue, Mar 18, 2025 at 11:40:47AM +0300, Dan Carpenter wrote:
> Hello Christoph Hellwig,
> 
> Commit e2874632a621 ("xfs: use vmalloc instead of vm_map_area for
> buffer backing memory") from Mar 10, 2025 (linux-next), leads to the
> following Smatch static checker warning:

Just a question to reconfirm how smatch works:  the vm_unmap_ram
replaced by vfree in this patch also had a might_sleep(), so I think
this bug is older and the check should have also triggeted before.
Or am I missing something?

Re: [bug report] xfs: use vmalloc instead of vm_map_area for buffer backing memory

[Dan Carpenter]

On Wed, Mar 19, 2025 at 07:31:13AM +0100, Christoph Hellwig wrote:
> On Tue, Mar 18, 2025 at 11:40:47AM +0300, Dan Carpenter wrote:
> > Hello Christoph Hellwig,
> > 
> > Commit e2874632a621 ("xfs: use vmalloc instead of vm_map_area for
> > buffer backing memory") from Mar 10, 2025 (linux-next), leads to the
> > following Smatch static checker warning:
> 
> Just a question to reconfirm how smatch works:  the vm_unmap_ram
> replaced by vfree in this patch also had a might_sleep(), so I think
> this bug is older and the check should have also triggeted before.
> Or am I missing something?

Oh, yeah, sorry.  It did trigger before but I never reported it.
It only showed up as a new bug because the warning moved from
xfs_buf_free_pages() to xfs_buf_free().

Sorry!

regards,
dan carpenter