On 2/6/23 18:08, Luis Chamberlain wrote:
On Mon, Feb 06, 2023 at 04:31:58PM -0800, Bart Van Assche wrote:
On 2/6/23 16:19, Luis Chamberlain wrote:
But I'm trying to do a careful review here.
That's appreciated :-)
The commit log did not describe what *does* happen in these situations today,
and you seem to now be suggesting in the worst case corruption can happen.
That changes the patch context quite a bit!
My question above still stands though, how many block drivers have a max
hw sector smaller than the equivalent PAGE_SIZE. If you make your
change, even if it fixes some new use case where corruption is seen, can
it regress some old use cases for some old controllers?
The blk_queue_max_hw_sectors() change has been requested by a contributor to
the MMC driver (I'm not familiar with the MMC driver).
I'm not aware of any storage controllers for which the maximum segment size
is below 4 KiB.
Then the commit log should mention that. Because do you admit that it
could possible change their behaviour?
I will make the commit message more detailed.
For some storage controllers, e.g. the UFS Exynos controller, the maximum
supported segment size is 4 KiB. This patch series makes such storage
controllers compatible with larger page sizes, e.g. 16 KiB.
Does this answer your question?
Does mine answer the reason to why I am asking it? If we are sure these
don't exist then please mention it in the commit log. And also more
importantly the possible corruption issue you describe which could
happen! Was a corruption actually observed in real life or reported!?
Incorrect data transfers have been observed in our tests. We noticed in
our tests with the Exynos controller and PAGE_SIZE = 16 KiB that booting
fails without this patch series. I think booting failed because the DMA
engine in this controller was asked to perform transfers that fall
outside the supported limits. I expect similar behavior for all other
storage controllers with a DMA engine.
Bart.
