From: Andrew Morton <akpm@linux-foundation.org>
To: Khalid Aziz <khalid.aziz@oracle.com>
Cc: tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
peterz@infradead.org, andi.kleen@intel.com, rob@landley.net,
viro@zeniv.linux.org.uk, oleg@redhat.com,
gnomes@lxorguk.ukuu.org.uk, riel@redhat.com, snorcht@gmail.com,
dhowells@redhat.com, luto@amacapital.net, daeseok.youn@gmail.com,
ebiederm@xmission.com, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org
Subject: Re: [PATCH v2] Pre-emption control for userspace
Date: Tue, 25 Mar 2014 11:14:37 -0700 [thread overview]
Message-ID: <20140325111437.8e7b7c86.akpm@linux-foundation.org> (raw)
In-Reply-To: <5331C34F.9020604@oracle.com>
On Tue, 25 Mar 2014 11:56:31 -0600 Khalid Aziz <khalid.aziz@oracle.com> wrote:
> On 03/25/2014 11:44 AM, Andrew Morton wrote:
> > So the procfs file is written in binary format and is read back in
> > ascii format. Seems odd.
> >
> > Perhaps this should all be done as a new syscall rather than some
> > procfs thing.
> >
>
> I didn't want to add yet another syscall which will then need to be
> added to glibc, but I am open to doing it through a syscall if that is
> the consensus.
>
> >> + struct preempt_delay {
> >> + u32 __user *delay_req; /* delay request flag pointer */
> >> + unsigned char delay_granted:1; /* currently in delay */
> >> + unsigned char yield_penalty:1; /* failure to yield penalty */
> >> + } sched_preempt_delay;
> >
> > The problem with bitfields is that a write to one bitfield can corrupt
> > a concurrent write to the other one. So it's your responsibility to
> > provide locking and/or to describe how this race is avoided. A comment
> > here in the definition would be a suitable way of addressing this.
> >
>
> I do not have a strong reason to use a bitfield, just trying to not use
> any more bytes than I need to. If using a char is safer, I would rather
> use safer code.
My point is that the locking rules should be documented, via a code comment.
Presumably that rule is "only ever modified by this task".
> >> + if (delay_req) {
> >> + int ret;
> >> +
> >> + pagefault_disable();
> >> + ret = __copy_from_user_inatomic(&delay_req_flag, delay_req,
> >> + sizeof(u32));
> >> + pagefault_enable();
> >
> > This all looks rather hacky and unneccesary. Can't we somehow use
> > plain old get_user() and avoid such fuss?
>
> get_user() takes longer and can sleep if page fault occurs. I need this
> code to be very fast for it to be beneficial and am willing to ignore
> page faults since page fault would imply the task has not touched
> pre-emption delay request field and hence we can resched safely.
That's what I meant by "hacky" :)
next prev parent reply other threads:[~2014-03-25 18:14 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-03 18:07 [RFC] [PATCH] Pre-emption control for userspace Khalid Aziz
2014-03-03 21:51 ` Davidlohr Bueso
2014-03-03 23:29 ` Khalid Aziz
2014-03-04 13:56 ` Oleg Nesterov
2014-03-04 17:44 ` Khalid Aziz
2014-03-04 18:38 ` Al Viro
2014-03-04 19:01 ` Khalid Aziz
2014-03-04 19:03 ` Oleg Nesterov
2014-03-04 20:14 ` Khalid Aziz
2014-03-05 14:38 ` Oleg Nesterov
2014-03-05 16:12 ` Oleg Nesterov
2014-03-05 17:10 ` Khalid Aziz
2014-03-04 21:12 ` H. Peter Anvin
2014-03-04 21:39 ` Khalid Aziz
2014-03-04 22:23 ` One Thousand Gnomes
2014-03-04 22:44 ` Khalid Aziz
2014-03-05 0:39 ` Thomas Gleixner
2014-03-05 0:51 ` Andi Kleen
2014-03-05 11:10 ` Peter Zijlstra
2014-03-05 17:29 ` Khalid Aziz
2014-03-05 19:58 ` Khalid Aziz
2014-03-06 9:57 ` Peter Zijlstra
2014-03-06 16:08 ` Khalid Aziz
2014-03-06 11:14 ` Thomas Gleixner
2014-03-06 16:32 ` Khalid Aziz
2014-03-05 14:54 ` Oleg Nesterov
2014-03-05 15:56 ` Andi Kleen
2014-03-05 16:36 ` Oleg Nesterov
2014-03-05 17:22 ` Khalid Aziz
2014-03-05 23:13 ` David Lang
2014-03-05 23:48 ` Khalid Aziz
2014-03-05 23:56 ` H. Peter Anvin
2014-03-06 0:02 ` Khalid Aziz
2014-03-06 0:13 ` H. Peter Anvin
2014-03-05 23:59 ` David Lang
2014-03-06 0:17 ` Khalid Aziz
2014-03-06 0:36 ` David Lang
2014-03-06 1:22 ` Khalid Aziz
2014-03-06 14:23 ` David Lang
2014-03-06 12:13 ` Kevin Easton
2014-03-06 13:59 ` Peter Zijlstra
2014-03-06 22:41 ` Andi Kleen
2014-03-06 14:25 ` David Lang
2014-03-06 16:12 ` Khalid Aziz
2014-03-06 13:24 ` Rasmus Villemoes
2014-03-06 13:34 ` Peter Zijlstra
2014-03-06 13:45 ` Rasmus Villemoes
2014-03-06 14:02 ` Peter Zijlstra
2014-03-06 14:33 ` Thomas Gleixner
2014-03-06 14:34 ` H. Peter Anvin
2014-03-06 14:04 ` Thomas Gleixner
2014-03-25 17:17 ` [PATCH v2] " Khalid Aziz
2014-03-25 17:44 ` Andrew Morton
2014-03-25 17:56 ` Khalid Aziz
2014-03-25 18:14 ` Andrew Morton [this message]
2014-03-25 17:46 ` Oleg Nesterov
2014-03-25 17:59 ` Khalid Aziz
2014-03-25 18:20 ` Andi Kleen
2014-03-25 18:47 ` Khalid Aziz
2014-03-25 19:47 ` Andi Kleen
2014-03-25 18:59 ` Eric W. Biederman
2014-03-25 19:15 ` Khalid Aziz
2014-03-25 20:31 ` Eric W. Biederman
2014-03-25 21:37 ` Khalid Aziz
2014-03-26 6:03 ` Mike Galbraith
2014-03-25 23:01 ` [RFC] [PATCH] " Davidlohr Bueso
2014-03-25 23:29 ` Khalid Aziz
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=20140325111437.8e7b7c86.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=andi.kleen@intel.com \
--cc=daeseok.youn@gmail.com \
--cc=dhowells@redhat.com \
--cc=ebiederm@xmission.com \
--cc=gnomes@lxorguk.ukuu.org.uk \
--cc=hpa@zytor.com \
--cc=khalid.aziz@oracle.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mingo@redhat.com \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=riel@redhat.com \
--cc=rob@landley.net \
--cc=snorcht@gmail.com \
--cc=tglx@linutronix.de \
--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 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).