linux-kernel - Re: [PATCH v4 6/9] f2fs: don't allow DIO reads but not DIO 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]

Date:   Mon, 25 Jul 2022 17:58:31 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     Jaegeuk Kim <jaegeuk@...nel.org>, linux-fsdevel@...r.kernel.org,
        linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
        linux-xfs@...r.kernel.org, linux-api@...r.kernel.org,
        linux-fscrypt@...r.kernel.org, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, Keith Busch <kbusch@...nel.org>
Subject: Re: [PATCH v4 6/9] f2fs: don't allow DIO reads but not DIO writes

On Jul 25, 2022, at 12:12 PM, Eric Biggers <ebiggers@...nel.org> wrote:
> 
> On Sat, Jul 23, 2022 at 07:01:59PM -0700, Jaegeuk Kim wrote:
>> On 07/22, Eric Biggers wrote:
>>> From: Eric Biggers <ebiggers@...gle.com>
>>> 
>>> Currently, if an f2fs filesystem is mounted with the mode=lfs and
>>> io_bits mount options, DIO reads are allowed but DIO writes are not.
>>> Allowing DIO reads but not DIO writes is an unusual restriction, which
>>> is likely to be surprising to applications, namely any application that
>>> both reads and writes from a file (using O_DIRECT).  This behavior is
>>> also incompatible with the proposed STATX_DIOALIGN extension to statx.
>>> Given this, let's drop the support for DIO reads in this configuration.
>> 
>> IIRC, we allowed DIO reads since applications complained a lower performance.
>> So, I'm afraid this change will make another confusion to users. Could
>> you please apply the new bahavior only for STATX_DIOALIGN?
>> 
> 
> Well, the issue is that the proposed STATX_DIOALIGN fields cannot represent this
> weird case where DIO reads are allowed but not DIO writes.  So the question is
> whether this case actually matters, in which case we should make STATX_DIOALIGN
> distinguish between DIO reads and DIO writes, or whether it's some odd edge case
> that doesn't really matter, in which case we could just fix it or make
> STATX_DIOALIGN report that DIO is unsupported.  I was hoping that you had some
> insight here.  What sort of applications want DIO reads but not DIO writes?
> Is this common at all?

I don't think this is f2fs related, but some backup applications I'm aware
of are using DIO reads to avoid polluting the page cache when reading large
numbers of files. They don't care about DIO writes, since that is usually
slower than async writes due to the sync before returning from the syscall.

Also, IMHO it doesn't make sense to remove useful functionality because the
new STATX_DIOALIGN fields don't handle this.  At worst the application will
still get an error when trying a DIO write, but in most cases they will
not use the brand new STATX call in the first place, and if this is documented
then any application that starts to use it should be able to handle it.

Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)