RE: [RFC][PATCH v4 1/2] printk: Make printk() completely async

byungchul.park
2016-03-18T08:23:52Z

> [..]
> > diff --git a/kernel/locking/spinlock_debug.c
> b/kernel/locking/spinlock_debug.c
> > index fd24588..30559c6 100644
> > --- a/kernel/locking/spinlock_debug.c
> > +++ b/kernel/locking/spinlock_debug.c
> > @@ -138,14 +138,25 @@ static void __spin_lock_debug(raw_spinlock_t
*lock)
> > {
> > u64 i;
> > u64 loops = loops_per_jiffy * HZ;
> > + static raw_spinlock_t *suspected_lock = NULL;
>
> this has no chances to survive on SMP systems that have spin_lockup-ed on
> at
> least two different spin locks.

I don't think so. It can have chances to survive on SMP with 2 different
locks. Remind the condition this problem can happen.

1. when it is "lockup suspected".
2. when it is within a printk().

2 different locks mean 2 different timing. Therefore it's important to
prevent a recursion at a place trying to obtain the lock. After a lock
is solved, the other lock can be solved step by step.

> I'd really prefer not to mix-in spin_dump/printk recursion problems into
> this

I didn't mix-in it into this. I only focused your patch. Your approach
introduces unnecessary losing a message which we don't want. But you
worried about "infinite recursion" when implementing it without losing
the message. That's why I am saying it doesn't need to be worried and
there's another way, that is, my suggestion.

> patch set. it makes sense not to make printk recursion detection worse due
> to

It makes more sense not to introduce newly added spin_locks. You are
currently utilize the last resort for detecting recursion. It's too bad.
Don't you think so?

> newly added spin_locks to vprintk_emit(), but that's it. this patch set
> set is
> fixing other things in the first place.

As I said, this patch's trying to fix a problem in your patch, which
is introduced while discussing yours.

Thanks,
Byungchul