Received-SPF: pass (google.com: domain of linux-kernel+bounces-143573-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Sat, 13 Apr 2024 05:15:34 +0100
From: Al Viro <viro@zeniv.linux.org.uk>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCHSET RFC 0/437] Kill off old fops ->read() and ->write()
Message-ID: <20240413041534.GO2118490@ZenIV>
References: <20240411153126.16201-1-axboe@kernel.dk>
 <20240412042910.GK2118490@ZenIV>
 <b6725c23-ad82-4082-9e72-b219fc2b453e@kernel.dk>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <b6725c23-ad82-4082-9e72-b219fc2b453e@kernel.dk>
Sender: Al Viro <viro@ftp.linux.org.uk>

On Fri, Apr 12, 2024 at 07:58:13AM -0600, Jens Axboe wrote:

> I'm aware of some drivers that do different things from write vs writev,
> or read vs readv for instance. But those I did cater to, by having a
> flag they can now check.
> 
> Can you be a bit more specific on an example of a driver that does the
> above?

Consider e.g. your #39.  Current mainline: 1 call of ->set() for each
segment passed to writev() on any of those.  With your patch: call
segments concatenated and if the concatenation looks like a number,
a single call of ->set().

If nothing else, it's a user-visible ABI change.  And in cases when ->set()
has non-trivial side effects, it just might break a real-world code that
is currently correct.

I picked that one because I didn't want to dig through the drivers -
I'm pretty sure that there's more to be found there.

It's not just "write() and writev() parse the data in different way" - we do
have a couple of those, but that's a minor problem.

"write(fd, buf, len1); write(fd, buf + len1, len1 + len2); is not the same
thing as write(fd, buf, len1 + len2)" is not rare for character devices and
for regular files on procfs/debugfs/etc.

For any of those you need to use you vfs_write_iter() helper or you'll
be breaking userland ABI.  The cost of audit of several thousands of ->write()
(and ->read() - similar problem applies there) instances, checking that property
is the main reason this conversion hadn't been already done.