lkml.org 
[lkml]   [2000]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH #2] console lock grabbed too early in printk...
    Date
    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/

    \
     
     \ /
      Last update: 2005-03-22 13:57    [W:0.026 / U:90.104 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site