linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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 16:11:29 +0530	[thread overview]
Message-ID: <50F683D9.1080908@linux.vnet.ibm.com> (raw)
In-Reply-To: <CAMSQXEE=Qp8kVcEp8nBhNDmOaDMUQUUou+5Z2f8-1znykqy6mQ@mail.gmail.com>

Hi Ivo,

On 01/16/2013 02:46 PM, Ivo Sieben wrote:
> Hi Preeti,
> 
> 2013/1/16 Preeti U Murthy <preeti@linux.vnet.ibm.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.
>>
> 
> Everytime the line discipline is dereferenced, the wakeup function is
> called. The wakeup() function contains a critical section protected by
> spinlocks. On a PREEMPT_RT system, a "normal" spinlock behaves just
> like a mutex: scheduling is not disabled and it is still possible that
> a new process on a higher RT priority is scheduled in. When a new -
> higher priority - process is scheduled in just when the put_ldisc() is
> in the critical section of the wakeup function, the higher priority
> process (that uses the same TTY instance) will finally also
> dereference the line discipline and try to wakeup the same waitqueue.
> This causes the high priority process to  block on the same spinlock.
> Priority inheritance will solve this blocked situation by a context
> switch to the lower priority process, run until that process leaves
> the critical section, and a context switch back to the higher priority
> process. This is unnecessary since the waitqueue was empty after all
> (during normal operation the waitqueue is empty most of the time).
> This unnecessary context switch from/to the high priority process is
> what a mean with "scheduler overhead" (maybe not a good name for it,
> sorry for the confusion).
> 
> Does this makes sense to you?

Yes.Thank you very much for the explanation :) But I dont see how the
context switching goes away with your patch.With your patch, when the
higher priority thread comes in when the lower priority thread is
running in the critical section,it will see the wait queue empty and
"continue its execution" without now wanting to enter the critical
section.So this means it will preempt the lower priority thread because
it is not waiting on a lock anyway.There is a context switch here right?
I dont see any problem in scheduling due to this,but I do think your
patch is essential.

The entire logic of
wakelist_active()
  wake_up()

could be integrated into wake_up(). I dont understand why we need a
separate function to check the emptiness of the wake list. But as Oleg
pointed out we must identify the places for this optimization.

Regards
Preeti U Murthy


  reply	other threads:[~2013-01-16 10:42 UTC|newest]

Thread overview: 17+ 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
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
2013-01-16  9:16             ` Ivo Sieben
2013-01-16 10:41               ` Preeti U Murthy [this message]
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=50F683D9.1080908@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 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).