lock_kernel check...

lock_kernel check...

Post by Dave Hanse » Wed, 10 Jul 2002 18:20:07



cc'ing LKML 'cause this is interesting...

 > As you can see, the attached script is dead simple.  It prints an
 > error every time you call return while lock_kernel is held.  On
 > your computer you will want to comment out print_url() and
 > uncomment the regular print statement.

I am continually amazed at all the simple, useful, cool stuff that
people come up with.  I like!

 > I tested it on linux-2.5.7 because then I could just put a link to
 > the offending lines and get your feedback.  I compiled fs/*/*.c

Time to run it on some more recent versions.  Don't worry about not
giving links, filename:line# is just fine for most people.  I promise
_I_ won't complain :)

time for the breakdown:
  > http://lxr.linux.no/source/fs/affs/namei.c?v=2.5.7#L349
error

 > http://lxr.linux.no/source/fs/hpfs/dir.c?v=2.5.7#L194
error

 > http://lxr.linux.no/source/fs/intermezzo/dir.c?v=2.5.7#L580
 > http://lxr.linux.no/source/fs/intermezzo/dir.c?v=2.5.7#L594
 > http://lxr.linux.no/source/fs/intermezzo/file.c?v=2.5.7#L303
 > http://lxr.linux.no/source/fs/intermezzo/vfs.c?v=2.5.7#L1952
 > http://lxr.linux.no/source/fs/intermezzo/vfs.c?v=2.5.7#L2053

Intermezzo is doing some subtle things here.  It needs to do some
dentry lookups and doesn't want to deadlock with dcache_lock (I
think).  It releases the BKL to do these, then reacquires it so that
the caller function doesn't double-unlock.  These are weird, but
correct.

 > http://lxr.linux.no/source/fs/jbd/journal.c?v=2.5.7#L270

Another subtle one.  This is a throwback to the earliest days of the
BKL where kernel daemons did a lock_kernel() while they were
executing, and an unlock_kernel() when they were ready to give up the
CPU.  This is still the case for kjournald, but in a much more
convoluted way.  Since kjournald is a thread, when it returns, the
process is destroyed and the BKL is released implcitly during the exit
sequence.  That is why you never see an unlock_kernel().

Here's the basic process (may god help us all):

kernel_thread(kjournald,...) // start the new kjournald thread
lock_kernel()
work: // do work

// go to sleep for some reason
schedule()
        unlock_kernel()
        sleeping...
        wake_up()
        lock_kernel()

if( more to do )
        goto work;
else
        exit()
        unlock_kernel().

 > The question is are any of these actual errors?  What other things
 > are possibly illegal to do while lock_kernel is held?

It isn't absoulutely a bad thing to return while you have a lock held.
    It might be hard to understand, or even crazy, but not immediately
wrong :)

// BKL protects both "a", and io port 0xF00D
bar()
{
        lock_kernel();
        return inb(0xF00D);

Quote:}

int a;
foo()
{
        a = bar();
        a--;
        unlock_kernel();

Quote:}

But, back to the subject at hand.  Yes, you've found some errors here
because in almost all normal cases, you don't mean to return with a
lock still held.   These were most likely caused by myself or others
pushing the BKL into these functions from above and are bugs.

Would you like to do the patches to fix it, or do you want me to do
them?  They shouldn't be hard to do.

What is smatch.pm? I can't find it anywhere.

--
Dave Hansen

[ lock-check.pl < 1K ]
#!/usr/bin/perl -w
use smatch;

sub print_url {
  if (get_filename() =~ /home\/carp0110\/progs\/kernel\/devel\/linux-2.5.7\/(.*)/){
    print 'http://lxr.linux.no/source/', $1, '?v=2.5.7#', get_lineno(), "\n";
  }

Quote:}

while ($data = get_data()){
  # if we see a lock_kernel then we set the state to "locked"
  if ($data =~ /call_expr\(\(addr_expr function_decl\(lock_kernel/){
    set_state("locked");
    next;
  }

  # if we see an unlock_kernel we set the state to "unlocked"
  if ($data =~ /call_expr\(\(addr_expr function_decl\(unlock_kernel/){
    set_state("unlocked");
    next;
  }

  # if we see a return statement and the kernel is
  # locked then we print a mesg.
  if ($data =~ /^return_stmt/){
    if (get_state() =~ /^locked$/){
      #print "Not unlocked: ", get_filename(), " ", get_start_line(), " ", get_lineno(), "\n";
      print_url();
      next;
    }
  }

Quote:}

 
 
 

lock_kernel check...

Post by Zwane Mwaikamb » Wed, 10 Jul 2002 19:20:10



> It isn't absoulutely a bad thing to return while you have a lock held.
>     It might be hard to understand, or even crazy, but not immediately
> wrong :)

> // BKL protects both "a", and io port 0xF00D
> bar()
> {
>    lock_kernel();
>    return inb(0xF00D);
> }

> int a;
> foo()
> {
>    a = bar();
>    a--;
>    unlock_kernel();
> }

But broken nonetheless, that kinda thing just looks ugly. Especially when
someone tries to call bar multiple times or consecutively or with the lock
already held or...

        Zwane Mwaikambo

--
function.linuxpower.ca

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

lock_kernel check...

Post by Dave Hanse » Thu, 11 Jul 2002 02:10:11




>>It isn't absoulutely a bad thing to return while you have a lock held.
>>    It might be hard to understand, or even crazy, but not immediately
>>wrong :)

>>// BKL protects both "a", and io port 0xF00D
>>bar()
>>{
>>        lock_kernel();
>>        return inb(0xF00D);
>>}

>>int a;
>>foo()
>>{
>>        a = bar();
>>        a--;
>>        unlock_kernel();
>>}

> But broken nonetheless, that kinda thing just looks ugly. Especially when
> someone tries to call bar multiple times or consecutively or with the lock
> already held or...

Yes, it is horribly ugly, but it is not broken.  As a function, if you
document what you require your caller to do, there shouldn't be a
problem.

Also, it is valid to have nested holds of the BKL.  You can never
deadlock with another lock_kernel() which was done in the same process.

--
Dave Hansen

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

lock_kernel check...

Post by dan carpente » Thu, 11 Jul 2002 02:30:11


----- Original Message -----

Date: Tue, 09 Jul 2002 02:08:18 -0700

Subject: Re: lock_kernel check...

> cc'ing LKML 'cause this is interesting...


>  > As you can see, the attached script is dead simple.  It prints an
>  > error every time you call return while lock_kernel is held.  On
>  > your computer you will want to comment out print_url() and
>  > uncomment the regular print statement.

> I am continually amazed at all the simple, useful, cool stuff that
> people come up with.  I like!

Glad you liked it.  :)

Smatch.pm is from the smatch.sf.net scripts page.  Smatch is a really unfinished code checker that I've been working on.  It is based on reading the papers about the Stanford checker.  

Unfortunately, after a night of sleep I realize that my script is broken for 2 reasons.  
1)  Smatch.pm is meant to track state changes down different code paths.  But unfortunately it wasn't doing that in this case; it was just going down the code without taking into consideration any if_stmts  etc.  I'm extremely embarassed about that.  Sorry.  
2)  What the Stanford checker does is print an error if one return_stmt is called while the kernel is locked and one is called while the kernel is unlocked.  This seems reasonable.

I will fix both mistakes later on this week.  Unfortunately I'm in the process of moving and looking for a job etc so I might not get to it for a bit.

regards,
dan carpenter

PS.  If you liked this script, try out my kmalloc script.  I don't think anyone besides me has successfully installed it yet, so if you have any questions I'd be glad to help.  :P  My phone number until tomorrow evening is (510) 835-7695.

--
__________________________________________________________
Sign-up for your own FREE Personalized E-mail at Mail.com
http://www.mail.com/?sr=signup

Save up to $160 by signing up for NetZero Platinum Internet service.
http://www.netzero.net/?refcd=N2P0602NEP8

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

lock_kernel check...

Post by Dave Hanse » Thu, 11 Jul 2002 02:50:07


 > Smatch.pm is from the smatch.sf.net scripts page.  Smatch is a
 > really unfinished code checker that I've been working on.  It is
 > based on reading the papers about the Stanford checker.

There was a time when I was thinking about the same thing.  It kept
scaring me the more I thought about it.

 > Unfortunately, after a night of sleep I realize that my script is
 > broken for 2 reasons. 1)  Smatch.pm is meant to track state changes
 > down different code paths.  But unfortunately it wasn't doing that
 > in this case; it was just going down the code without taking into
 > consideration any if_stmts  etc.  I'm extremely embarassed about
 > that.  Sorry.

Don't be sorry.  The script is smarter than the people who caused the
errors.  (once again, probably me)

 > 2)  What the Stanford checker does is print an error
 > if one return_stmt is called while the kernel is locked and one is
 > called while the kernel is unlocked.  This seems reasonable.

Could you clarify that a bit?

 > I will fix both mistakes later on this week.  Unfortunately I'm in
 > the process of moving and looking for a job etc so I might not get
 > to it for a bit.

--
Dave Hansen

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

lock_kernel check...

Post by dan carpente » Thu, 11 Jul 2002 03:30:11


----- Original Message -----

Date: Tue, 09 Jul 2002 10:41:01 -0700

Subject: Re: lock_kernel check...


>  > Smatch.pm is from the smatch.sf.net scripts page.  Smatch is a
>  > really unfinished code checker that I've been working on.  It is
>  > based on reading the papers about the Stanford checker.

> There was a time when I was thinking about the same thing.  It kept
> scaring me the more I thought about it.

True.  But someone is going to write a checker at some point.  It's only a couple days work if you know what you are doing.  There doesn't seem to be much advantage in waiting a year or two.

Quote:>  > Unfortunately, after a night of sleep I realize that my script is
>  > broken for 2 reasons. 1)  Smatch.pm is meant to track state changes
>  > down different code paths.  But unfortunately it wasn't doing that
>  > in this case; it was just going down the code without taking into
>  > consideration any if_stmts  etc.  I'm extremely embarassed about
>  > that.  Sorry.

> Don't be sorry.  The script is smarter than the people who caused the
> errors.  (once again, probably me)

>  > 2)  What the Stanford checker does is print an error
>  > if one return_stmt is called while the kernel is locked and one is
>  > called while the kernel is unlocked.  This seems reasonable.

> Could you clarify that a bit?

If someone made a mistake where they always returned under a kernel_lock() they would find the mistake themselves.  

Why would they return under kernel_lock() on error, for example, but not on success?  That would be confusing.  There would still be some false positives but the number of cases is really small.

regards,
dan carpenter

--
__________________________________________________________
Sign-up for your own FREE Personalized E-mail at Mail.com
http://www.mail.com/?sr=signup

Save up to $160 by signing up for NetZero Platinum Internet service.
http://www.netzero.net/?refcd=N2P0602NEP8

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

 
 
 

1. What does lock_kernel and unlock_kernel actualy makes ?

 Is it performs a full stop of kernel (except some life-parts of course) ?
What parts are still working ? What can I do (call, implement) and what I
can't between lock and unlock ?
By the way I want to prevent process memory access from anyone but my kernel
process, is a lock_kernel helps ?
Thanks

Eugene

2. Diamond cards and svgalib

3. lock_kernel in 2.2.0pre4

4. : Re: multiple cron jobs

5. kernel: down/up inside of lock_kernel/unlock_kernel?

6. y

7. do_exit() and lock_kernel() semantics

8. $argv in sh?

9. lock_kernel() usage and sync_*() functions

10. adds missed lock_kernel/unlock_kernel to reiserfs_dir_fsync().

11. driver/sound/soundcard.c lock_kernel()/unlock_kernel()

12. lock_kernel in 2.2.0pre4 w/o typo

13. Want RCS co NOT to check out if nothing new checked in