All of lore.kernel.org
 help / color / mirror / Atom feed
From: Juri Lelli <juri.lelli@arm.com>
To: Byungchul Park <byungchul.park@lge.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	peterz@infradead.org, mingo@kernel.org,
	linux-kernel@vger.kernel.org, juri.lelli@gmail.com,
	bristot@redhat.com, kernel-team@lge.com
Subject: Re: [PATCH v4 1/5] sched/deadline: Refer to cpudl.elements atomically
Date: Tue, 16 May 2017 11:32:41 +0100	[thread overview]
Message-ID: <20170516103241.xony5rtmqi66gczy@e106622-lin> (raw)
In-Reply-To: <20170516065223.GA24127@X58A-UD3R>

On 16/05/17 15:52, Byungchul Park wrote:
> On Fri, May 12, 2017 at 10:25:30AM -0400, Steven Rostedt wrote:
> > On Fri, 12 May 2017 14:48:45 +0900
> > Byungchul Park <byungchul.park@lge.com> wrote:
> > 
> > > cpudl.elements is an instance that should be protected with a spin lock.
> > > Without it, the code would be insane.
> > 
> > And how much contention will this add? Spin locks in the scheduler code
> > that are shared among a domain can cause huge latency. This was why I
> > worked hard not to add any in the cpupri code.
> 
> Yes. That's also whay I hesitated to post this patch..
> 
> > > Current cpudl_find() has problems like,
> > > 
> > >    1. cpudl.elements[0].cpu might not match with cpudl.elements[0].dl.
> > >    2. cpudl.elements[0].dl(u64) might not be referred atomically.
> > >    3. Two cpudl_maximum()s might return different values.
> > >    4. It's just insane.
> > 
> > And lockless algorithms usually are insane. But locks come with a huge
> > cost. What happens when we have 32 core domains. This can cause
> > tremendous contention and makes the entire cpu priority for deadlines
> > useless. Might as well rip out the code.
> 
> I think it would be better if we, even keeping it lockless,
> 
>    1. make cp->elements[].cpu referred once than twice,
>    2. add retry logic in order to match elements[].cpu with its dl,
>    3. add retry logic for the u64 variable to be read atomically,
> 
> So what do you think about the suggestions? Of course it does not solve
> the problems perfectly though.. Or do you think it's not worth?
> 

Not sure, but if we are going to retry a lot it might be better off to
put proper locking instead? We could also simply bail out when we notice
that something is changed under our feet. I'd say (again :) we might
first want to understand (with some numbers) how bad the problem is and
then fix it. I guess numbers might also help us to better understand
what the best fix is?

Thanks,

- Juri

  reply	other threads:[~2017-05-16 10:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-12  5:48 [PATCH v4 0/5] Make find_later_rq() choose a closer cpu in topology Byungchul Park
2017-05-12  5:48 ` [PATCH v4 1/5] sched/deadline: Refer to cpudl.elements atomically Byungchul Park
2017-05-12 14:25   ` Steven Rostedt
2017-05-15  8:36     ` Juri Lelli
2017-05-16  7:00       ` Byungchul Park
2017-05-16  6:52     ` Byungchul Park
2017-05-16 10:32       ` Juri Lelli [this message]
2017-05-16 13:10         ` Steven Rostedt
2017-05-23  1:11           ` Byungchul Park
2017-05-12  5:48 ` [PATCH v4 2/5] sched/deadline: Make find_later_rq() choose a closer cpu in topology Byungchul Park
2017-05-12  5:48 ` [PATCH v4 3/5] sched/deadline: Change return value of cpudl_find() Byungchul Park
2017-05-12  5:48 ` [PATCH v4 4/5] sched/deadline: Add support for SD_PREFER_SIBLING on find_later_rq() Byungchul Park
2017-05-12  5:48 ` [PATCH v4 5/5] sched/rt: Add support for SD_PREFER_SIBLING on find_lowest_rq() Byungchul Park

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=20170516103241.xony5rtmqi66gczy@e106622-lin \
    --to=juri.lelli@arm.com \
    --cc=bristot@redhat.com \
    --cc=byungchul.park@lge.com \
    --cc=juri.lelli@gmail.com \
    --cc=kernel-team@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.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.