Re: [PATCH v11 02/16] xfs: only call xfs_setsize_buftarg once per buffer target
From: John Garry <john.g.garry@oracle.com>
Date: 2025-05-05 15:28:26
Also in:
linux-block, linux-ext4, linux-fsdevel, linux-xfs, lkml
Subsystem:
filesystems (vfs and infrastructure), the rest, xfs filesystem · Maintainers:
Alexander Viro, Christian Brauner, Linus Torvalds, Carlos Maiolino
On 05/05/2025 15:48, John Garry wrote:
quoted
quoted
@Darrick, please comment on whether happy with changes discussed.I put the sync_blockdev calls in a separate function so that the EIO/ENOSPC/whatever errors that come from the block device sync don't get morphed into ENOMEM by xfs_alloc_buftarg before being passed up. I suppose we could make that function return an ERR_PTR, but I was trying to avoid making even more changes at the last minute, again.It seems simpler to just have the individual sync_blockdev() calls from xfs_alloc_buftarg(), rather than adding ERR_PTR() et al handling in both xfs_alloc_buftarg() and xfs_open_devices().
Which of the following is better: ------8<----- int
@@ -1810,10 +1806,10 @@ xfs_alloc_buftarg( * When allocating the buftargs we have not yet read the super block and * thus don't know the file system sector size yet. */ - if (xfs_setsize_buftarg(btp, bdev_logical_block_size(btp->bt_bdev))) - goto error_free; - if (xfs_init_buftarg(btp, bdev_logical_block_size(btp->bt_bdev), - mp->m_super->s_id)) + btp->bt_meta_sectorsize = bdev_logical_block_size(btp->bt_bdev); + btp->bt_meta_sectormask = btp->bt_meta_sectorsize - 1; + + if (xfs_init_buftarg(btp, btp->bt_meta_sectorsize, mp->m_super->s_id)) goto error_free; return btp;
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 5e456a6073ca..48d5b630fe46 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c@@ -481,21 +481,38 @@ xfs_open_devices( /* * Setup xfs_mount buffer target pointers + * + * Flush and invalidate all devices' pagecaches before reading any + * metadata because XFS doesn't use the bdev pagecache. */ - error = -ENOMEM; mp->m_ddev_targp = xfs_alloc_buftarg(mp, sb->s_bdev_file); - if (!mp->m_ddev_targp) + if (!mp->m_ddev_targp) { + error = -ENOMEM; + goto out_close_rtdev; + } + error = sync_blockdev(mp->m_ddev_targp->bt_bdev); + if (error) goto out_close_rtdev; if (rtdev_file) { mp->m_rtdev_targp = xfs_alloc_buftarg(mp, rtdev_file); - if (!mp->m_rtdev_targp) + if (!mp->m_rtdev_targp) { + error = -ENOMEM; + goto out_free_ddev_targ; + } + error = sync_blockdev(mp->m_rtdev_targp->bt_bdev); + if (error) goto out_free_ddev_targ; } if (logdev_file && file_bdev(logdev_file) != ddev) { mp->m_logdev_targp = xfs_alloc_buftarg(mp, logdev_file); - if (!mp->m_logdev_targp) + if (!mp->m_logdev_targp) { + error = -ENOMEM; + goto out_free_rtdev_targ; + } + error = sync_blockdev(mp->m_logdev_targp->bt_bdev); + if (error) goto out_free_rtdev_targ; ----->8------
int
@@ -1786,6 +1782,8 @@ xfs_alloc_buftarg( { struct xfs_buftarg *btp; const struct dax_holder_operations *ops = NULL; + int error; + #if defined(CONFIG_FS_DAX) && defined(CONFIG_MEMORY_FAILURE) ops = &xfs_dax_holder_operations;
@@ -1806,21 +1804,31 @@ xfs_alloc_buftarg( btp->bt_bdev); } + /* + * Flush and invalidate all devices' pagecaches before reading any + * metadata because XFS doesn't use the bdev pagecache. + */ + error = sync_blockdev(mp->m_ddev_targp->bt_bdev); + if (error) + goto error_free; + /* * When allocating the buftargs we have not yet read the super block and * thus don't know the file system sector size yet. */ - if (xfs_setsize_buftarg(btp, bdev_logical_block_size(btp->bt_bdev))) - goto error_free; - if (xfs_init_buftarg(btp, bdev_logical_block_size(btp->bt_bdev), - mp->m_super->s_id)) + btp->bt_meta_sectorsize = bdev_logical_block_size(btp->bt_bdev); + btp->bt_meta_sectormask = btp->bt_meta_sectorsize - 1; + + if (xfs_init_buftarg(btp, btp->bt_meta_sectorsize, mp->m_super->s_id)) { + error = -ENOMEM; goto error_free; + } return btp; error_free: kfree(btp); - return NULL; + return ERR_PTR(error); } static inline void
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 5e456a6073ca..4daf0cc480af 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c@@ -482,21 +482,26 @@ xfs_open_devices( /* * Setup xfs_mount buffer target pointers */ - error = -ENOMEM; mp->m_ddev_targp = xfs_alloc_buftarg(mp, sb->s_bdev_file); - if (!mp->m_ddev_targp) + if (IS_ERR(mp->m_ddev_targp)) { + error = PTR_ERR(mp->m_ddev_targp); goto out_close_rtdev; + } if (rtdev_file) { mp->m_rtdev_targp = xfs_alloc_buftarg(mp, rtdev_file); - if (!mp->m_rtdev_targp) + if (IS_ERR(mp->m_rtdev_targp)) { + error = PTR_ERR(mp->m_rtdev_targp); goto out_free_ddev_targ; + } } if (logdev_file && file_bdev(logdev_file) != ddev) { mp->m_logdev_targp = xfs_alloc_buftarg(mp, logdev_file); - if (!mp->m_logdev_targp) + if (IS_ERR(mp->m_logdev_targp)) { + error = PTR_ERR(mp->m_logdev_targp); goto out_free_rtdev_targ; + } ----8<-----