Re: [PATCH RFC 0/5] device mapper atomic write support

[Mikulas Patocka <mpatocka@redhat.com>]

On Thu, 16 Jan 2025, John Garry wrote:

> On 16/01/2025 12:59, Mikulas Patocka wrote:
> > > > dm-mirror uses dm-io to perform the writes on multiple mirror legs (see
> > > > the function do_write() -> dm_io()), I looked at the code and it seems
> > > > that the support for atomic writes in dm-mirror and dm-io would be
> > > > straightforward.
> > > I tried this out, and it seems to work ok.
> > > 
> > > However, I need to set DM_TARGET_ATOMIC_WRITES in the
> > > mirror_target.features
> > > member, like:
> > > 
> > > diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c
> > > index 9511dae5b556..913a92c55904 100644
> > > --- a/drivers/md/dm-raid1.c
> > > +++ b/drivers/md/dm-raid1.c
> > > @@ -1485,6 +1485,7 @@ static struct target_type mirror_target = {
> > > 	.name    = "mirror",
> > > 	.version = {1, 14, 0},
> > > 	.module  = THIS_MODULE,
> > > +	.features = DM_TARGET_ATOMIC_WRITES,
> > > 	.ctr     = mirror_ctr,
> > > 	.dtr     = mirror_dtr,
> > > 	.map     = mirror_map,
> > > 
> > > 
> > > Is this the right thing to do? I ask, as none of the other DM_TARGET*
> > > flags
> > > are set already, which makes me suspicious.
> > > 
> > > Thanks,
> > > John
> > Yes - that's right. I suggest that you verify that the atomic flag is
> > really passed through the dm-raid1.c and dm-io.c stack. Add a printk that
> > tests if REQ_ATOMIC is set to the function do_region in dm-io.c just
> > before "submit_bio(bio)".
> > 
> > Alternatively, you can use blktrace to test if the REQ_ATOMIC is passed
> > through correctly.
> 
> Yes, it is passed ok.
> 
> JFYI, I can also verify proper atomic write functionality on /dev/dmX with fio
> in verify mode.
> 
> Thanks,
> John

Yes - so please send version 2 of the patches and I will stage them for 
this merge window.

Mikulas