public inbox for linux-kernel@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox