All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Jones <davej@redhat.com>
To: Hugh Dickins <hughd@google.com>
Cc: Dave Hansen <dave@sr71.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Wu Fengguang <fengguang.wu@intel.com>, Jan Kara <jack@suse.cz>,
	Mel Gorman <mgorman@suse.de>,
	linux-mm@kvack.org, Andi Kleen <ak@linux.intel.com>,
	Matthew Wilcox <willy@linux.intel.com>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Hillf Danton <dhillf@gmail.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/8] thp, mm: locking tail page is a bug
Date: Wed, 17 Jul 2013 22:04:14 -0400	[thread overview]
Message-ID: <20130718020414.GA1786@redhat.com> (raw)
In-Reply-To: <alpine.LNX.2.00.1307171747210.23502@eggly.anvils>

On Wed, Jul 17, 2013 at 05:58:13PM -0700, Hugh Dickins wrote:
 > On Wed, 17 Jul 2013, Dave Hansen wrote:
 > > On 07/17/2013 02:09 PM, Andrew Morton wrote:
 > > > lock_page() is a pretty commonly called function, and I assume quite a
 > > > lot of people run with CONFIG_DEBUG_VM=y.
 > > > 
 > > > Is the overhead added by this patch really worthwhile?
 > > 
 > > I always thought of it as a developer-only thing.  I don't think any of
 > > the big distros turn it on by default.
 > 
 > That's how I think of it too (and the problem is often that too few mm
 > developers turn it on); but Dave Jones did confirm last November that
 > Fedora turns it on.
 > 
 > I believe Fedora turns it on to help us all, and wouldn't mind a mere
 > VM_BUG_ON(PageTail(page)) in __lock_page() if it's helpful to Kirill.
 > 
 > But if VM_BUG_ONs become expensive, I do think it's for Fedora to
 > turn off CONFIG_DEBUG_VM, rather than for mm developers to avoid it.

I'm ambivalent about whether we keep it on or off, we have no shortage
of bugs to fix already, though I think as mentioned above, very few people
actually enable it, so we're going to lose a lot of testing.

Another idea, perhaps is an extra config option for more expensive debug options ?

	Dave



WARNING: multiple messages have this Message-ID (diff)
From: Dave Jones <davej@redhat.com>
To: Hugh Dickins <hughd@google.com>
Cc: Dave Hansen <dave@sr71.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Wu Fengguang <fengguang.wu@intel.com>, Jan Kara <jack@suse.cz>,
	Mel Gorman <mgorman@suse.de>,
	linux-mm@kvack.org, Andi Kleen <ak@linux.intel.com>,
	Matthew Wilcox <willy@linux.intel.com>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Hillf Danton <dhillf@gmail.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/8] thp, mm: locking tail page is a bug
Date: Wed, 17 Jul 2013 22:04:14 -0400	[thread overview]
Message-ID: <20130718020414.GA1786@redhat.com> (raw)
In-Reply-To: <alpine.LNX.2.00.1307171747210.23502@eggly.anvils>

On Wed, Jul 17, 2013 at 05:58:13PM -0700, Hugh Dickins wrote:
 > On Wed, 17 Jul 2013, Dave Hansen wrote:
 > > On 07/17/2013 02:09 PM, Andrew Morton wrote:
 > > > lock_page() is a pretty commonly called function, and I assume quite a
 > > > lot of people run with CONFIG_DEBUG_VM=y.
 > > > 
 > > > Is the overhead added by this patch really worthwhile?
 > > 
 > > I always thought of it as a developer-only thing.  I don't think any of
 > > the big distros turn it on by default.
 > 
 > That's how I think of it too (and the problem is often that too few mm
 > developers turn it on); but Dave Jones did confirm last November that
 > Fedora turns it on.
 > 
 > I believe Fedora turns it on to help us all, and wouldn't mind a mere
 > VM_BUG_ON(PageTail(page)) in __lock_page() if it's helpful to Kirill.
 > 
 > But if VM_BUG_ONs become expensive, I do think it's for Fedora to
 > turn off CONFIG_DEBUG_VM, rather than for mm developers to avoid it.

I'm ambivalent about whether we keep it on or off, we have no shortage
of bugs to fix already, though I think as mentioned above, very few people
actually enable it, so we're going to lose a lot of testing.

Another idea, perhaps is an extra config option for more expensive debug options ?

	Dave


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2013-07-18  2:54 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-15 10:47 [PATCH, REBASED 0/8] Transparent huge page cache: phase 0, prep work Kirill A. Shutemov
2013-07-15 10:47 ` Kirill A. Shutemov
2013-07-15 10:47 ` [PATCH 1/8] mm: drop actor argument of do_generic_file_read() Kirill A. Shutemov
2013-07-15 10:47   ` Kirill A. Shutemov
2013-07-16  3:31   ` Wanpeng Li
2013-07-16  3:31   ` Wanpeng Li
2013-07-16  3:31     ` Wanpeng Li
2013-07-16 19:10   ` Matthew Wilcox
2013-07-16 19:10     ` Matthew Wilcox
2013-07-16 23:13     ` Kirill A. Shutemov
2013-07-16 23:13       ` Kirill A. Shutemov
2013-07-15 10:47 ` [PATCH 2/8] thp, mm: avoid PageUnevictable on active/inactive lru lists Kirill A. Shutemov
2013-07-15 10:47   ` Kirill A. Shutemov
2013-07-15 10:47 ` [PATCH 3/8] thp: account anon transparent huge pages into NR_ANON_PAGES Kirill A. Shutemov
2013-07-15 10:47   ` Kirill A. Shutemov
2013-07-15 10:47 ` [PATCH 4/8] mm: cleanup add_to_page_cache_locked() Kirill A. Shutemov
2013-07-15 10:47   ` Kirill A. Shutemov
2013-07-15 13:55   ` Kirill A. Shutemov
2013-07-15 13:55     ` Kirill A. Shutemov
2013-07-15 10:47 ` [PATCH 5/8] thp, mm: locking tail page is a bug Kirill A. Shutemov
2013-07-15 10:47   ` Kirill A. Shutemov
2013-07-17 21:09   ` Andrew Morton
2013-07-17 21:09     ` Andrew Morton
2013-07-17 22:43     ` Dave Hansen
2013-07-17 22:43       ` Dave Hansen
2013-07-18  0:58       ` Hugh Dickins
2013-07-18  0:58         ` Hugh Dickins
2013-07-18  2:04         ` Dave Jones [this message]
2013-07-18  2:04           ` Dave Jones
2013-07-18 19:59           ` Hugh Dickins
2013-07-18 19:59             ` Hugh Dickins
2013-07-17 22:45     ` Kirill A. Shutemov
2013-07-17 22:45       ` Kirill A. Shutemov
2013-07-15 10:47 ` [PATCH 6/8] thp: move maybe_pmd_mkwrite() out of mk_huge_pmd() Kirill A. Shutemov
2013-07-15 10:47   ` Kirill A. Shutemov
2013-07-15 10:47 ` [PATCH 7/8] thp: do_huge_pmd_anonymous_page() cleanup Kirill A. Shutemov
2013-07-15 10:47   ` Kirill A. Shutemov
2013-07-15 10:47 ` [PATCH 8/8] thp: consolidate code between handle_mm_fault() and do_huge_pmd_anonymous_page() Kirill A. Shutemov
2013-07-15 10:47   ` Kirill A. Shutemov
  -- strict thread matches above, loose matches on Subject: below --
2013-06-11 15:35 [PATCH 0/8] Transparent huge page cache: phase 0, prep work Kirill A. Shutemov
2013-06-11 15:35 ` [PATCH 5/8] thp, mm: locking tail page is a bug Kirill A. Shutemov
2013-06-11 15:35   ` Kirill A. Shutemov

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=20130718020414.GA1786@redhat.com \
    --to=davej@redhat.com \
    --cc=aarcange@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave@sr71.net \
    --cc=dhillf@gmail.com \
    --cc=fengguang.wu@intel.com \
    --cc=hughd@google.com \
    --cc=jack@suse.cz \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kirill@shutemov.name \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@linux.intel.com \
    /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.