public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@digeo.com>
To: Matthew Wilcox <willy@debian.org>
Cc: John Levon <levon@movementarian.org>, linux-kernel@vger.kernel.org
Subject: Re: flock(fd, LOCK_UN) taking 500ms+ ?
Date: Wed, 02 Oct 2002 10:04:52 -0700	[thread overview]
Message-ID: <3D9B2734.D983E835@digeo.com> (raw)
In-Reply-To: 20021002141435.A18377@parcelfarce.linux.theplanet.co.uk

Matthew Wilcox wrote:
> 
> On Wed, Oct 02, 2002 at 04:23:27AM +0100, John Levon wrote:
> > --- linux-linus/fs/locks.c    Sat Sep 28 15:56:28 2002
> > +++ linux/fs/locks.c  Wed Oct  2 04:15:54 2002
> > @@ -727,11 +727,11 @@
> >       }
> >       unlock_kernel();
> >
> > -     if (found)
> > -             yield();
> > -
> >       if (new_fl->fl_type == F_UNLCK)
> >               return 0;
> > +
> > +     if (found)
> > +             yield();
> >
> >       lock_kernel();
> >       for_each_lock(inode, before) {
> >
> > "fixes" the problem (a simultaneous kernel compile is going on; it's a
> > 2-way SMP machine). What is the yield for ? What's the right fix (if
> > any) ? This turns our app's main loop from a second or two into a
> > 90-second behemoth.
> 
> I'm pretty sure this is correct.  There's no particular reason to yield()
> if we're unlocking.
> 
> I wonder if yield() is the correct call to make anyway.  We certainly need
> to schedule() to allow any higher-priority task the opportunity to run.
> But if we're the highest-priority task downgrading our write-lock to
> a read-lock which permits other tasks the opportunity to run, i see no
> reason we should _yield_ to them.
> 

sched_yield() sementics changed a lot.  It used to mean "take a quick
nap", but it now means "go to the back of the runqueue and stay there
for absolutely ages".  The latter is a closer interpretation of the spec,
but it has broken stuff which was tuned to the old behaviour.

It's not really clear why that yield is in there at all?  Unless that
code is really, really slow (milliseconds) then probably it should just
be deleted.

If there really is a solid need to hand the CPU over to some now-runnable
higher-priority process then a cond_resched() will suffice.

  reply	other threads:[~2002-10-02 16:59 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-10-02  2:39 flock(fd, LOCK_UN) taking 500ms+ ? John Levon
2002-10-02  3:23 ` John Levon
2002-10-02 13:14   ` Matthew Wilcox
2002-10-02 17:04     ` Andrew Morton [this message]
2002-10-02 18:30       ` Matthew Wilcox
2002-10-02 18:58         ` John Levon
2002-10-02 19:10           ` Robert Love
2002-10-02 19:21         ` Robert Love
2002-10-02 19:23         ` Andrew Morton
2002-10-02 20:05           ` Matthew Wilcox
  -- strict thread matches above, loose matches on Subject: below --
2002-10-02 21:36 Manfred Spraul

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=3D9B2734.D983E835@digeo.com \
    --to=akpm@digeo.com \
    --cc=levon@movementarian.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=willy@debian.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