Thread (10 messages) flat view 10 messages, 4 authors, 2021-02-26

Re: [GIT PULL] Kmap conversions for 5.12

From: Ira Weiny <hidden>
Date: 2021-02-23 19:26:29
Also in: lkml

On Tue, Feb 23, 2021 at 09:13:42AM -0800, Linus Torvalds wrote:
On Tue, Feb 23, 2021 at 7:03 AM David Sterba [off-list ref] wrote:
quoted
Ira Weiny (8):
      iov_iter: Remove memzero_page() in favor of zero_user()
Ugh. I absolutely _detest_ this patch.
Sorry.
"zero_user()" is a completely horrendous function, and not at all the
same as memzero_page().

Just look at it.

Yes, it's mis-used in a lot of places that really always wanted
"memzero_page()", but this conversion is going exactly the wrong way
around.
Originally I lifted memzero_page()[1] but was pointed to zero_user_segments()
which lead me astray to use zero_user().  I should have thought about it more
rather than blindly changing to zero_user().
Existing users of that zero_user() should have been converted to
memzero_page(), rather than doing it this way.
I can do that.  No Problem.
The "user" naming should have given it away. It's a very very magical
interface for user-mapped pages that have additional odd issues (ie
look at the dcache flushing etc).
Agreed.
I'll think some more about this pull request, but honestly, this one
broken is pretty much enough for me to say "No way in hell", because
it shows a complete disregard for sanity.
Can we just drop the zero_user() patches?  Christoph and others would like to
see memcpy_[to|from]_page() lifted to the core for other work which is pending.
Would you agree to those?
The last commit in the series:
quoted
      btrfs: convert to zero_user()
is also very mixed up about whether it actually wants the dcache
flushing or not (and thus zero_user() or memzero_page()).
Drop this patch too?
Honestly, I suspect all the dcache flushing is totally pointless,
because any architecture with virtual caches that does kmap needs to
flush at kunmap anyway, afaik. Some of it is probably just voodoo
programming and copying a pattern.

But in any case, zero_user() is not the same thing as memzero_page(),
and even if they *were* the same thing, zero_user() is objectively the
horribly much worse name.
Sorry.  I will change it.
Ira
              Linus
[1] https://lore.kernel.org/lkml/20201124141941.GB4327@casper.infradead.org/#t (local)

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help