| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <200d855d-550d-4207-9118-6a0c10d14f8a@oracle.com>
Date: Mon, 5 May 2025 16:27:56 +0100
From: John Garry <john.g.garry@...cle.com>
To: "Darrick J. Wong" <djwong@...nel.org>, Christoph Hellwig <hch@....de>
Cc: brauner@...nel.org, viro@...iv.linux.org.uk, jack@...e.cz, cem@...nel.org,
linux-fsdevel@...r.kernel.org, dchinner@...hat.com,
linux-xfs@...r.kernel.org, linux-kernel@...r.kernel.org,
ojaswin@...ux.ibm.com, ritesh.list@...il.com,
martin.petersen@...cle.com, linux-ext4@...r.kernel.org,
linux-block@...r.kernel.org, catherine.hoang@...cle.com,
linux-api@...r.kernel.org
Subject: Re: [PATCH v11 02/16] xfs: only call xfs_setsize_buftarg once per
buffer target
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<-----
Powered by blists - more mailing lists