Re: [PATCH net 1/3] net/rds: don't use unpin_user_pages_dirty_lock() from atomic context

On Wed, 2026-07-22 at 09:43 -0700, Jakub Kicinski wrote:
> On Fri, 10 Jul 2026 19:51:16 -0700 Allison Henderson wrote:
> > rds_rdma_free_op() and rds_atomic_free_op() are reached from the IB
> > send completion path via
> > 
> >   rds_ib_tasklet_fn_send()
> >     rds_ib_send_cqe_handler()
> >       rds_message_put()
> >         rds_message_purge()
> >           rds_rdma_free_op() / rds_atomic_free_op()
> 
> Sashiko points out that this just moves the problem to the filesystem,
> which also may have mutexes:
> 
>   Does calling set_page_dirty() from softirq context introduce a
>   potential deadlock?
> 
>   If the user pages are file-backed, set_page_dirty() will route to filesystem
>   dirty callbacks like filemap_dirty_folio() or block_dirty_folio(). These
>   callbacks acquire non-irq-safe spinlocks, such as inode->i_lock or
>   mapping->i_private_lock. If the tasklet interrupts a process holding one of
>   these locks, could the softirq spin forever?
> 
>   Additionally, does calling set_page_dirty() here without holding the folio
>   lock or an active file reference risk a use-after-free?
> 
>   If a concurrent file truncation removes the page from the page cache and
>   frees its mapping, and set_page_dirty() reads folio->mapping just before
>   it is zeroed, could it dereference a freed pointer when calling the
>   dirty_folio callback or __mark_inode_dirty()? Should this work be
>   deferred to a process context workqueue instead?
> 
> Is this not true, or not true in most cases? Otherwise we should find a
> better fix?

After a bit of digging, I think its uncommon, but true.  I'll see if I can find
come up with an alternate fix.  I think we can use a work item to defer the
rds_message_purge() when needed.  Then the worker can use the correct locked
variant unpin_user_pages_dirty_lock().  I'll send a v2 when I get it worked out.

Thanks!
Allison