Hello Bart,
On Thu, Sep 21, 2023 at 07:27:08AM -0700, Bart Van Assche wrote:
> On 9/21/23 00:46, Niklas Cassel wrote:
> > Considering that this API (F_GET_FILE_RW_HINT / F_SET_FILE_RW_HINT) was
> > previously only used by NVMe (NVMe streams).
>
> That doesn't sound correct to me. I think support for this API was added
> in F2FS in November 2017 (commit 4f0a03d34dd4 ("f2fs: apply write hints
> to select the type of segments for buffered write")). That was a few
> months after NVMe stream support was added (June 2017) by commit
> f5d118406247 ("nvme: add support for streams and directives").
I wrote the "this API (F_GET_FILE_RW_HINT / F_SET_FILE_RW_HINT),
i.e. the support for hints in the block layer.
This addition to the block layer API was added in:
c75b1d9421f8 ("fs: add fcntl() interface for setting/getting write life time hints")
As part of this series:
https://lore.kernel.org/linux-block/1498491480-16306-1-git-send-email-axboe@xxxxxxxxx/
So this support included:
-the block layer API changes
-the support for NVMe streams
The modifications to f2fs to actually make use of these block layer write
hints was not included in this initial series. They were added several
months later.
> From commit 561593a048d7 ("Merge tag 'for-5.18/write-streams-2022-03-18'
> of git://git.kernel.dk/linux-block"): "This removes the write streams
> support in NVMe. No vendor ever really shipped working support for this,
> and they are not interested in supporting it."
>
> I do not want to reopen the discussion about NVMe streams.
I don't think we need to.
I simply think that your cover letter should mention it somehow...
As the whole reason why the block layer API was merged was to be
able to support NVMe streams.
So you bringing back this API, I think that you should at least
mention that you don't bring back NVMe streams...
and mention that you bring back the support for f2fs,
and add support for SCSI.. with some short motivation of why support
is needed in both SCSI and f2fs.
Right now your cover letter is 4 lines :)
I don't recall when I last saw such a small cover letter for a feature
impacting so many different parts of the kernel.
Kind regards,
Niklas
