linux-ext4 - Re: [PATCH 0/1] iomap regression for aio dio 4k 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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZJULQjTpcRdEUHY8@dread.disaster.area>
Date:   Fri, 23 Jun 2023 13:02:26 +1000
From:   Dave Chinner <david@...morbit.com>
To:     Theodore Ts'o <tytso@....edu>
Cc:     Jeremy Bongio <bongiojp@...il.com>,
        "Darrick J . Wong" <djwong@...nel.org>,
        Allison Henderson <allison.henderson@...cle.com>,
        linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-xfs@...r.kernel.org
Subject: Re: [PATCH 0/1] iomap regression for aio dio 4k writes

On Thu, Jun 22, 2023 at 10:32:33PM -0400, Theodore Ts'o wrote:
> On Thu, Jun 22, 2023 at 09:59:29AM +1000, Dave Chinner wrote:
> > Ah, you are testing pure overwrites, which means for ext4 the only
> > thing it needs to care about is cached mappings. What happens when
> > you add O_DSYNC here?
> 
> I think you mean O_SYNC, right?

No, I *explicitly* meant O_DSYNC.

> In a pure overwrite case, where all
> of the extents are initialized and where the Oracle or DB2 server is
> doing writes to preallocated, pre-initialized space in the tablespace
> file followed by fdatasync(), there *are* no post-I/O data integrity
> operations which are required.

Wrong: O_DSYNC DIO write IO requires the data to be on stable
storage at IO completion. This means the pure overwrite IO must be
either issued as a REQ_FUA write or as a normal write followed by a
device cache flush.

That device cache flush is a post-I/O data integrity operation and
that is handled by iomap_dio_complete() -> generic_write_sync() -> 
vfs_fsync_range()....

> If the file is opened O_SYNC or if the blocks were not
> preallocated using fallocate(2) and not initialized ahead of time,
> then sure, we can't use this optimization.

Well, yes. That's the whole point of the IOMAP_F_DIRTY flag - if
that is set, we don't attempt any pure overwrite optimisations
because it's not a pure overwrite and metadata needs flushing to the
journal. Hence we need to call generic_write_sync().

> What we might to do is to let the file system tell the iomap layer
> via a flag whether or not there are no post-I/O metadata
> operations required, and then *if* that flag is set, and *if* the
> inode has no pages in the page cache (so there are no invalidate
> operations necessary), it should be safe to skip using
> queue_work().  That way, the file system has to affirmatively
> state that it is safe to skip the workqueue, so it shouldn't do
> any harm to other file systems using the iomap DIO layer.
> 
> What am I missing?

You didn't read my followup email. IOMAP_F_DIRTY is the flag
you describe, and it already exists.

-Dave.
-- 
Dave Chinner
david@...morbit.com