Re: [PATCH 1/3] libxfs: don't set mp->m_rtdev_targp when no RT devices is provided

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Sep 11, 2026 at 05:11:20PM +0200, Christoph Hellwig wrote:
> On Fri, Sep 11, 2026 at 07:57:26AM -0700, Darrick J. Wong wrote:
> > > -	if (!xi->rt.dev || xi->rt.dev == xi->data.dev)
> > > +	if (xi->rt.dev == xi->data.dev)
> > >  		mp->m_rtdev_targp = mp->m_ddev_targp;
> > >  	else
> > >  		mp->m_rtdev_targp = libxfs_buftarg_alloc(mp, xi, &xi->rt,
> > 
> > Does this mean that m_rtdev_targp is never NULL, even when we don't
> > specify or have a rt device attached?
> 
> I think that is the case right now.
> 
> > I would have expected this to be:
> > 
> > 	if (!xi->rt.dev)
> > 		mp->m_rtdev_targp = NULL;
> > 	else if (xi->rt.dev == xi->data.dev)
> > 		mp->m_rtdev_targp = mp->m_ddev_targp;
> > 	else
> > 		mp->m_rtdev_targp = libxfs_buftarg_alloc(...);
> 
> And that is what we get with this patch.  The zeroing is done by
> zeroing the entire mount structure in the callers of libxfs_mount
> (don't ask me why..).

Sorry, but I don't see how we get to m_rtdev_targp==NULL with this
patch?  The end of libxfs_buftarg_init becomes:


	mp->m_ddev_targp = libxfs_buftarg_alloc(mp, xi, &xi->data, dfail);
	if (!xi->log.dev || xi->log.dev == xi->data.dev)
		mp->m_logdev_targp = mp->m_ddev_targp;
	else
		mp->m_logdev_targp = libxfs_buftarg_alloc(mp, xi, &xi->log,
				lfail);
	if (xi->rt.dev == xi->data.dev)
		mp->m_rtdev_targp = mp->m_ddev_targp;
	else
		mp->m_rtdev_targp = libxfs_buftarg_alloc(mp, xi, &xi->rt,
				rfail);

In the case where the rt and data devices are not the same, we create a
new buftarg object, even if xi->rt.dev == 0.  That only happens if we
didn't set xi->rt.name to a path, which means that libxfs_device_open
ignores it:

# truncate -s 3g /tmp/a
# mkfs.xfs -f /tmp/a
# gdb --args ./build-x86_64/db/xfs_db /tmp/a
(gdb) p *mp->m_rtdev_targp
$5 = {
  bt_mount = 0x555555659660 <xmount>,
  lock = {
    __data = {
      __lock = 0,
      __count = 0,
      __owner = 0,
      __nusers = 0,
      __kind = 0,
      __spins = 0,
      __elision = 0,
      __list = {
        __prev = 0x0,
        __next = 0x0
      }
    },
    __size = '\000' <repeats 39 times>,
    __align = 0
  },
  writes_left = 0,
  bt_bdev = 0,
  bt_bdev_fd = -1,
  bt_xfile = 0x0,
  flags = 0,
  bcache = 0x555555684ab0
}

Here we didn't supply an -R option to xfs_db, but libxfs assigns
m_rtdev_targp to a buftarg that can't do anything useful, instead of
NULL which (AFAICT) the code expects:

$ git grep m_rtdev_targp
mkfs/xfs_mkfs.c:5635:   if (mp->m_rtdev_targp->bt_bdev &&
mkfs/xfs_mkfs.c:5636:       mp->m_rtdev_targp != mp->m_ddev_targp &&
<snip>
libxfs/init.c:981:      if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp) {
libxfs/init.c:982:              err2 = libxfs_flush_buftarg(mp->m_rtdev_targp,

OTOH there are other places that use other checks that clearly assume
that m_rtdev_targp is always present:

libxfs/init.c:311:      if (mp->m_rtdev_targp->bt_bdev == 0 && !xfs_is_debugger(mp)) {
libxfs/init.c:1034:     if (mp->m_rtdev_targp != mp->m_ddev_targp)
libxfs/init.c:1035:             libxfs_buftarg_free(mp->m_rtdev_targp);

Ok maybe this patch is fine as-is?  But xfsprogs needs a treewide change
to make the "do we have a rt device attached?" checks match the kernel
code?

<even more confused>

--D