From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933700AbcATMbO (ORCPT ); Wed, 20 Jan 2016 07:31:14 -0500 Received: from mx2.suse.de ([195.135.220.15]:37966 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933502AbcATMbK (ORCPT ); Wed, 20 Jan 2016 07:31:10 -0500 Date: Wed, 20 Jan 2016 13:31:07 +0100 From: Petr Mladek To: Sergey Senozhatsky Cc: Sergey Senozhatsky , Andrew Morton , Tejun Heo , Jan Kara , Kyle McMartin , Dave Jones , Calvin Owens , linux-kernel@vger.kernel.org Subject: Re: [RFC][PATCH -next 2/2] printk: set may_schedule for some of console_trylock callers Message-ID: <20160120123107.GB3305@pathway.suse.cz> References: <1452747443-9927-1-git-send-email-sergey.senozhatsky@gmail.com> <1452747443-9927-3-git-send-email-sergey.senozhatsky@gmail.com> <20160117141137.GA631@swordfish> <20160117142332.GA543@swordfish> <20160118161744.GZ3178@pathway.suse.cz> <20160119011510.GB4963@swordfish> <20160119151801.GB2148@dhcp128.suse.cz> <20160120035056.GA506@swordfish> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160120035056.GA506@swordfish> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed 2016-01-20 12:50:56, Sergey Senozhatsky wrote: > so in_atomic() covers both preemption disabled and in_interrupt(). I see. > well, if we run a !PREEMPT_COUNT kernel, then > #define preempt_disable() barrier() > #define preempt_enable() barrier() > #define preemptible() 0 > > so I don't think that preempt_disable()/preempt_enable() were for > non-preempt kernels there. > > otoh, preempt_count still counts irqs, because in_interrupt() (and friends) > macro is supposed to work regardless the preempt config selection, so we > just lose preempt_disable bit from preempt_count... hm... I think I'll > just introduce preemptible() check there. > > for preempt kernels, preemptible() does > #define preemptible() (preempt_count() == 0 && !irqs_disabled()) > > and for !preempt kernels > #define preemptible() 0 > > iow, no cond_resched() in console_unlock() called from vprintk_emit() > on non-preempt kernels. > > so the console_may_schedule now turns into (composed in mail-app, not > actually tested): > > > console_may_schedule = !oops_in_progress && preemptible() && > !rcu_preempt_depth(); I though about it from some other side and I wonder if we need all the console_may_schedule stuff at all. printk_emit() has the following code: /* This stops the holder of console_sem just where we want him */ local_irq_save(flags); [...] lockdep_off(); raw_spin_lock(&logbuf_lock); logbuf_cpu = this_cpu; [...] logbuf_cpu = UINT_MAX; raw_spin_unlock(&logbuf_lock); lockdep_on(); local_irq_restore(flags); /* If called from the scheduler, we can not call up(). */ if (!in_sched) { lockdep_off(); /* * Disable preemption to avoid being preempted while holding * console_sem which would prevent anyone from printing to * console */ preempt_disable(); /* * Try to acquire and then immediately release the console * semaphore. The release will print out buffers and wake up * /dev/kmsg and syslog() users. */ if (console_trylock_for_printk()) console_unlock(); preempt_enable(); First, the message "This stops the holder of console_sem just where we want him" is suspitious. It is there sice the initial git commit on 2005-04-16. I do not understand how this could block the console holder on another CPU. I think that it rather allows to make the recursion detection without the lockbuf lock. But this is not that important. More interesting is the counter part: raw_spin_unlock(&logbuf_lock); lockdep_on(); local_irq_restore(flags); raw_spin_unlock() calls preempt_enable() that calls preempt_schedule(). It does nothing here because IRQs are still disabled. But if we do not need the lockdep_on() hack, we could use raw_spin_unlock_irqrestore(&lockbuf_lock, flags). It means that we do not call cond_resched here "only by chance". In each case, preemtible kernel seems to be free to reschedule after we enable the inrerrupts. By other words, preemptible kernel seems to be able to reschedule in printk() already these days. Why it might work? I think that it might work because cond_resched() or rather preempt_schedule() are clever enough. They both check preempt_count and do nothing if preemtion is disabled. As a result, I think that we do not need the extra checks for the save context in printk(). IMHO, it is safe to remove all the console_may_schedule stuff and also remove the extra preempt_disable/preempt_enable() in vprintk_emit(). Or did I miss anything? Best Regards, Petr