linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrea Arcangeli <aarcange@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>,
	"Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>,
	"Rafael J. Wysocki" <rjw@suse.com>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jiri Slaby <jirislaby@gmail.com>,
	linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] thp: reduce khugepaged freezing latency
Date: Wed, 9 Nov 2011 19:19:25 +0100	[thread overview]
Message-ID: <20111109181925.GN5075@redhat.com> (raw)
In-Reply-To: <20111109180900.GF1260@google.com>

On Wed, Nov 09, 2011 at 10:09:00AM -0800, Tejun Heo wrote:
> I'm confused.  You're doing add_wait_queue() before
> schedule_timeout_interruptible().  prepare_to_wait() is essentially
> add_wait_queue() + set_current_state().  What am I missing?  ie. why
> not do the following?

Ah the reason of the waitqueue is the sysfs store, to get out of there
if somebody decreases the wait time from 1min to 10sec or
similar. It's not really needed for other things, in theory it could
be a separate waitqueue just for sysfs but probably not worth it.

> 	prepare_to_wait(INTERRUPTIBLE);
> 	try_to_freeze();
> 	schedule_timeout();
> 	try_to_freeze();
> 	finish_wait();
> 
> or even simpler,
> 
> 	wait_event_freezable_timeout(wq, false, timeout);
> 
> In terms of overhead, there is no appreciable difference from
> 
> 	add_wait_queue();
> 	schedule_timeout_interruptible();
> 	remove_wait_queue()
> 
> Or is the logic there scheduled to change?

I have no "event" to wait other than the wakeup itself, this in the
end is the only reason it isn't already using
wait_event_freezable_timeout. Of course I can pass "false" as the
event.

> Hmmm... I don't know.  I really hope all freezable tasks stick to
> higher level interface.  It's way too easy to get things wrong and eat
> either freezing or actual wakeup condition.

Well you've just to tell me if I have to pass "false" and if
add_wait_queue+schedule_timeout_interruptible is obsoleted. If it's
not obsoleted the patch I posted should already be ok. It also will be
useful if others need to wait for a long time (> the freezer max wait)
without a waitqueue which I don't think is necessarily impossible. It
wasn't the case here just because I need to promptly react to the
sysfs writes (or setting the wait time to 1 day would then require 1
day before sysfs new value becomes meaningful, well unless somebody
doess killall khugepaged.. :)

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-11-09 18:19 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-08  8:33 khugepaged doesn't want to freeze Jiri Slaby
2011-11-08 15:29 ` Andrea Arcangeli
2011-11-08 15:29   ` [PATCH] thp: reduce khugepaged freezing latency Andrea Arcangeli
2011-11-08 20:01     ` Srivatsa S. Bhat
2011-11-09  0:01       ` Andrea Arcangeli
2011-11-09  9:03         ` Srivatsa S. Bhat
2011-11-09 12:45       ` Srivatsa S. Bhat
2011-11-09 15:53         ` Tejun Heo
2011-11-09 16:20           ` Srivatsa S. Bhat
2011-11-09 16:52           ` Andrea Arcangeli
2011-11-09 16:59             ` Tejun Heo
2011-11-09 17:02               ` Tejun Heo
2011-11-09 17:29                 ` Andrea Arcangeli
2011-11-09 18:09                   ` Tejun Heo
2011-11-09 18:19                     ` Andrea Arcangeli [this message]
2011-11-09 18:34                       ` Tejun Heo
2011-11-09 19:40                         ` Andrea Arcangeli
2011-11-11 12:20                           ` Jiri Slaby
2011-11-09 17:06               ` Tejun Heo
2011-11-09 17:33                 ` Andrea Arcangeli

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=20111109181925.GN5075@redhat.com \
    --to=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=jirislaby@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=rjw@suse.com \
    --cc=srivatsa.bhat@linux.vnet.ibm.com \
    --cc=tj@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).