On 05/05/2025 15:48, John Garry wrote:
@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<-----
