From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6BE93C433FE for ; Tue, 18 Jan 2022 03:36:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=lts/0t2Rxq/TJA58nZ37l+UYKmlJ/V6piJ8G7OieJMc=; b=ANA68NCkLY41Tb Rq3xwNFOUPTZWlOyA7CDnG7JkIWclrvwhaK04QRZz9LV1b7kxz+oQzAn3WxcyGQ0utpnhdUOs9gEm fCb6PEYwTU+e75+ht2TTVi7nXmHOIX892Jik6cZyss43HJL1VLG2ByA//cS8DnKclckxOgoU1VQEj 58IFUuI3cWsZxmdahmpcs/B+W72Tkgtd3OszYyp6ytzlXMQegoYngc6cQ0/DOS4mUO7+srZlmBKmh 6hKHOMZD76sUHM+G7xxWjYhfoL2beqXHFoNnCVdZTbagS8pnRkDYjX4nLy+XL1LLFM3HgdHuxn6vr wTq3rFVvYGOIqLM/DMfw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9fHZ-00HVG6-WC; Tue, 18 Jan 2022 03:35:42 +0000 Received: from ams.source.kernel.org ([2604:1380:4601:e00::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9eaH-00H8hz-Cf for linux-mtd@lists.infradead.org; Tue, 18 Jan 2022 02:50:59 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 0BDC5B811CE; Tue, 18 Jan 2022 02:50:56 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E81CFC36AEB; Tue, 18 Jan 2022 02:50:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1642474254; bh=NWZwtIg3tmBBM8P4IdY6qZLM6UCm8Zt9XReei62OV98=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WBoKw9tbtXYkaQ33WPdS9KNyxVQc8rTJuW5PwMHjyPhbBEb6h77WROZAlgH4idqqL 2h0mBxSlFQNNeGyaeUKkINBCtRkHQWR9+aYfdBGUCQzSnIS7Wl3Z+QOTn7jVJi2qim atVUmqzBKcly3VYviSGiA8kCNWptbCozR04j7Ho96pkRqd6PcUnlPAVjDfsehLMDoQ lyxmtt+ccyV2Wr8r9U4zmXYvMUNFUakQ/i4N340k2HE++1OA3FOnq1dvv0h6/qjdnl G6b4naNYQmA4HaxQ8uutBE/8XcwrpxpMEVKbOnawxR8FiwBL6JE2p5ELfR4KB1XKdb QTGKc7gZR/tSQ== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Kyeong Yoo , Richard Weinberger , Sasha Levin , dwmw2@infradead.org, joel@jms.id.au, linux-mtd@lists.infradead.org Subject: [PATCH AUTOSEL 4.14 42/56] jffs2: GC deadlock reading a page that is used in jffs2_write_begin() Date: Mon, 17 Jan 2022 21:48:54 -0500 Message-Id: <20220118024908.1953673-42-sashal@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220118024908.1953673-1-sashal@kernel.org> References: <20220118024908.1953673-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220117_185057_744469_A0A6BE8C X-CRM114-Status: GOOD ( 20.84 ) X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-mtd" Errors-To: linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org From: Kyeong Yoo [ Upstream commit aa39cc675799bc92da153af9a13d6f969c348e82 ] GC task can deadlock in read_cache_page() because it may attempt to release a page that is actually allocated by another task in jffs2_write_begin(). The reason is that in jffs2_write_begin() there is a small window a cache page is allocated for use but not set Uptodate yet. This ends up with a deadlock between two tasks: 1) A task (e.g. file copy) - jffs2_write_begin() locks a cache page - jffs2_write_end() tries to lock "alloc_sem" from jffs2_reserve_space() <-- STUCK 2) GC task (jffs2_gcd_mtd3) - jffs2_garbage_collect_pass() locks "alloc_sem" - try to lock the same cache page in read_cache_page() <-- STUCK So to avoid this deadlock, hold "alloc_sem" in jffs2_write_begin() while reading data in a cache page. Signed-off-by: Kyeong Yoo Signed-off-by: Richard Weinberger Signed-off-by: Sasha Levin --- fs/jffs2/file.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c index bd0428bebe9b7..221eb2bd205e4 100644 --- a/fs/jffs2/file.c +++ b/fs/jffs2/file.c @@ -135,20 +135,15 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, struct page *pg; struct inode *inode = mapping->host; struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode); + struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); pgoff_t index = pos >> PAGE_SHIFT; uint32_t pageofs = index << PAGE_SHIFT; int ret = 0; - pg = grab_cache_page_write_begin(mapping, index, flags); - if (!pg) - return -ENOMEM; - *pagep = pg; - jffs2_dbg(1, "%s()\n", __func__); if (pageofs > inode->i_size) { /* Make new hole frag from old EOF to new page */ - struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); struct jffs2_raw_inode ri; struct jffs2_full_dnode *fn; uint32_t alloc_len; @@ -159,7 +154,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len, ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE); if (ret) - goto out_page; + goto out_err; mutex_lock(&f->sem); memset(&ri, 0, sizeof(ri)); @@ -189,7 +184,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, ret = PTR_ERR(fn); jffs2_complete_reservation(c); mutex_unlock(&f->sem); - goto out_page; + goto out_err; } ret = jffs2_add_full_dnode_to_inode(c, f, fn); if (f->metadata) { @@ -204,13 +199,26 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, jffs2_free_full_dnode(fn); jffs2_complete_reservation(c); mutex_unlock(&f->sem); - goto out_page; + goto out_err; } jffs2_complete_reservation(c); inode->i_size = pageofs; mutex_unlock(&f->sem); } + /* + * While getting a page and reading data in, lock c->alloc_sem until + * the page is Uptodate. Otherwise GC task may attempt to read the same + * page in read_cache_page(), which causes a deadlock. + */ + mutex_lock(&c->alloc_sem); + pg = grab_cache_page_write_begin(mapping, index, flags); + if (!pg) { + ret = -ENOMEM; + goto release_sem; + } + *pagep = pg; + /* * Read in the page if it wasn't already present. Cannot optimize away * the whole page write case until jffs2_write_end can handle the @@ -220,15 +228,17 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, mutex_lock(&f->sem); ret = jffs2_do_readpage_nolock(inode, pg); mutex_unlock(&f->sem); - if (ret) - goto out_page; + if (ret) { + unlock_page(pg); + put_page(pg); + goto release_sem; + } } jffs2_dbg(1, "end write_begin(). pg->flags %lx\n", pg->flags); - return ret; -out_page: - unlock_page(pg); - put_page(pg); +release_sem: + mutex_unlock(&c->alloc_sem); +out_err: return ret; } -- 2.34.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 61B24C433FE for ; Tue, 18 Jan 2022 03:12:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1349448AbiARDJn (ORCPT ); Mon, 17 Jan 2022 22:09:43 -0500 Received: from ams.source.kernel.org ([145.40.68.75]:58986 "EHLO ams.source.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1349988AbiARCu5 (ORCPT ); Mon, 17 Jan 2022 21:50:57 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id EACE4B811D7; Tue, 18 Jan 2022 02:50:55 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E81CFC36AEB; Tue, 18 Jan 2022 02:50:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1642474254; bh=NWZwtIg3tmBBM8P4IdY6qZLM6UCm8Zt9XReei62OV98=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WBoKw9tbtXYkaQ33WPdS9KNyxVQc8rTJuW5PwMHjyPhbBEb6h77WROZAlgH4idqqL 2h0mBxSlFQNNeGyaeUKkINBCtRkHQWR9+aYfdBGUCQzSnIS7Wl3Z+QOTn7jVJi2qim atVUmqzBKcly3VYviSGiA8kCNWptbCozR04j7Ho96pkRqd6PcUnlPAVjDfsehLMDoQ lyxmtt+ccyV2Wr8r9U4zmXYvMUNFUakQ/i4N340k2HE++1OA3FOnq1dvv0h6/qjdnl G6b4naNYQmA4HaxQ8uutBE/8XcwrpxpMEVKbOnawxR8FiwBL6JE2p5ELfR4KB1XKdb QTGKc7gZR/tSQ== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Kyeong Yoo , Richard Weinberger , Sasha Levin , dwmw2@infradead.org, joel@jms.id.au, linux-mtd@lists.infradead.org Subject: [PATCH AUTOSEL 4.14 42/56] jffs2: GC deadlock reading a page that is used in jffs2_write_begin() Date: Mon, 17 Jan 2022 21:48:54 -0500 Message-Id: <20220118024908.1953673-42-sashal@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220118024908.1953673-1-sashal@kernel.org> References: <20220118024908.1953673-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Kyeong Yoo [ Upstream commit aa39cc675799bc92da153af9a13d6f969c348e82 ] GC task can deadlock in read_cache_page() because it may attempt to release a page that is actually allocated by another task in jffs2_write_begin(). The reason is that in jffs2_write_begin() there is a small window a cache page is allocated for use but not set Uptodate yet. This ends up with a deadlock between two tasks: 1) A task (e.g. file copy) - jffs2_write_begin() locks a cache page - jffs2_write_end() tries to lock "alloc_sem" from jffs2_reserve_space() <-- STUCK 2) GC task (jffs2_gcd_mtd3) - jffs2_garbage_collect_pass() locks "alloc_sem" - try to lock the same cache page in read_cache_page() <-- STUCK So to avoid this deadlock, hold "alloc_sem" in jffs2_write_begin() while reading data in a cache page. Signed-off-by: Kyeong Yoo Signed-off-by: Richard Weinberger Signed-off-by: Sasha Levin --- fs/jffs2/file.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c index bd0428bebe9b7..221eb2bd205e4 100644 --- a/fs/jffs2/file.c +++ b/fs/jffs2/file.c @@ -135,20 +135,15 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, struct page *pg; struct inode *inode = mapping->host; struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode); + struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); pgoff_t index = pos >> PAGE_SHIFT; uint32_t pageofs = index << PAGE_SHIFT; int ret = 0; - pg = grab_cache_page_write_begin(mapping, index, flags); - if (!pg) - return -ENOMEM; - *pagep = pg; - jffs2_dbg(1, "%s()\n", __func__); if (pageofs > inode->i_size) { /* Make new hole frag from old EOF to new page */ - struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); struct jffs2_raw_inode ri; struct jffs2_full_dnode *fn; uint32_t alloc_len; @@ -159,7 +154,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len, ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE); if (ret) - goto out_page; + goto out_err; mutex_lock(&f->sem); memset(&ri, 0, sizeof(ri)); @@ -189,7 +184,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, ret = PTR_ERR(fn); jffs2_complete_reservation(c); mutex_unlock(&f->sem); - goto out_page; + goto out_err; } ret = jffs2_add_full_dnode_to_inode(c, f, fn); if (f->metadata) { @@ -204,13 +199,26 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, jffs2_free_full_dnode(fn); jffs2_complete_reservation(c); mutex_unlock(&f->sem); - goto out_page; + goto out_err; } jffs2_complete_reservation(c); inode->i_size = pageofs; mutex_unlock(&f->sem); } + /* + * While getting a page and reading data in, lock c->alloc_sem until + * the page is Uptodate. Otherwise GC task may attempt to read the same + * page in read_cache_page(), which causes a deadlock. + */ + mutex_lock(&c->alloc_sem); + pg = grab_cache_page_write_begin(mapping, index, flags); + if (!pg) { + ret = -ENOMEM; + goto release_sem; + } + *pagep = pg; + /* * Read in the page if it wasn't already present. Cannot optimize away * the whole page write case until jffs2_write_end can handle the @@ -220,15 +228,17 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, mutex_lock(&f->sem); ret = jffs2_do_readpage_nolock(inode, pg); mutex_unlock(&f->sem); - if (ret) - goto out_page; + if (ret) { + unlock_page(pg); + put_page(pg); + goto release_sem; + } } jffs2_dbg(1, "end write_begin(). pg->flags %lx\n", pg->flags); - return ret; -out_page: - unlock_page(pg); - put_page(pg); +release_sem: + mutex_unlock(&c->alloc_sem); +out_err: return ret; } -- 2.34.1