Messages in this thread | | | From | Russell King <> | Subject | Re: [PATCH #2] console lock grabbed too early in printk... | Date | Tue, 4 Jul 2000 20:58:43 +0100 (BST) |
| |
Chris Lattner writes: > What makes an address a user address?
Depends on the architecture.
> When going through someone elses code, sometimes you can tell when it > gets copied in, sometimes not...
If the kernel is using it without it copied in, then that is a bug, and all praises to you who have found it.
> and should i have to do something like this everytime I want to printk? > > { char Buffer[1000]; > copyinto buffer > printk(buffer); > }
Err, no. You should just be aware that calling any kernel routine which is not expected to handle user-mode addresses should not be passed user-mode addresses.
> I think this is completely ridiculous. I'm asking to put one low > contention lock and unlock into the printk path. Printk is not called > very often except when "things are going wrong" or you are debugging... in > either case, don't you want it to be as forgiving as possible??
Lets examine the facts shall we?
1. Your original problem was caused by calling printk with a user mode address.
Currently, we have ONE error here - calling printk from a user-mode address.
2. The user-mode address was not pointing at a present page, so printk faulted with its spinlock held.
At this point, we have TWO errors here - calling printk from a user-mode address AND faulting with a spinlock held. Lets carry on with your case, and you call printk from the page fault handling code.
3a. The page fault code calls printk
4a. We deadlock waiting on the spinlock.
Ok, lets say for argument that you didn't have that printk in:
3b. The page fault handler needs to access the disk to supply the page, so the handler sleeps.
Oh dear, another problem - sleeping with a spinlock held. Another favorite for deadlocks. In fact, if the code before (1) locked a spinlock, you could deadlock when that lock comes into contention.
You will also get the SAME behaviour and outcome as (3b) with your patch. You're just making it harder to find out what the problem is when it does happen. Who would think that the deadlock on foo_bar_lock in the depths of the filesystem code was caused by someone calling printk with a user-mode address?
> I'm starting to get annoyed at people who keep arguing that people don't > make mistakes, and when they do, they should get punished for it. That is > completely inappropriate crap.
Is it appropriate to add code to say "well, if this spinlock is held, we can get around it this way and still execute the critical code?". Suddenly all spinlocks have this condition in, just in case some kernel hacker makes a mistake. Just think about the performance impact of all those extra unnecessary tests which are just there incase of a mistake, and we don't get a nice message to say "oi! you're being stupid calling me like that!"
> Maybe you are some macho experienced kernel developer, but I would be > willing to be that even Linux and Alan make a mistake every once in a > while... hell I bet they have to debug stuff they write too [and they > might even use printk on occasion to do so!].
FYI, printk is my debugging tool too. Therefore your argument does not hold water I'm afraid.
> What is the big conceptual problem here???
My point is that the rules are very very simple. The functions where you can't use printk are very small in number (as I hinted in my previous email), plus, even if you do put a printk in one of those, you're well on the way to doom as soon as you save that source file and type make.
If you want a list, here they are:
printk() vsprintf() spin_lock_irqsave() spin_unlock_irqrestore() wake_up_interruptible() vt_console_print() test_and_set_bit() pm_access() vc_cons_allocated() hide_cursor() scr_writew() set_cursor() poke_blanked_console() clear_bit()
And as I tried to point out before, your patch will not help if someone puts a printk in any of those functions.
I hoped you'd understand the concept of infinite recusion.
Please try to understand the issues before you start getting annoyed that your patch isn't being given a welcoming reception. _____ |_____| ------------------------------------------------- ---+---+- | | Russell King rmk@arm.linux.org.uk --- --- | | | | http://www.arm.linux.org.uk/~rmk/aboutme.html / / | | +-+-+ --- -+- / | THE developer of ARM Linux |+| /|\ / | | | --- | +-+-+ ------------------------------------------------- /\\\ |
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |