From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
To: Ivo Sieben <meltedpianoman@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
linux-kernel@vger.kernel.org, Andi Kleen <andi@firstfloor.org>,
Oleg Nesterov <oleg@redhat.com>, Jiri Slaby <jslaby@suse.cz>,
Alan Cox <alan@lxorguk.ukuu.org.uk>,
linux-serial@vger.kernel.org, Alan Cox <alan@linux.intel.com>,
Greg KH <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] tty: Only wakeup the line discipline idle queue when queue is active
Date: Wed, 16 Jan 2013 13:43:20 +0530 [thread overview]
Message-ID: <50F66120.50704@linux.vnet.ibm.com> (raw)
In-Reply-To: <CAMSQXEGU2WmhmMqP7XMK2x39GN9iDDWpg-Wa7_cAauHWx5OWbQ@mail.gmail.com>
Hi Ivo,
Can you explain how this problem could create a scheduler overhead?
I am a little confused, because as far as i know,scheduler does not come
in the picture of the wake up path right? select_task_rq() in
try_to_wake_up() is where the scheduler comes in,and this is after the
task wakes up.
On 01/03/2013 03:19 PM, Ivo Sieben wrote:
> Oleg, Peter, Ingo, Andi & Preeti,
>
> 2013/1/2 Jiri Slaby <jslaby@suse.cz>:
>> On 01/02/2013 04:21 PM, Ivo Sieben wrote:
>>> I don't understand your responses: do you suggest to implement this
>>> "if active" behavior in:
>>> * A new wake_up function called wake_up_if_active() that is part of
>>> the waitqueue layer?
>>
>> Sounds good.
>>
>> --
>> js
>> suse labs
>
> I want to ask you 'scheduler' people for your opinion:
>
> Maybe you remember my previous patch where I suggested an extra
> 'waitqueue empty' check before entering the critical section of the
> wakeup() function (If you do not remember see
> https://lkml.org/lkml/2012/10/25/159)
>
> Finally Oleg responded that a lot of callers do
>
> if (waitqueue_active(q))
> wake_up(...);
>
> what made my patch pointless and adds a memory barrier. I then decided
> to also implement the 'waitqueue_active' approach for my problem.
>
> But now I get a review comment by Jiri that he would like to hide this
> 'if active behavior' in a wake_up_if_active() kind of function. I
> think he is right that implementing this check in the wakeup function
> would clean things up, right?
>
> I would like to have your opinion on the following two suggestions:
> - We still can do the original patch on the wake_up() that I
> suggested. I then can do an additional code cleanup patch that removes
> the double 'waitqueue_active' call (a quick grep found about 150 of
> these waitqueue active calls) on several places in the code.
I think this is a good move.
> - Or - as an alternative - I could add extra _if_active() versions of
> all wake_up() functions, that implement this extra test.
Why add 'extra' if_active versions? Why not optimize this within the
existing wake_up() functions?
>
> Regards,
> Ivo
Regards
Preeti U Murthy
>
next prev parent reply other threads:[~2013-01-16 8:14 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-18 14:48 [PATCH] tty: Only wakeup the line discipline idle queue when queue is active Ivo Sieben
2012-12-18 14:48 ` Ivo Sieben
2013-01-02 9:29 ` Jiri Slaby
2013-01-02 11:43 ` Alan Cox
2013-01-02 15:21 ` Ivo Sieben
2013-01-02 19:06 ` Jiri Slaby
2013-01-03 9:49 ` Ivo Sieben
2013-01-03 18:36 ` Oleg Nesterov
2013-01-15 9:16 ` Ivo Sieben
2013-01-15 18:03 ` Oleg Nesterov
2013-01-16 8:13 ` Preeti U Murthy [this message]
2013-01-16 9:16 ` Ivo Sieben
2013-01-16 10:41 ` Preeti U Murthy
2013-01-16 12:02 ` Ivo Sieben
2013-01-17 10:56 ` Preeti U Murthy
2013-01-18 15:45 ` Oleg Nesterov
2013-01-21 2:56 ` Preeti U Murthy
2013-01-21 7:20 ` Ivo Sieben
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=50F66120.50704@linux.vnet.ibm.com \
--to=preeti@linux.vnet.ibm.com \
--cc=alan@linux.intel.com \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=andi@firstfloor.org \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=meltedpianoman@gmail.com \
--cc=mingo@redhat.com \
--cc=oleg@redhat.com \
--cc=peterz@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.