diff mbox series

[2/2] xfs: mark xfs_buf_free as might_sleep()

Message ID 20250320075221.1505190-3-hch@lst.de (mailing list archive)
State New
Headers show
Series [1/2] xfs: remove the leftover xfs_{set,clear}_li_failed infrastructure | expand

Commit Message

hch March 20, 2025, 7:52 a.m. UTC
xfs_buf_free can call vunmap, which can sleep.  The vunmap path is an
unlikely one, so add might_sleep to ensure calling xfs_buf_free from
atomic context gets caught more easily.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_buf.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Carlos Maiolino March 21, 2025, 9:21 a.m. UTC | #1
On Thu, Mar 20, 2025 at 08:52:14AM +0100, Christoph Hellwig wrote:
> xfs_buf_free can call vunmap, which can sleep.  The vunmap path is an
> unlikely one, so add might_sleep to ensure calling xfs_buf_free from
> atomic context gets caught more easily.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

> ---
>  fs/xfs/xfs_buf.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 8e7f1b324b3b..1a2b3f06fa71 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -105,6 +105,7 @@ xfs_buf_free(
>  {
>  	unsigned int		size = BBTOB(bp->b_length);
> 
> +	might_sleep();
>  	trace_xfs_buf_free(bp, _RET_IP_);
> 
>  	ASSERT(list_empty(&bp->b_lru));

If I followed it correct, vunmap can be caught via
xfs_buf_free_pages(). If that's the case, wouldn't make
more sense to put might_sleep() inside xfs_buf_free_pages()
giving it is not called only from xfs_buf_free()?

Otherwise,
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> --
> 2.45.2
>
hch March 23, 2025, 6:29 a.m. UTC | #2
On Fri, Mar 21, 2025 at 10:21:35AM +0100, Carlos Maiolino wrote:
> If I followed it correct, vunmap can be caught via
> xfs_buf_free_pages(). If that's the case, wouldn't make
> more sense to put might_sleep() inside xfs_buf_free_pages()
> giving it is not called only from xfs_buf_free()?

xfs_buf_free_pages has been folded into xfs_buf_free in for-next.
Carlos Maiolino March 23, 2025, 7:54 a.m. UTC | #3
On Sun, Mar 23, 2025 at 07:29:21AM +0100, Christoph Hellwig wrote:
> On Fri, Mar 21, 2025 at 10:21:35AM +0100, Carlos Maiolino wrote:
> > If I followed it correct, vunmap can be caught via
> > xfs_buf_free_pages(). If that's the case, wouldn't make
> > more sense to put might_sleep() inside xfs_buf_free_pages()
> > giving it is not called only from xfs_buf_free()?
> 
> xfs_buf_free_pages has been folded into xfs_buf_free in for-next.
> 

Woops, looked at the wrong code base... Thanks for the heads up.
diff mbox series

Patch

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 8e7f1b324b3b..1a2b3f06fa71 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -105,6 +105,7 @@  xfs_buf_free(
 {
 	unsigned int		size = BBTOB(bp->b_length);
 
+	might_sleep();
 	trace_xfs_buf_free(bp, _RET_IP_);
 
 	ASSERT(list_empty(&bp->b_lru));