lkml.org 
[lkml]   [2018]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup
    On (01/15/18 15:45), Petr Mladek wrote:
    [..]
    > > With the preempt_disable() there really isn't a delay. I agree, we
    > > shouldn't let printk preempt (unless we have CONFIG_PREEMPT_RT enabled,
    > > but that's another story).
    > >
    > > >
    > > > so very schematically, for hand-off it's something like
    > > >
    > > > if (... console_trylock_spinning()) // grabbed the ownership
    > > >
    > > > << ... preempted ... >>
    > > >
    > > > console_unlock();
    > >
    > > Which I think we should stop, with the preempt_disable().
    >
    > Adding the preempt_disable() basically means to revert the already
    > mentioned commit 6b97a20d3a7909daa06625 ("printk: set may_schedule
    > for some of console_trylock() callers").
    >
    > I originally wanted to solve this separately to make it easier. But
    > the change looks fine to me. Therefore we reached a mutual agreement.
    > Sergey, do you want to send a patch or should I just put it at
    > the end of this patchset?

    you can add the patch.

    [..]
    > > I think adding the preempt_disable() would fix printk() but let non
    > > printk console_unlock() still preempt.
    >
    > I would personally remove cond_resched() from console_unlock()
    > completely.

    hmm, not so sure. I think it's there for !PREEMPT systems which have
    to print a lot of messages. the case I'm speaking about in particular
    is when we register a CON_PRINTBUFFER console and need to console_unlock()
    (flush) all of the messages we currently have in the logbuf. we better
    have that cond_resched() there, I think.

    > Sleeping in console_unlock() increases the chance that more messages
    > would need to be handled. And more importantly it reduces the chance
    > of a successful handover.
    >
    > As a result, the caller might spend there very long time, it might
    > be getting increasingly far behind. There is higher risk of lost
    > messages. Also the eventual taker might have too much to proceed
    > in preemption disabled context.

    yes.

    > Removing cond_resched() is in sync with printk() priorities.

    hmm, not sure. we have sleeping console_lock()->console_unlock() path
    for PREEMPT kernels, that cond_resched() makes the !PREEMPT kernels to
    have the same sleeping console_lock()->console_unlock().

    printk()->console_unlock() seems to be a pretty independent thing,
    unfortunately (!), yet sleeping console_lock()->console_unlock()
    messes up with it a lot.

    > The highest one is to get the messages out.
    >
    > Finally, removing cond_resched() should make the behavior more
    > predictable (never preempted)

    but we are always preempted in PREEMPT kernels when the current
    console_sem owner acquired the lock via console_lock(), not via
    console_trylock(). cond_resched() does the same, but for !PREEMPT.

    -ss

    \
     
     \ /
      Last update: 2018-01-16 03:24    [W:4.052 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site