All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mikulas Patocka <mpatocka@redhat.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>,
	David Miller <davem@davemloft.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, eric.dumazet@gmail.com, edumazet@google.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	mst@redhat.com, jasowang@redhat.com,
	virtualization@lists.linux-foundation.org, dm-devel@redhat.com,
	Vlastimil Babka <vbabka@suse.cz>
Subject: Re: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
Date: Tue, 24 Apr 2018 11:30:40 -0400 (EDT)	[thread overview]
Message-ID: <alpine.LRH.2.02.1804241107010.31601@file01.intranet.prod.int.rdu2.redhat.com> (raw)
In-Reply-To: <20180424133146.GG17484@dhcp22.suse.cz>



On Tue, 24 Apr 2018, Michal Hocko wrote:

> On Mon 23-04-18 20:25:15, Mikulas Patocka wrote:
> 
> > Fixing __vmalloc code 
> > is easy and it doesn't require cooperation with maintainers.
> 
> But it is a hack against the intention of the scope api.

It is not! You can fix __vmalloc now and you can convert the kernel to the 
scope API in 4 years. It's not one way or the other.

> It also alows maintainers to not care about their broken code.

Most maintainers don't even know that it's broken. Out of 14 subsystems 
using __vmalloc with GFP_NOIO/NOFS, only 2 realized that its 
implementation is broken and implemented a workaround (me and the XFS 
developers).

Misimplementing a function in a subtle and hard-to-notice way won't drive 
developers away from using it.

> > > > 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.
> > > 
> > > I didn't NACK the patch AFAIR. I've said it is not a good idea longterm.
> > > I would be much more willing to change my mind if you would back your
> > > patch by a real bug report. Hacks are acceptable when we have a real
> > > issue in hands. But if we want to fix potential issue then better make
> > > it properly.
> > 
> > Developers should fix bugs in advance, not to wait until a crash hapens, 
> > is analyzed and reported.
> 
> I agree. But are those existing users broken in the first place? I have
> seen so many GFP_NOFS abuses that I would dare to guess that most of
> those vmalloc NOFS abusers can be simply turned into GFP_KERNEL. Maybe
> that is the reason we haven't heard any complains in years.

alloc_pages reclaims clean pages and most hard work is done by kswapd, so 
GFP_KERNEL doesn't cause much issues with writeback. But cheating isn't 
justified if you can get away with it. Incorrect GFP flags cause real 
problems with shrinkers - because shrinkers are called from alloc_pages 
and they do respond to GFP flags.

I had reported deadlock due to GFP issues (9d28eb12447). And the worst 
thing about these bug reports is that they are totally unreproducible and 
I get nothing, but a stacktrace in bugzilla. I had to guess what happened 
and I couldn't even test if the patch fixed the bug.

I'm not really happy that you are deliberately leaving these issues behind 
and making excuses.

Mikulas

WARNING: multiple messages have this Message-ID (diff)
From: Mikulas Patocka <mpatocka@redhat.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: dm-devel@redhat.com, eric.dumazet@gmail.com, mst@redhat.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Matthew Wilcox <willy@infradead.org>,
	virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
	edumazet@google.com, Andrew Morton <akpm@linux-foundation.org>,
	David Miller <davem@davemloft.net>,
	Vlastimil Babka <vbabka@suse.cz>
Subject: Re: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
Date: Tue, 24 Apr 2018 11:30:40 -0400 (EDT)	[thread overview]
Message-ID: <alpine.LRH.2.02.1804241107010.31601@file01.intranet.prod.int.rdu2.redhat.com> (raw)
In-Reply-To: <20180424133146.GG17484@dhcp22.suse.cz>



On Tue, 24 Apr 2018, Michal Hocko wrote:

> On Mon 23-04-18 20:25:15, Mikulas Patocka wrote:
> 
> > Fixing __vmalloc code 
> > is easy and it doesn't require cooperation with maintainers.
> 
> But it is a hack against the intention of the scope api.

It is not! You can fix __vmalloc now and you can convert the kernel to the 
scope API in 4 years. It's not one way or the other.

> It also alows maintainers to not care about their broken code.

Most maintainers don't even know that it's broken. Out of 14 subsystems 
using __vmalloc with GFP_NOIO/NOFS, only 2 realized that its 
implementation is broken and implemented a workaround (me and the XFS 
developers).

Misimplementing a function in a subtle and hard-to-notice way won't drive 
developers away from using it.

> > > > 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.
> > > 
> > > I didn't NACK the patch AFAIR. I've said it is not a good idea longterm.
> > > I would be much more willing to change my mind if you would back your
> > > patch by a real bug report. Hacks are acceptable when we have a real
> > > issue in hands. But if we want to fix potential issue then better make
> > > it properly.
> > 
> > Developers should fix bugs in advance, not to wait until a crash hapens, 
> > is analyzed and reported.
> 
> I agree. But are those existing users broken in the first place? I have
> seen so many GFP_NOFS abuses that I would dare to guess that most of
> those vmalloc NOFS abusers can be simply turned into GFP_KERNEL. Maybe
> that is the reason we haven't heard any complains in years.

alloc_pages reclaims clean pages and most hard work is done by kswapd, so 
GFP_KERNEL doesn't cause much issues with writeback. But cheating isn't 
justified if you can get away with it. Incorrect GFP flags cause real 
problems with shrinkers - because shrinkers are called from alloc_pages 
and they do respond to GFP flags.

I had reported deadlock due to GFP issues (9d28eb12447). And the worst 
thing about these bug reports is that they are totally unreproducible and 
I get nothing, but a stacktrace in bugzilla. I had to guess what happened 
and I couldn't even test if the patch fixed the bug.

I'm not really happy that you are deliberately leaving these issues behind 
and making excuses.

Mikulas

  reply	other threads:[~2018-04-24 15:30 UTC|newest]

Thread overview: 218+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-18 14:34 [PATCH] net: don't use kvzalloc for DMA memory Mikulas Patocka
2018-04-18 16:05 ` Eric Dumazet
2018-04-18 16:44   ` Mikulas Patocka
2018-04-18 16:44   ` Mikulas Patocka
2018-04-18 16:51     ` Eric Dumazet
2018-04-18 16:51     ` Eric Dumazet
2018-04-18 17:47       ` David Miller
2018-04-18 17:47         ` David Miller
2018-04-18 17:55         ` Mikulas Patocka
2018-04-18 17:55         ` Mikulas Patocka
2018-04-18 18:00         ` Michael S. Tsirkin
2018-04-18 18:00           ` Michael S. Tsirkin
2018-04-18 17:49       ` Mikulas Patocka
2018-04-18 17:49       ` Mikulas Patocka
2018-04-18 16:51     ` Eric Dumazet
2018-04-18 17:46     ` David Miller
2018-04-18 17:53       ` Mikulas Patocka
2018-04-19 16:12         ` [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM Mikulas Patocka
2018-04-19 16:12         ` Mikulas Patocka
2018-04-19 16:25           ` Eric Dumazet
2018-04-19 16:25             ` Eric Dumazet
2018-04-19 16:28             ` Mikulas Patocka
2018-04-19 16:28             ` Mikulas Patocka
2018-04-19 16:43           ` Michael S. Tsirkin
2018-04-19 21:27             ` [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_SG Mikulas Patocka
2018-04-19 21:27             ` Mikulas Patocka
2018-04-19 16:43           ` [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM Michael S. Tsirkin
2018-04-19 18:28           ` Vlastimil Babka
2018-04-19 18:28           ` Vlastimil Babka
2018-04-19 19:47           ` Andrew Morton
2018-04-19 19:47           ` Andrew Morton
2018-04-19 21:19             ` Mikulas Patocka
2018-04-19 21:19               ` Mikulas Patocka
2018-04-19 23:22               ` Andrew Morton
2018-04-19 23:22               ` Andrew Morton
2018-04-20 12:16                 ` Mikulas Patocka
2018-04-20 12:16                 ` Mikulas Patocka
2018-04-20 11:47           ` Matthew Wilcox
2018-04-20 11:47             ` Matthew Wilcox
2018-04-20 12:20             ` Mikulas Patocka
2018-04-23 15:25               ` Michael S. Tsirkin
2018-04-23 15:25               ` Michael S. Tsirkin
2018-04-20 12:20             ` Mikulas Patocka
2018-04-20 13:08           ` Michal Hocko
2018-04-20 13:08             ` Michal Hocko
2018-04-20 13:41             ` Matthew Wilcox
2018-04-20 13:41             ` Matthew Wilcox
2018-04-20 13:49               ` Michal Hocko
2018-04-20 13:49               ` Michal Hocko
2018-04-20 20:56                 ` Mikulas Patocka
2018-04-20 20:56                 ` Mikulas Patocka
2018-04-20 20:54             ` Mikulas Patocka
2018-04-20 21:02               ` Matthew Wilcox
2018-04-20 21:02               ` Matthew Wilcox
2018-04-20 21:21                 ` Mikulas Patocka
2018-04-21 14:47                   ` Matthew Wilcox
2018-04-22 13:03                     ` Michal Hocko
2018-04-22 13:03                     ` Michal Hocko
2018-04-23 14:24                       ` Mikulas Patocka
2018-04-23 15:10                         ` Michal Hocko
2018-04-23 15:10                         ` Michal Hocko
2018-04-23 23:20                           ` Mikulas Patocka
2018-04-23 23:20                             ` Mikulas Patocka
2018-04-23 14:24                       ` Mikulas Patocka
2018-04-23 14:06                     ` Mikulas Patocka
2018-04-23 14:06                     ` Mikulas Patocka
2018-04-23 15:15                       ` Michal Hocko
2018-04-23 15:15                       ` Michal Hocko
2018-04-24  0:06                         ` [PATCH v3] kvmalloc: always use vmalloc if CONFIG_DEBUG_SG Mikulas Patocka
2018-04-24  0:06                         ` Mikulas Patocka
2018-04-24  2:47                           ` David Rientjes
2018-04-24 11:04                             ` Mikulas Patocka
2018-04-24 11:04                               ` Mikulas Patocka
2018-04-24  3:46                           ` Matthew Wilcox
2018-04-24  3:46                           ` Matthew Wilcox
2018-04-24 12:29                             ` Mikulas Patocka
2018-04-24 12:29                             ` Mikulas Patocka
2018-04-24 17:16                               ` Matthew Wilcox
2018-04-24 18:41                                 ` Mikulas Patocka
2018-04-24 18:41                                 ` Mikulas Patocka
2018-05-15  1:13                                   ` Joonsoo Kim
2018-05-15  1:13                                   ` Joonsoo Kim
2018-04-24 17:16                               ` Matthew Wilcox
2018-04-24 12:51                           ` Michal Hocko
2018-04-24 12:51                             ` Michal Hocko
2018-04-24 15:50                             ` Mikulas Patocka
2018-04-24 16:29                               ` Michal Hocko
2018-04-24 17:00                                 ` Mikulas Patocka
2018-04-24 17:00                                   ` Mikulas Patocka
2018-04-24 17:03                                   ` Michal Hocko
2018-04-24 17:03                                     ` Michal Hocko
2018-04-24 17:28                                     ` Mikulas Patocka
2018-04-24 17:38                                       ` Michal Hocko
2018-04-24 17:38                                       ` Michal Hocko
2018-04-25 20:02                                         ` [PATCH] fault-injection: reorder config entries Mikulas Patocka
2018-04-25 20:02                                           ` Mikulas Patocka
2018-04-26  3:21                                           ` Randy Dunlap
2018-04-26  3:21                                           ` Randy Dunlap
2018-04-25 20:02                                         ` [PATCH v4] fault-injection: introduce kvmalloc fallback options Mikulas Patocka
2018-04-25 20:02                                         ` Mikulas Patocka
2018-04-25 20:20                                           ` Randy Dunlap
2018-04-25 20:20                                             ` Randy Dunlap
2018-04-25 20:57                                             ` [PATCH v5] " Mikulas Patocka
2018-04-25 20:57                                               ` Mikulas Patocka
2018-04-25 21:11                                               ` Randy Dunlap
2018-04-25 21:11                                                 ` Randy Dunlap
2018-04-25 21:18                                               ` David Rientjes
2018-04-25 21:22                                                 ` Mikulas Patocka
2018-04-25 21:22                                                 ` Mikulas Patocka
2018-04-25 22:17                                                   ` [dm-devel] " James Bottomley
2018-04-25 22:17                                                     ` James Bottomley
2018-04-25 22:42                                                     ` Mikulas Patocka
2018-04-25 22:42                                                       ` Mikulas Patocka
2018-04-25 22:49                                                       ` David Rientjes
2018-04-25 22:56                                                         ` Mikulas Patocka
2018-04-25 22:56                                                         ` Mikulas Patocka
2018-04-26 12:58                                                       ` Michal Hocko
2018-04-26 12:58                                                         ` Michal Hocko
2018-04-26 14:28                                                         ` Mikulas Patocka
2018-04-26 14:28                                                           ` Mikulas Patocka
2018-04-26 14:45                                                           ` [dm-devel] " James Bottomley
2018-04-26 14:45                                                             ` James Bottomley
2018-04-26 15:05                                                             ` Mikulas Patocka
2018-04-26 15:05                                                               ` Mikulas Patocka
2018-04-26 15:24                                                               ` James Bottomley
2018-04-26 15:24                                                                 ` James Bottomley
2018-04-26 15:44                                                                 ` Mikulas Patocka
2018-04-26 15:44                                                                   ` Mikulas Patocka
2018-04-26 15:44                                                                   ` Mikulas Patocka
2018-04-26 15:59                                                                   ` Michael S. Tsirkin
2018-04-26 15:59                                                                     ` Michael S. Tsirkin
2018-04-26 16:07                                                                     ` Mikulas Patocka
2018-04-26 16:07                                                                       ` Mikulas Patocka
2018-04-26 18:49                                                                       ` Michael S. Tsirkin
2018-04-26 18:54                                                                         ` Mikulas Patocka
2018-04-26 18:54                                                                           ` Mikulas Patocka
2018-04-26 19:14                                                                           ` Michael S. Tsirkin
2018-04-26 19:14                                                                           ` Michael S. Tsirkin
2018-04-26 19:36                                                                             ` Mikulas Patocka
2018-04-26 19:36                                                                               ` Mikulas Patocka
2018-04-26 19:45                                                                               ` Michael S. Tsirkin
2018-04-26 19:45                                                                                 ` Michael S. Tsirkin
2018-04-26 20:05                                                                                 ` Mikulas Patocka
2018-04-26 20:05                                                                                 ` Mikulas Patocka
2018-04-26 18:49                                                                       ` Michael S. Tsirkin
2018-04-26 16:07                                                                     ` Mikulas Patocka
2018-04-26 18:58                                                                     ` Mikulas Patocka
2018-04-26 18:58                                                                     ` Mikulas Patocka
2018-04-26 19:05                                                                       ` Michael S. Tsirkin
2018-04-26 19:05                                                                       ` Michael S. Tsirkin
2018-04-26 15:59                                                                   ` Michael S. Tsirkin
2018-04-26 15:55                                                                 ` Mikulas Patocka
2018-04-26 15:55                                                                 ` Mikulas Patocka
2018-04-26 15:05                                                             ` Mikulas Patocka
2018-04-26 14:28                                                         ` Mikulas Patocka
2018-04-25 22:42                                                     ` Mikulas Patocka
2018-04-25 23:00                                                     ` Mikulas Patocka
2018-04-25 23:00                                                       ` Mikulas Patocka
2018-04-25 23:00                                                       ` Mikulas Patocka
2018-04-25 23:08                                                       ` James Bottomley
2018-04-25 23:08                                                         ` James Bottomley
2018-04-26 14:55                                                         ` Mikulas Patocka
2018-04-26 14:55                                                         ` Mikulas Patocka
2018-04-26 14:55                                                           ` Mikulas Patocka
2018-04-26 14:55                                                           ` Mikulas Patocka
2018-04-26 15:22                                                           ` Mikulas Patocka
2018-04-26 15:22                                                           ` Mikulas Patocka
2018-04-26 15:22                                                             ` Mikulas Patocka
2018-04-26 18:58                                                         ` John Stoffel
2018-04-26 21:50                                                           ` Mikulas Patocka
2018-04-26 21:50                                                             ` Mikulas Patocka
2018-04-26 22:21                                                             ` Michael S. Tsirkin
2018-04-26 22:52                                                               ` Mikulas Patocka
2018-04-26 22:52                                                                 ` Mikulas Patocka
2018-04-27  8:25                                                                 ` Michal Hocko
2018-04-27  8:25                                                                 ` Michal Hocko
2018-04-27 10:20                                                                   ` Mikulas Patocka
2018-04-27 10:20                                                                   ` Mikulas Patocka
2018-04-27 23:20                                                               ` Mikulas Patocka
2018-04-27 23:20                                                               ` Mikulas Patocka
2018-04-26 22:21                                                             ` Michael S. Tsirkin
2018-04-30 18:27                                                             ` John Stoffel
2018-04-30 21:07                                                               ` Mikulas Patocka
2018-05-02 13:38                                                                 ` John Stoffel
2018-05-03 17:40                                                                   ` Mikulas Patocka
2018-05-03 17:40                                                                   ` Mikulas Patocka
2018-04-30 21:07                                                               ` Mikulas Patocka
2018-04-24 17:28                                     ` [PATCH v3] kvmalloc: always use vmalloc if CONFIG_DEBUG_SG Mikulas Patocka
2018-04-24 16:29                               ` Michal Hocko
2018-04-24 15:50                             ` Mikulas Patocka
2018-04-24  0:25                         ` [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM Mikulas Patocka
2018-04-24 13:31                           ` Michal Hocko
2018-04-24 13:31                           ` Michal Hocko
2018-04-24 15:30                             ` Mikulas Patocka [this message]
2018-04-24 15:30                               ` Mikulas Patocka
2018-04-24 16:12                               ` Michal Hocko
2018-04-24 16:12                                 ` Michal Hocko
2018-04-24 16:29                                 ` Michal Hocko
2018-04-24 16:29                                   ` Michal Hocko
2018-04-24 16:33                                 ` Mikulas Patocka
2018-04-24 16:33                                   ` Mikulas Patocka
2018-05-02  0:36                                   ` Andrew Morton
2018-05-02  0:36                                     ` Andrew Morton
2018-05-02 13:33                                     ` Mike Snitzer
2018-05-02 13:40                                       ` [dm-devel] " John Stoffel
2018-05-02 13:33                                     ` Mike Snitzer
2018-05-03 17:32                                     ` [PATCH] " Mikulas Patocka
2018-05-03 17:32                                     ` Mikulas Patocka
2018-04-24  0:25                         ` Mikulas Patocka
2018-04-21 14:47                   ` Matthew Wilcox
2018-04-20 21:21                 ` Mikulas Patocka
2018-04-20 20:54             ` Mikulas Patocka
2018-04-18 17:53       ` [PATCH] net: don't use kvzalloc for DMA memory Mikulas Patocka
2018-04-18 17:46     ` David Miller
2018-04-18 17:45   ` David Miller
2018-04-18 17:55   ` Michael S. Tsirkin
2018-04-18 20:38     ` Eric Dumazet
2018-04-19  4:00       ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LRH.2.02.1804241107010.31601@file01.intranet.prod.int.rdu2.redhat.com \
    --to=mpatocka@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=dm-devel@redhat.com \
    --cc=edumazet@google.com \
    --cc=eric.dumazet@gmail.com \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=vbabka@suse.cz \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.