From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935309AbcLVFbQ (ORCPT ); Thu, 22 Dec 2016 00:31:16 -0500 Received: from mail-pg0-f67.google.com ([74.125.83.67]:32938 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933708AbcLVFbO (ORCPT ); Thu, 22 Dec 2016 00:31:14 -0500 Date: Thu, 22 Dec 2016 14:31:19 +0900 From: Sergey Senozhatsky To: Linus Torvalds , Petr Mladek Cc: Andrew Morton , Jan Kara , Tejun Heo , Calvin Owens , Steven Rostedt , Ingo Molnar , Peter Zijlstra , Andy Lutomirski , Peter Hurley , linux-kernel@vger.kernel.org, Sergey Senozhatsky , Sergey Senozhatsky Subject: Re: [PATCHv6 6/7] printk: use printk_safe buffers in printk Message-ID: <20161222053119.GE644@jagdpanzerIV.localdomain> References: <20161221143605.2272-1-sergey.senozhatsky@gmail.com> <20161221143605.2272-7-sergey.senozhatsky@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161221143605.2272-7-sergey.senozhatsky@gmail.com> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On (12/21/16 23:36), Sergey Senozhatsky wrote: > Use printk_safe per-CPU buffers in printk recursion-prone blocks: > -- around logbuf_lock protected sections in vprintk_emit() and > console_unlock() > -- around down_trylock_console_sem() and up_console_sem() > > Note that this solution addresses deadlocks caused by printk() > recursive calls only. That is vprintk_emit() and console_unlock(). several questions. so my plan was to introduce printk-safe and to switch vprintk_emit() and console_sem related functions (like console_unlock(), etc.) to printk-safe first. and switch the remaining logbuf_lock users, like devkmsg_open()/syslog_print()/etc, in a followup, pretty much mechanical "find logbuf_lock - add printk_safe", patch. but that followup patch is bigger than I expected (still mechanical tho); so I want to re-group. there are 9 raw_spin_lock_irq(&logbuf_lock) 7 raw_spin_lock_irqsave(&logbuf_lock, flags) and 12 raw_spin_lock_irq(&logbuf_lock) 8 raw_spin_unlock_irqrestore(&logbuf_lock, flags) wrapping each one of them in printk_safe_enter()/printk_safe_enter_irq() and printk_safe_exit()/printk_safe_exit_irq() is a bit boring. so I have several options: one of them is to add printk_safe_{enter,exit}_irq() and, along with it, a bunch of help macros (to printk.c): (questions below) /* * Helper macros to lock/unlock logbuf_lock in deadlock safe * manner (logbuf_lock may spin_dump() in lock/unlock). */ #define lock_logbuf(flags) \ do { \ printk_safe_enter(flags); \ raw_spin_lock(&logbuf_lock); \ } while (0) #define unlock_logbuf(flags) \ do { \ raw_spin_unlock(&logbuf_lock); \ printk_safe_exit(flags); \ } while (0) #define lock_logbuf_irq() \ do { \ printk_safe_enter_irq(); \ raw_spin_lock(&logbuf_lock); \ } while (0) #define unlock_logbuf_irq() \ do { \ raw_spin_unlock(&logbuf_lock); \ printk_safe_exit_irq(); \ } while (0) so this printk_safe_enter_irq(); raw_spin_lock(&logbuf_lock); ... raw_spin_unlock(&logbuf_lock); printk_safe_exit(flags); or this printk_safe_enter_irq(); raw_spin_lock(&logbuf_lock); ... raw_spin_unlock(&logbuf_lock); printk_safe_exit_irq(); becomes this lock_logbuf(flags); ... unlock_logbuf(flags); and this lock_logbuf_irq(); ... unlock_logbuf_irq(); questions: -- the approach another solution? switch those raw_spin_{lock,unlock}_irq to irqsave/irqrestore (?) and use the existing printk_safe_enter()/printk_safe_exit(), so *_irq() versions of lock_logbuf/printk_safe macros will not be needed? -- the naming are lock_logbuf()/unlock_logbuf() and lock_logbuf_irq()/unlock_logbuf_irq() good enough? (if good at all) -ss