From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from a.ns.miles-group.at ([95.130.255.143] helo=radon.swed.at) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZKCMM-0000Ju-1N for linux-mtd@lists.infradead.org; Tue, 28 Jul 2015 21:24:27 +0000 Subject: Re: [PATCH] ubifs: Kill unneeded locking in ubifs_init_security To: Boris Brezillon References: <1436348796-19725-1-git-send-email-richard@nod.at> <20150728132103.7f6f52dd@bbrezillon> Cc: linux-mtd@lists.infradead.org, dedekind1@gmail.com, adrian.hunter@intel.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org From: Richard Weinberger Message-ID: <55B7F2EB.4070902@nod.at> Date: Tue, 28 Jul 2015 23:23:55 +0200 MIME-Version: 1.0 In-Reply-To: <20150728132103.7f6f52dd@bbrezillon> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Am 28.07.2015 um 13:21 schrieb Boris Brezillon: > Hi Richard, > > On Wed, 8 Jul 2015 11:46:36 +0200 > Richard Weinberger wrote: > >> Fixes the following lockdep splat: >> [ 1.244527] ============================================= >> [ 1.245193] [ INFO: possible recursive locking detected ] >> [ 1.245193] 4.2.0-rc1+ #37 Not tainted >> [ 1.245193] --------------------------------------------- >> [ 1.245193] cp/742 is trying to acquire lock: >> [ 1.245193] (&sb->s_type->i_mutex_key#9){+.+.+.}, at: [] ubifs_init_security+0x29/0xb0 >> [ 1.245193] >> [ 1.245193] but task is already holding lock: >> [ 1.245193] (&sb->s_type->i_mutex_key#9){+.+.+.}, at: [] path_openat+0x3af/0x1280 >> [ 1.245193] >> [ 1.245193] other info that might help us debug this: >> [ 1.245193] Possible unsafe locking scenario: >> [ 1.245193] >> [ 1.245193] CPU0 >> [ 1.245193] ---- >> [ 1.245193] lock(&sb->s_type->i_mutex_key#9); >> [ 1.245193] lock(&sb->s_type->i_mutex_key#9); >> [ 1.245193] >> [ 1.245193] *** DEADLOCK *** >> [ 1.245193] >> [ 1.245193] May be due to missing lock nesting notation >> [ 1.245193] >> [ 1.245193] 2 locks held by cp/742: >> [ 1.245193] #0: (sb_writers#5){.+.+.+}, at: [] mnt_want_write+0x1f/0x50 >> [ 1.245193] #1: (&sb->s_type->i_mutex_key#9){+.+.+.}, at: [] path_openat+0x3af/0x1280 >> [ 1.245193] >> [ 1.245193] stack backtrace: >> [ 1.245193] CPU: 2 PID: 742 Comm: cp Not tainted 4.2.0-rc1+ #37 >> [ 1.245193] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.7.5-0-ge51488c-20140816_022509-build35 04/01/2014 >> [ 1.245193] ffffffff8252d530 ffff88007b023a38 ffffffff814f6f49 ffffffff810b56c5 >> [ 1.245193] ffff88007c30cc80 ffff88007b023af8 ffffffff810a150d ffff88007b023a68 >> [ 1.245193] 000000008101302a ffff880000000000 00000008f447e23f ffffffff8252d500 >> [ 1.245193] Call Trace: >> [ 1.245193] [] dump_stack+0x4c/0x65 >> [ 1.245193] [] ? console_unlock+0x1c5/0x510 >> [ 1.245193] [] __lock_acquire+0x1a6d/0x1ea0 >> [ 1.245193] [] ? __lock_is_held+0x58/0x80 >> [ 1.245193] [] lock_acquire+0xd3/0x270 >> [ 1.245193] [] ? ubifs_init_security+0x29/0xb0 >> [ 1.245193] [] mutex_lock_nested+0x6b/0x3a0 >> [ 1.245193] [] ? ubifs_init_security+0x29/0xb0 >> [ 1.245193] [] ? ubifs_init_security+0x29/0xb0 >> [ 1.245193] [] ubifs_init_security+0x29/0xb0 >> [ 1.245193] [] ubifs_create+0xa6/0x1f0 >> [ 1.245193] [] ? path_openat+0x3af/0x1280 >> [ 1.245193] [] vfs_create+0x95/0xc0 >> [ 1.245193] [] path_openat+0x7cc/0x1280 >> [ 1.245193] [] ? __lock_acquire+0x543/0x1ea0 >> [ 1.245193] [] ? sched_clock_cpu+0x90/0xc0 >> [ 1.245193] [] ? calc_global_load_tick+0x60/0x90 >> [ 1.245193] [] ? sched_clock_cpu+0x90/0xc0 >> [ 1.245193] [] ? __alloc_fd+0xaf/0x180 >> [ 1.245193] [] do_filp_open+0x75/0xd0 >> [ 1.245193] [] ? _raw_spin_unlock+0x26/0x40 >> [ 1.245193] [] ? __alloc_fd+0xaf/0x180 >> [ 1.245193] [] do_sys_open+0x129/0x200 >> [ 1.245193] [] SyS_open+0x19/0x20 >> [ 1.245193] [] entry_SYSCALL_64_fastpath+0x12/0x6f >> >> While the lockdep splat is a false positive, becuase path_openat holds i_mutex >> of the parent directory and ubifs_init_security() tries to acquire i_mutex >> of a new inode, it reveals that taking i_mutex in ubifs_init_security() is >> in vain because it is only being called in the inode allocation path >> and therefore nobody else can see the inode yet. >> >> Reported-by: Boris Brezillon >> Signed-off-by: Richard Weinberger > > It might be too late, but if it's not you can add my > > Tested-by: Boris Brezillon Applied and pushed! Thanks, //richard