Re: [PATCH 1/2] fs: remove kiocb.ki_hint

Jens Axboe
2022-03-22T03:00:38Z

On 3/21/22 8:57 PM, Keith Busch wrote:
> On Mon, Mar 21, 2022 at 08:50:10PM -0600, Jens Axboe wrote:
>> On 3/21/22 8:45 PM, Matthew Wilcox wrote:
>>> On Mon, Mar 21, 2022 at 08:13:10PM -0600, Jens Axboe wrote:
>>>> On 3/8/22 5:55 PM, Jens Axboe wrote:
>>>>> On Tue, 8 Mar 2022 07:05:28 +0100, Christoph Hellwig wrote:
>>>>>> This field is entirely unused now except for a tracepoint in f2fs, so
>>>>>> remove it.
>>>>>>
>>>>>>
>>>>>
>>>>> Applied, thanks!
>>>>>
>>>>> [1/2] fs: remove kiocb.ki_hint
>>>>> commit: 41d36a9f3e5336f5b48c3adba0777b8e217020d7
>>>>> [2/2] fs: remove fs.f_write_hint
>>>>> commit: 7b12e49669c99f63bc12351c57e581f1f14d4adf
>>>>
>>>> Upon thinking about the EINVAL solution a bit more, I do have a one
>>>> worry - if you're currently using write_hints in your application,
>>>> nobody should expect upgrading the kernel to break it. It's a fine
>>>> solution for anything else, but that particular point does annoy me.
>>>
>>> But if your application is run on an earlier kernel, it'll get
>>> -EINVAL. So it must already be prepared to deal with that?
>>
>> Since support wasn't there, it's not unreasonable to expect that an
>> application was written on a newer kernel. Might just be an in-house or
>> application thing, who knows? But the point is that upgrading from 5.x
>> to 5.x+1, nobody should expect their application to break. And it will
>> with this change. If you downgrade a kernel and a feature didn't exist
>> back then, it's reasonable to expect that things may break.
>>
>> Maybe this is being overly cautious, but... As a matter of principle,
>> it's unquestionably wrong.
>>
>> We can just let Linus make the decision here, arming him with the info
>> he needs to make it in terms of hardware support. I'll write that up in
>> the pull request.
>
> It's really just an advisory hint at the hardware level anyway, so
> returning success without actually doing anything with it technically
> satisfies the requirement.

Yes that's my point, nobody will know that the hints do nothing, all I
care about here is that the user ABI doesn't change. I know I originally
advocated for -EINVAL and I do think it's correct for _most_ cases, it's
just that it does have the potential to mess up a legit kernel upgrade
scenario which is exactly the kind of thing we don't want to happen.

--
Jens Axboe