bkl cleanup in do_sysctl

I'd like to hear people's thoughts on replacing the bkl in do_sysctl with a localized spin lock that protects the sysctl structures. Instead of grabbing the bkl, anyone that needs to mess with those values could grab the localized lock (1 to protect all structures). Such a localized lock would allow us to get rid of bkl usage in at least one other place as well (do_coredump). In order to do this though, we would have to make sure all code that grabs the bkl instead of the localized lock while using sysctl values switches to the new lock. Might be a big job, but perhaps it would be a good one to start after 2.6.8 is out the door.

Remember that the BKL isn't a plain-old spinlock. You're allowed to sleep while holding it and it can be recursively held, which isn't true for other spinlocks.

So, if you want to replace it with a spinlock, you'll need to do audits looking for sysctl users that might_sleep() or get called recursively somehow. The might_sleep() debugging checks should help immensely for the first part, but all you'll get are deadlocks at runtime for any recursive holders. But, those cases are increasingly rare, so you might luck out and not have any.

Or, you could just make it a semaphore and forget about the no sleeping requirement.

Lee> For example reiserfs uses the BKL for all write locking (!), Lee> but it probably would not be too hard to fix, because you can Lee> just look at another filesystem that has proper locking.

Fixing up a filesystem's write locking doesn't sound like a very good newbie project to me.

Quoted 11 lines · Dave Hansen

Remember that the BKL isn't a plain-old spinlock. You're allowed to sleep while holding it and it can be recursively held, which isn't true for other spinlocks.

So, if you want to replace it with a spinlock, you'll need to do audits looking for sysctl users that might_sleep() or get called recursively somehow. The might_sleep() debugging checks should help immensely for the first part, but all you'll get are deadlocks at runtime for any recursive holders. But, those cases are increasingly rare, so you might luck out and not have any.

Quoted 5 lines

Or, you could just make it a semaphore and forget about the no sleeping requirement.

Someone once suggested that newbies who show up on LKML wanting to learn kernel hacking should be assigned to find one use of the BKL and replace it with proper locking. Something similar worked very well with my previous employer, before giving someone root, new hires would first be assigned some task like writing a script to take the user account database and generate a report of old accounts on a bunch of machines, or rewrite the RADIUS accounting scripts, where the point was really to get them familiar with the system.

This way, even if they come back with a totally botched fix, someone will probably just post a correct one. We could get rid of the BKL very soon, I count only 247 files with lock_kernel in them.

For example reiserfs uses the BKL for all write locking (!), but it probably would not be too hard to fix, because you can just look at another filesystem that has proper locking.

Maybe this should be added to the FAQ:

Q: I want to hack the kernel, and I *think* I know what I am doing. Where do I start?

Lee

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 5 lines · Lee Revell

For example reiserfs uses the BKL for all write locking (!), but it probably would not be too hard to fix, because you can just look at another filesystem that has proper locking.

Wrong. ;-) Balancing makes it way hard. Use reiser4. That has very sophisticated locking that pushes the research envelope, if you want to read code to learn about locking.....