On Mon, May 05, 2025 at 11:55:13AM +0100, John Garry wrote: > On 05/05/2025 11:49, Christoph Hellwig wrote: > > On Mon, May 05, 2025 at 11:04:55AM +0100, John Garry wrote: > > > @@ -503,6 +509,9 @@ xfs_open_devices( > > > mp->m_logdev_targp = xfs_alloc_buftarg(mp, logdev_file); > > > if (!mp->m_logdev_targp) > > > goto out_free_rtdev_targ; > > > + error = sync_blockdev(mp->m_logdev_targp->bt_bdev); > > > + if (error) > > > + goto out_free_rtdev_targ; > > > } else { > > > mp->m_logdev_targp = mp->m_ddev_targp; > > > /* Handle won't be used, drop it */ > > > > > > > > > Right? > > Yes. Or in fact just folding it into xfs_alloc_buftarg, which might > > be even simpler. > > Yes, that was my next question.. > > > While you're at it adding a command why we are doing > > the sync would also be really useful, and having it in just one place > > helps with that. > > ok, there was such comment in xfs_preflush_devices(). > > @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. --D > Thanks, > John > >