lkml.org 
[lkml]   [2018]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM


On Sat, 21 Apr 2018, Matthew Wilcox wrote:

> On Fri, Apr 20, 2018 at 05:21:26PM -0400, Mikulas Patocka wrote:
> > On Fri, 20 Apr 2018, Matthew Wilcox wrote:
> > > On Fri, Apr 20, 2018 at 04:54:53PM -0400, Mikulas Patocka wrote:
> > > > On Fri, 20 Apr 2018, Michal Hocko wrote:
> > > > > No way. This is just wrong! First of all, you will explode most likely
> > > > > on many allocations of small sizes. Second, CONFIG_DEBUG_VM tends to be
> > > > > enabled quite often.
> > > >
> > > > You're an evil person who doesn't want to fix bugs.
> > >
> > > Steady on. There's no need for that. Michal isn't evil. Please
> > > apologise.
> >
> > I see this attitude from Michal again and again.
>
> Fine; then *say that*. I also see Michal saying "No" a lot. Sometimes
> I agree with him, sometimes I don't. I think he genuinely wants the best
> code in the kernel, and saying "No" is part of it.
>
> > He didn't want to fix vmalloc(GFP_NOIO)
>
> I don't remember that conversation, so I don't know whether I agree with
> his reasoning or not. But we are supposed to be moving away from GFP_NOIO
> towards marking regions with memalloc_noio_save() / restore. If you do
> that, you won't need vmalloc(GFP_NOIO).

He said the same thing a year ago. And there was small progress. 6 out of
27 __vmalloc calls were converted to memalloc_noio_save in a year - 5 in
infiniband and 1 in btrfs. (the whole discussion is here
http://lkml.iu.edu/hypermail/linux/kernel/1706.3/04681.html )

He refuses 15-line patch to fix GFP_NOIO bug because he believes that in 4
years, the kernel will be refactored and GFP_NOIO will be eliminated. Why
does he have veto over this part of the code? I'd much rather argue with
people who have constructive comments about fixing bugs than with him.

(note that even if the refactoring eventually succeeds, it will not be
backported to stable branches. The small vmalloc patch could be
backported)

> > he didn't want to fix alloc_pages sleeping when __GFP_NORETRY is used.
>
> The GFP flags are a mess, still.

That's the problem - the flag doesn't have a clear contract and the
developers change behavior ad hoc according to bug reports.

> > So what should I say? Fix them and you won't be evil :-)
>
> No, you should reserve calling somebody evil for truly evil things.

How would you call it? Michal falsely believes that a 15-line patch would
prevent him from doing long-term refactoring work and so he refuses it.

> > I already said that we can change it from CONFIG_DEBUG_VM to
> > CONFIG_DEBUG_SG - or to whatever other option you may want, just to make
> > sure that it is enabled in distro debug kernels by default.
>
> Yes, and I think that's the right idea. So send a v2 and ignore the
> replies that are clearly relating to an earlier version of the patch.
> Not everybody reads every mail in the thread before responding to one they
> find interesting. Yes, ideally, one would, but sometimes one doesn't.

I sent the CONFIG_DEBUG_SG patch before (I wonder why he didn't repond to
it). I'll send a third version of the patch that actually randomly chooses
between kmalloc and vmalloc, because some abuses can only be detected with
kmalloc and we should test both.

For bisecting, it is better to always fallback to vmalloc, but for general
testing, it is better to test both branches.

Mikulas

\
 
 \ /
  Last update: 2018-04-23 16:06    [W:0.133 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site