Received-SPF: pass (google.com: domain of linux-kernel+bounces-143786-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <8513fceb-6a5e-4dcc-8df1-8ae81b6db77f@kernel.dk>
Date: Sat, 13 Apr 2024 09:37:23 -0600
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCHSET RFC 0/437] Kill off old fops ->read() and ->write()
Content-Language: en-US
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-kernel@vger.kernel.org
References: <20240411153126.16201-1-axboe@kernel.dk>
 <20240412042910.GK2118490@ZenIV>
 <b6725c23-ad82-4082-9e72-b219fc2b453e@kernel.dk>
 <20240413041534.GO2118490@ZenIV>
From: Jens Axboe <axboe@kernel.dk>
In-Reply-To: <20240413041534.GO2118490@ZenIV>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 4/12/24 10:15 PM, Al Viro wrote:
> 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.

Thanks Al, I see what you mean now, and yes  that certainly makes
sense. I'll go over the conversions and see what might be affected, and
for those we'll probably just play it safe initially and use the FOPS
iterators helpers to avoid changing behavior.

-- 
Jens Axboe