All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Andrey Albershteyn <aalbersh@kernel.org>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 04/10] libxfs: unify buftarg handling with the kernel
Date: Fri, 25 Sep 2026 15:37:16 -0700	[thread overview]
Message-ID: <20260925223716.GS2705364@frogsfrogsfrogs> (raw)
In-Reply-To: <20260925051336.2997014-5-hch@lst.de>

On Fri, Sep 25, 2026 at 07:12:50AM +0200, Christoph Hellwig wrote:
> In the kernel, the RT buftarg is set to NULL when not realtime device is
> present, but libxfs still allocates one, and all users instead check for
> a NULL ->bt_bdev.
> 
> Switch to the kernel model of never allocating the buftarg, and check
> mp->m_rtdev_targp for NULL instead of just ->bt_bdev.

Might want to mention that xfsprogs now matches what the kernel does.

> Signed-off-by: Christoph Hellwig <hch@lst.de>

With that added in,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  db/io.c         |  4 ++--
>  db/metadump.c   |  7 +++----
>  libxfs/init.c   | 40 +++++++++++++++++++---------------------
>  libxfs/rdwr.c   | 25 ++++++++++++++++---------
>  mkfs/xfs_mkfs.c |  6 ++----
>  5 files changed, 42 insertions(+), 40 deletions(-)
> 
> diff --git a/db/io.c b/db/io.c
> index fc9a25d41024..6aafe5bd8a15 100644
> --- a/db/io.c
> +++ b/db/io.c
> @@ -651,7 +651,7 @@ set_log_cur(
>  	int		ring_flag,
>  	bbmap_t		*bbmap)
>  {
> -	if (mp->m_logdev_targp->bt_bdev == mp->m_ddev_targp->bt_bdev) {
> +	if (mp->m_logdev_targp == mp->m_ddev_targp) {
>  		fprintf(stderr, "no external log specified\n");
>  		exitcode = 1;
>  		return;
> @@ -668,7 +668,7 @@ set_rt_cur(
>  	int		ring_flag,
>  	bbmap_t		*bbmap)
>  {
> -	if (!mp->m_rtdev_targp->bt_bdev) {
> +	if (!mp->m_rtdev_targp) {
>  		printf(_("realtime device not loaded, use -R.\n"));
>  		return ENODEV;
>  	}
> diff --git a/db/metadump.c b/db/metadump.c
> index 39639a0d51b0..6c12a5270fea 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -3344,8 +3344,7 @@ write_metadump_v2(
>  	uint64_t		addr;
>  
>  	addr = off;
> -	if (type == TYP_LOG &&
> -	    mp->m_logdev_targp->bt_bdev != mp->m_ddev_targp->bt_bdev)
> +	if (type == TYP_LOG && mp->m_logdev_targp != mp->m_ddev_targp)
>  		addr |= XME_ADDR_LOG_DEVICE;
>  	else if (type == TYP_RTSB)
>  		addr |= XME_ADDR_RT_DEVICE;
> @@ -3464,7 +3463,7 @@ metadump_f(
>  		return 0;
>  	}
>  
> -	if (mp->m_logdev_targp->bt_bdev != mp->m_ddev_targp->bt_bdev)
> +	if (mp->m_logdev_targp != mp->m_ddev_targp)
>  		metadump.external_log = true;
>  
>  	if (metadump.external_log && !version_opt_set)
> @@ -3481,7 +3480,7 @@ metadump_f(
>  	 * enabled.
>  	 */
>  	if (xfs_has_realtime(mp) && xfs_has_rtsb(mp)) {
> -		if (mp->m_rtdev_targp->bt_bdev) {
> +		if (mp->m_rtdev_targp) {
>  			metadump.realtime_data = true;
>  			if (!version_opt_set)
>  				metadump.version = 2;
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 1613ce2d14a3..65dadb1a23f7 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -307,7 +307,7 @@ rtmount_init(
>  		return -1;
>  	}
>  
> -	if (mp->m_rtdev_targp->bt_bdev == 0 && !xfs_is_debugger(mp)) {
> +	if (!mp->m_rtdev_targp && !xfs_is_debugger(mp)) {
>  		fprintf(stderr, _("%s: filesystem has a realtime subvolume\n"),
>  			progname);
>  		return -1;
> @@ -541,27 +541,26 @@ libxfs_buftarg_reinit(
>  		exit(1);
>  	}
>  
> -	if (!xi->log.dev || xi->log.dev == xi->data.dev) {
> -		if (mp->m_logdev_targp != mp->m_ddev_targp) {
> +	if (mp->m_logdev_targp != mp->m_ddev_targp) {
> +		if ((xi->log.dev && xi->log.dev == xi->data.dev) ||
> +		    mp->m_logdev_targp->bt_bdev != xi->log.dev ||
> +		    mp->m_logdev_targp->bt_mount != mp) {
>  			fprintf(stderr,
> -			_("%s: bad buftarg reinit, ldev mismatch\n"),
> +				_("%s: bad buftarg reinit, logdev\n"),
>  				progname);
>  			exit(1);
>  		}
> -	} else if (mp->m_logdev_targp->bt_bdev != xi->log.dev ||
> -		   mp->m_logdev_targp->bt_mount != mp) {
> -		fprintf(stderr,
> -			_("%s: bad buftarg reinit, logdev\n"),
> -			progname);
> -		exit(1);
>  	}
> -	if ((xi->rt.dev || xi->rt.dev == xi->data.dev) &&
> -	    (mp->m_rtdev_targp->bt_bdev != xi->rt.dev ||
> -	     mp->m_rtdev_targp->bt_mount != mp)) {
> -		fprintf(stderr,
> -			_("%s: bad buftarg reinit, rtdev\n"),
> -			progname);
> -		exit(1);
> +
> +	if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp) {
> +		if ((xi->rt.dev && xi->rt.dev == xi->data.dev) ||
> +		    mp->m_rtdev_targp->bt_bdev != xi->rt.dev ||
> +		    mp->m_rtdev_targp->bt_mount != mp) {
> +			fprintf(stderr,
> +				_("%s: bad buftarg reinit, rtdev\n"),
> +				progname);
> +			exit(1);
> +		}
>  	}
>  }
>  
> @@ -687,7 +686,7 @@ check_many_rtgroups(
>  	xfs_daddr_t		d;
>  	int			error;
>  
> -	if (!mp->m_rtdev_targp->bt_bdev) {
> +	if (!mp->m_rtdev_targp) {
>  		fprintf(stderr, _("%s: no rt device, ignoring rgcount %u\n"),
>  				progname, sbp->sb_rgcount);
>  		if (!xfs_is_debugger(mp))
> @@ -824,8 +823,7 @@ libxfs_mount(
>  	} else
>  		libxfs_buf_relse(bp);
>  
> -	if (mp->m_logdev_targp->bt_bdev &&
> -	    mp->m_logdev_targp->bt_bdev != mp->m_ddev_targp->bt_bdev) {
> +	if (mp->m_logdev_targp != mp->m_ddev_targp) {
>  		d = (xfs_daddr_t) XFS_FSB_TO_BB(mp, mp->m_sb.sb_logblocks);
>  		if (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_logblocks ||
>  		    libxfs_buf_read(mp->m_logdev_targp,
> @@ -1045,7 +1043,7 @@ libxfs_umount(
>  	free(mp->m_fsname);
>  	mp->m_fsname = NULL;
>  
> -	if (mp->m_rtdev_targp != mp->m_ddev_targp)
> +	if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp)
>  		libxfs_buftarg_free(mp->m_rtdev_targp);
>  	if (mp->m_logdev_targp != mp->m_ddev_targp)
>  		libxfs_buftarg_free(mp->m_logdev_targp);
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 90f2d56687ca..14cfec7a2859 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -171,7 +171,7 @@ libxfs_getrtsb(
>  	struct xfs_buf		*bp;
>  	int			error;
>  
> -	if (!mp->m_rtdev_targp->bt_bdev)
> +	if (!mp->m_rtdev_targp)
>  		return NULL;
>  
>  	ASSERT(!mp->m_sb.sb_rtstart);
> @@ -1028,7 +1028,8 @@ libxfs_bcache_purge(struct xfs_mount *mp)
>  		return;
>  	cache_purge(mp->m_ddev_targp->bcache);
>  	cache_purge(mp->m_logdev_targp->bcache);
> -	cache_purge(mp->m_rtdev_targp->bcache);
> +	if (mp->m_rtdev_targp)
> +		cache_purge(mp->m_rtdev_targp->bcache);
>  }
>  
>  void
> @@ -1038,7 +1039,8 @@ libxfs_bcache_flush(struct xfs_mount *mp)
>  		return;
>  	cache_flush(mp->m_ddev_targp->bcache);
>  	cache_flush(mp->m_logdev_targp->bcache);
> -	cache_flush(mp->m_rtdev_targp->bcache);
> +	if (mp->m_rtdev_targp)
> +		cache_flush(mp->m_rtdev_targp->bcache);
>  }
>  
>  int
> @@ -1046,7 +1048,8 @@ libxfs_bcache_overflowed(struct xfs_mount *mp)
>  {
>  	return cache_overflowed(mp->m_ddev_targp->bcache) ||
>  		cache_overflowed(mp->m_logdev_targp->bcache) ||
> -		cache_overflowed(mp->m_rtdev_targp->bcache);
> +		(mp->m_rtdev_targp &&
> +		 cache_overflowed(mp->m_rtdev_targp->bcache));
>  }
>  
>  struct cache_operations libxfs_bcache_operations = {
> @@ -1185,15 +1188,19 @@ libxfs_log_clear(
>  	xfs_daddr_t		end_blk;
>  	char			*ptr;
>  
> -	if (((btp && dptr) || (!btp && !dptr)) ||
> -	    (btp && !btp->bt_bdev) || !fs_uuid)
> +	if (!fs_uuid)
>  		return -EINVAL;
>  
> -	/* first zero the log */
> -	if (btp)
> +	if (btp) {
> +		if (dptr)
> +			return -EINVAL;
> +		/* first zero the log */
>  		libxfs_device_zero(btp, start, length);
> -	else
> +	} else {
> +		if (!dptr)
> +			return -EINVAL;
>  		memset(dptr, 0, BBTOB(length));
> +	}
>  
>  	/*
>  	 * Initialize the log record length and LSNs. XLOG_INIT_CYCLE is a
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 3ce49a36eaa8..6eca76f0c3c9 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -5632,10 +5632,8 @@ prepare_devices(
>  			 &sbp->sb_uuid, cfg->sb_feat.log_version,
>  			 lsunit, XLOG_FMT, XLOG_INIT_CYCLE, false);
>  	/* finally, check we can write the last block in the realtime area */
> -	if (mp->m_rtdev_targp->bt_bdev &&
> -	    mp->m_rtdev_targp != mp->m_ddev_targp &&
> -	    cfg->rtblocks > 0 &&
> -	    !xfs_has_zoned(mp)) {
> +	if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp &&
> +	    cfg->rtblocks > 0 && !xfs_has_zoned(mp)) {
>  		buf = alloc_write_buf(mp->m_rtdev_targp,
>  				XFS_FSB_TO_BB(mp, cfg->rtblocks - 1LL),
>  				BTOBB(cfg->blocksize));
> -- 
> 2.53.0
> 
> 

  reply	other threads:[~2026-09-25 22:37 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-25  5:12 fix missing RT detection in libxfs v2 Christoph Hellwig
2026-09-25  5:12 ` [PATCH 01/10] libxfs: refactor the reinit case in libxfs_buftarg_init Christoph Hellwig
2026-09-25 22:32   ` Darrick J. Wong
2026-09-25  5:12 ` [PATCH 02/10] libxfs: split error injection option parsing out of libxfs_buftarg_init Christoph Hellwig
2026-09-25 22:33   ` Darrick J. Wong
2026-09-25  5:12 ` [PATCH 03/10] libxfs: remove buftarg member aliases Christoph Hellwig
2026-09-25  5:12 ` [PATCH 04/10] libxfs: unify buftarg handling with the kernel Christoph Hellwig
2026-09-25 22:37   ` Darrick J. Wong [this message]
2026-09-26  5:59     ` Christoph Hellwig
2026-09-27 15:18       ` Darrick J. Wong
2026-09-25  5:12 ` [PATCH 05/10] libxfs: make the size check in rtmount_init internal RT device aware Christoph Hellwig
2026-09-25 22:38   ` Darrick J. Wong
2026-09-25  5:12 ` [PATCH 06/10] libxfs: don't clear ->dev in libxfs_device_close Christoph Hellwig
2026-09-25 22:39   ` Darrick J. Wong
2026-09-25  5:12 ` [PATCH 07/10] libxfs: fix buftarg initialization for the RT device Christoph Hellwig
2026-09-25 22:42   ` Darrick J. Wong
2026-09-25  5:12 ` [PATCH 08/10] libxfs: rename bt_bdev* Christoph Hellwig
2026-09-25 22:43   ` Darrick J. Wong
2026-09-25  5:12 ` [PATCH 09/10] libfrog: improve ramdisk handling in platform_flush_device Christoph Hellwig
2026-09-25 22:47   ` Darrick J. Wong
2026-09-26  6:02     ` Christoph Hellwig
2026-09-27 15:22       ` Darrick J. Wong
2026-09-25  5:12 ` [PATCH 10/10] xfs_db: remove a double-whitespace in rdump_regfile_data Christoph Hellwig
2026-09-25 22:47   ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260925223716.GS2705364@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=aalbersh@kernel.org \
    --cc=hch@lst.de \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.