All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Thorlton <athorlton@sgi.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: linux-mm@kvack.org, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Rik van Riel <riel@redhat.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Kees Cook <keescook@chromium.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] mm: thp: Add per-mm_struct flag to control THP
Date: Mon, 13 Jan 2014 12:59:44 -0600	[thread overview]
Message-ID: <20140113185944.GD10649@sgi.com> (raw)
In-Reply-To: <20140112135600.GA15051@redhat.com>

On Sun, Jan 12, 2014 at 02:56:00PM +0100, Oleg Nesterov wrote:
> On 01/11, Alex Thorlton wrote:
> >
> > On Sat, Jan 11, 2014 at 04:53:37PM +0100, Oleg Nesterov wrote:
> >
> > > I simply can't understand, this all looks like overkill. Can't you simply add
> > >
> > > 	#idfef CONFIG_TRANSPARENT_HUGEPAGE
> > > 	case GET:
> > > 		error = test_bit(MMF_THP_DISABLE);
> > > 		break;
> > > 	case PUT:
> > > 		if (arg2)
> > > 			set_bit();
> > > 		else
> > > 			clear_bit();
> > > 		break;
> > > 	#endif
> > >
> > > into sys_prctl() ?	
> >
> > That's probably a better solution.  I wasn't sure whether or not it was
> > better to have two functions to handle this, or to have one function
> > handle both.  If you think it's better to just handle both with one,
> > that's easy enough to change.
> 
> Personally I think sys_prctl() can handle this itself, without a helper.
> But of course I won't argue, this is up to you.
> 
> My only point is, the kernel is already huge ;) Imho it makes sense to
> try to lessen the code size, when the logic is simple.

I agree with you here as well.  There was a mixed bag of PRCTLs using
helpers vs. ones that put the code right into sys_prctl.  I just
arbitrarily chose to use a helper here.  I'll switch that over for v2.

- Alex

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Alex Thorlton <athorlton@sgi.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: linux-mm@kvack.org, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Rik van Riel <riel@redhat.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Kees Cook <keescook@chromium.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] mm: thp: Add per-mm_struct flag to control THP
Date: Mon, 13 Jan 2014 12:59:44 -0600	[thread overview]
Message-ID: <20140113185944.GD10649@sgi.com> (raw)
In-Reply-To: <20140112135600.GA15051@redhat.com>

On Sun, Jan 12, 2014 at 02:56:00PM +0100, Oleg Nesterov wrote:
> On 01/11, Alex Thorlton wrote:
> >
> > On Sat, Jan 11, 2014 at 04:53:37PM +0100, Oleg Nesterov wrote:
> >
> > > I simply can't understand, this all looks like overkill. Can't you simply add
> > >
> > > 	#idfef CONFIG_TRANSPARENT_HUGEPAGE
> > > 	case GET:
> > > 		error = test_bit(MMF_THP_DISABLE);
> > > 		break;
> > > 	case PUT:
> > > 		if (arg2)
> > > 			set_bit();
> > > 		else
> > > 			clear_bit();
> > > 		break;
> > > 	#endif
> > >
> > > into sys_prctl() ?	
> >
> > That's probably a better solution.  I wasn't sure whether or not it was
> > better to have two functions to handle this, or to have one function
> > handle both.  If you think it's better to just handle both with one,
> > that's easy enough to change.
> 
> Personally I think sys_prctl() can handle this itself, without a helper.
> But of course I won't argue, this is up to you.
> 
> My only point is, the kernel is already huge ;) Imho it makes sense to
> try to lessen the code size, when the logic is simple.

I agree with you here as well.  There was a mixed bag of PRCTLs using
helpers vs. ones that put the code right into sys_prctl.  I just
arbitrarily chose to use a helper here.  I'll switch that over for v2.

- Alex

  reply	other threads:[~2014-01-13 18:59 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-10 19:55 [RFC PATCH] mm: thp: Add per-mm_struct flag to control THP Alex Thorlton
2014-01-10 19:55 ` Alex Thorlton
2014-01-10 20:23 ` Kirill A. Shutemov
2014-01-10 20:23   ` Kirill A. Shutemov
2014-01-10 22:01   ` Alex Thorlton
2014-01-10 22:01     ` Alex Thorlton
2014-01-10 22:10     ` Peter Zijlstra
2014-01-10 22:10       ` Peter Zijlstra
2014-01-10 22:39       ` Alex Thorlton
2014-01-10 22:39         ` Alex Thorlton
2014-01-14 15:44         ` Mel Gorman
2014-01-14 15:44           ` Mel Gorman
2014-01-14 19:38           ` Alex Thorlton
2014-01-14 19:38             ` Alex Thorlton
2014-01-22 10:26             ` Mel Gorman
2014-01-22 10:26               ` Mel Gorman
2014-01-22 17:53               ` Alex Thorlton
2014-01-22 17:53                 ` Alex Thorlton
2014-01-22 21:46                 ` David Rientjes
2014-01-22 21:46                   ` David Rientjes
2014-01-10 22:23     ` Kirill A. Shutemov
2014-01-10 22:23       ` Kirill A. Shutemov
2014-01-14 15:47       ` Mel Gorman
2014-01-14 15:47         ` Mel Gorman
2014-01-11 16:11   ` Oleg Nesterov
2014-01-11 16:11     ` Oleg Nesterov
2014-01-11 15:53 ` Oleg Nesterov
2014-01-11 15:53   ` Oleg Nesterov
2014-01-11 19:30   ` Alex Thorlton
2014-01-11 19:30     ` Alex Thorlton
2014-01-12 13:56     ` Oleg Nesterov
2014-01-12 13:56       ` Oleg Nesterov
2014-01-13 18:59       ` Alex Thorlton [this message]
2014-01-13 18:59         ` Alex Thorlton

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=20140113185944.GD10649@sgi.com \
    --to=athorlton@sgi.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=ebiederm@xmission.com \
    --cc=keescook@chromium.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=mingo@redhat.com \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /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.