lkml.org 
[lkml]   [2021]   [Feb]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [GIT PULL] Kmap conversions for 5.12
On Tue, Feb 23, 2021 at 09:13:42AM -0800, Linus Torvalds wrote:
> On Tue, Feb 23, 2021 at 7:03 AM David Sterba <dsterba@suse.com> wrote:
> > 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:
>
> > 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

\
 
 \ /
  Last update: 2021-02-23 20:27    [W:0.144 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site