All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <haveblue@us.ibm.com>
To: linux-kernel@vger.kernel.org
Cc: "Martin J. Bligh" <Martin.Bligh@us.ibm.com>
Subject: BKL in do_exit
Date: Wed, 27 Mar 2002 10:16:59 -0800	[thread overview]
Message-ID: <3CA20C9B.20309@us.ibm.com> (raw)

I was asked by a coworker why the BKL was held in do_exit().  I didn't 
have a good answer for him, so I went digging and found a couple of 
Linus quotations on the subject (thank you Google).

 > And the do_exit() case should be _trivial_ to fix: almost none of the
 > code protected by the kernel lock in the exit path actually needs the
 > lock. I suspect you could cut down the kernel lock there to much
 > smaller.

> Andrew Morton  <akpm@zip.com.au> wrote:
>>That'll be where exit() takes down the tasks's address spaces.  
>>zap_page_range().  That's a nasty one.
 > No, lock_kernel happens after exit_mm, and in fact I suspect it's not
 > really needed at all any more except for the current "sem_exit()". I
 > think most everything else is threaded already.
 >
 > (Hmm.. Maybe disassociate_ctty() too).
 >
 > So minimizing the BLK footprint in do_exit() should be pretty much
 > trivial: all the really interesting stuff should be ok already.

Because of Linus's optimism, I went looking at do_exit(). Here is my 
take on it:

         lock_kernel();
         sem_exit();
// This is questionable, but appears safe to me.  Several
// sem_(un)lock()s are done the semid.  What else needs to be protected
// here?  What was Linus talking about that looks unsafe?  Is there a
// chance for another process to be mucking with the current process's
// semaphore lists?

// these hold the task lock and look safe
         __exit_files(tsk);
         __exit_fs(tsk);

         exit_sighand(tsk); // does spin_lock_irq(&tsk->sigmask_lock);
                            // looks safe			

         exit_thread(); // nop on most architectures
                        // FPU cleanup on most others
                        // looks safe

// there is no locking of the tty.  As Linus said, this appears to
// be the bad one now
         if (current->leader)
                 disassociate_ctty(1);

// I can't see why these would need BKL. Looks safe
         put_exec_domain(tsk->exec_domain);
         if (tsk->binfmt && tsk->binfmt->module)
                 __MOD_DEC_USE_COUNT(tsk->binfmt->module);

// Does the task lock need to be taken for this?
         tsk->exit_code = code;

// Does this need to make sure that none of the relatives exit?
         exit_notify();

// BKL implicitly released by calling this, if it is held
         schedule();

Can we just hold the BKL around the operations that actually need it? 
Is there any other reason to hold it the whole time?

P.S. There are some really fork-happy benchmarks which could see an 
improvement from a reduction of lock contention here.  It isn't just 
theoretical.
-- 
Dave Hansen
haveblue@us.ibm.com


                 reply	other threads:[~2002-03-27 18:17 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=3CA20C9B.20309@us.ibm.com \
    --to=haveblue@us.ibm.com \
    --cc=Martin.Bligh@us.ibm.com \
    --cc=linux-kernel@vger.kernel.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.