Messages in this thread | | | Date | Tue, 23 Feb 2021 11:25:06 -0800 | From | Ira Weiny <> | Subject | Re: [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
| |