linux-kernel - Re: [PATCH RFC 03/10] iomap: Support CoW-based atomic writes

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250206214023.GV21808@frogsfrogsfrogs>
Date: Thu, 6 Feb 2025 13:40:23 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: John Garry <john.g.garry@...cle.com>
Cc: brauner@...nel.org, cem@...nel.org, dchinner@...hat.com, hch@....de,
	linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, ojaswin@...ux.ibm.com,
	ritesh.list@...il.com, martin.petersen@...cle.com
Subject: Re: [PATCH RFC 03/10] iomap: Support CoW-based atomic writes

On Thu, Feb 06, 2025 at 11:21:13AM +0000, John Garry wrote:
> 
> > >   			if (iomi.pos >= dio->i_size ||
> > > diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> > > index 75bf54e76f3b..0a0b6798f517 100644
> > > --- a/include/linux/iomap.h
> > > +++ b/include/linux/iomap.h
> > > @@ -56,6 +56,8 @@ struct vm_fault;
> > >    *
> > >    * IOMAP_F_BOUNDARY indicates that I/O and I/O completions for this iomap must
> > >    * never be merged with the mapping before it.
> > > + *
> > > + * IOMAP_F_ATOMIC_COW indicates that we require atomic CoW end IO handling.
> > 
> > It more indicates that the filesystem is using copy on write to handle
> > an untorn write, and will provide the ioend support necessary to commit
> > the remapping atomically, right?
> 
> yes, correct
> 
> > 
> > >    */
> > >   #define IOMAP_F_NEW		(1U << 0)
> > >   #define IOMAP_F_DIRTY		(1U << 1)
> > > @@ -68,6 +70,7 @@ struct vm_fault;
> > >   #endif /* CONFIG_BUFFER_HEAD */
> > >   #define IOMAP_F_XATTR		(1U << 5)
> > >   #define IOMAP_F_BOUNDARY	(1U << 6)
> > > +#define IOMAP_F_ATOMIC_COW	(1U << 7)
> > >   /*
> > >    * Flags set by the core iomap code during operations:
> > > @@ -183,6 +186,7 @@ struct iomap_folio_ops {
> > >   #define IOMAP_DAX		0
> > >   #endif /* CONFIG_FS_DAX */
> > >   #define IOMAP_ATOMIC		(1 << 9)
> > > +#define IOMAP_ATOMIC_COW	(1 << 10)
> > 
> > What does IOMAP_ATOMIC_COW do?  There's no description for it (or for
> > IOMAP_ATOMIC).
> 
> I'll add a description for both.
> 
> > Can you have IOMAP_ATOMIC and IOMAP_ATOMIC_COW both set?
> 
> Yes
> 
> > Or are they mutually exclusive?
> 
> I am not thinking that it might be neater to have a distinct flag for

"not"?

> IOMAP_ATOMIC when we want to try an atomic bio - maybe IOMAP_ATOMIC_HW or
> IOMAP_ATOMIC_BIO? And then also IOMAP_DIO_ATOMIC_BIO (in addition to
> IOMAP_DIO_ATOMIC_COW).

Yeah, ATOMIC_BIO/ATOMIC_COW is probably clearer both for IOMAP_ and
IOMAP_DIO_ .

--D

> > 
> > I'm guessing from the code that ATOMIC_COW requires ATOMIC to be set,
> > but I wonder why because there's no documentation update in the header
> > files or in Documentation/filesystems/iomap/.
> 
> Will do.
> 
> Thanks,
> John
>