On 1/20/23 18:43, Jens Axboe wrote:
On 1/20/23 5:11?PM, Bart Van Assche wrote:
If CONFIG_BLK_SUB_PAGE_SEGMENTS is made invisible, how should this
option be enabled for the scsi_debug and null_blk drivers? Adding
"select BLK_SUB_PAGE_SEGMENTS" to the Kconfig section of these drivers
would have the unfortunate side effect that enabling either driver
would make all block drivers slower. How about making sub-page segment
support configurable for the scsi_debug and null_blk drivers only?
That would allow kernel developers who want to test the sub-page
segment support to enable this functionality without making e.g.
distro kernels slower.
You'd need to have a separate sub-option for each of them, Kconfig
style. But this also highlights the usual issue with pretending that
Kconfig options means that this doesn't matter, because inevitably they
end up getting enabled by default in distros anyway. And then you may as
well not even have them...
Hi Jens,
How about the following approach?
* Remove CONFIG_BLK_SUB_PAGE_SEGMENTS.
* Use the static key mechanism instead of #ifdef
CONFIG_BLK_SUB_PAGE_SEGMENTS to prevent that this patch series makes
the hot path in the block layer slower.
* Count the number of request queues that need support for segments
smaller than a page or max_hw_sector values smaller than
PAGE_SIZE >> SECTOR_SHIFT. If that number changes from zero to one,
enable the code introduced by this patch series. If that number drops
to zero, toggle the static key such that the overhead of the code that
supports small segments is eliminated.
Why can't we just handle this in the driver? The segment path is hard
enough to grok in the first place, and this just makes it worse.
Generally not a huge fan of punting this to the driver, but just maybe
it'd make sense in this case since it's just the one. At least that
seems a lot more palatable than the alternative.
One of the functions modified by this patch series is
blk_rq_append_bio(). That function is called by code that builds a bio
(e.g. filesystems). Code that builds a bio does not call any block
driver code. This is why I think it would be hard to move the
functionality introduced by this patch series into block drivers.
Thanks,
Bart.
