From patchwork Tue Dec 15 00:03:24 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Mason X-Patchwork-Id: 7850201 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 3DE19BEEE1 for ; Tue, 15 Dec 2015 00:03:43 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5238B20272 for ; Tue, 15 Dec 2015 00:03:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 571342026D for ; Tue, 15 Dec 2015 00:03:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932861AbbLOADi (ORCPT ); Mon, 14 Dec 2015 19:03:38 -0500 Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:54485 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932197AbbLOADh (ORCPT ); Mon, 14 Dec 2015 19:03:37 -0500 Received: from pps.filterd (m0044012.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.15.0.59/8.15.0.59) with SMTP id tBF00P0L004312; Mon, 14 Dec 2015 16:03:30 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=fb.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=facebook; bh=Tm4ihySNMtpugmfyNnsyvPcrX/8ycNSKpJSj3tz/Z0c=; b=mrK26Kp4eYBa368fcvb9wheQ84d/Jcee5BPy0z5aCZirvWRuidEqlNcI0vNsZ334q1Uz BGkIEVu5L2Dz34xtMrJmFwomqzZzkuOYdoLim4N4ig9lD8Vd1YLMg4WSS5+jyfFC6UiG 0IHq6iZlDsf1gNkOjv92/cBMbkItXM9tfwU= Received: from mail.thefacebook.com ([199.201.64.23]) by mx0a-00082601.pphosted.com with ESMTP id 1yt7k8r47y-1 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Mon, 14 Dec 2015 16:03:30 -0800 Received: from localhost (192.168.52.123) by mail.thefacebook.com (192.168.16.24) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 14 Dec 2015 16:03:27 -0800 Date: Mon, 14 Dec 2015 19:03:24 -0500 From: Chris Mason To: Dave Jones CC: , , , Filipe Manana Subject: Re: !PageLocked BUG_ON hit in clear_page_dirty_for_io Message-ID: <20151215000324.GD3570@ret.masoncoding.com> Mail-Followup-To: Chris Mason , Dave Jones , jbacik@fb.com, dsterba@suse.com, linux-btrfs@vger.kernel.org, Filipe Manana References: <20151209042528.GA2413@codemonkey.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20151209042528.GA2413@codemonkey.org.uk> User-Agent: Mutt/1.5.23.1 (2014-03-12) X-Originating-IP: [192.168.52.123] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2015-12-15_01:, , signatures=0 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Dec 08, 2015 at 11:25:28PM -0500, Dave Jones wrote: > Not sure if I've already reported this one, but I've been seeing this > a lot this last couple days. > > kernel BUG at mm/page-writeback.c:2654! > invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC KASAN We ended up discussing this in more detail on lkml, but I'll summarize here. There were two problems. First lock_page() might not actually lock the page in v4.4-rc4, it can bail out if a signal is pending. This got fixed just before v4.4-rc5, so if you were on rc4, upgrade asap. Second, prepare_pages had a bug for single page writes: From f0be89af049857bcc537a53fe2a2fae080e7a5bd Mon Sep 17 00:00:00 2001 From: Chris Mason Date: Mon, 14 Dec 2015 15:40:44 -0800 Subject: [PATCH] Btrfs: check prepare_uptodate_page() error code earlier prepare_pages() may end up calling prepare_uptodate_page() twice if our write only spans a single page. But if the first call returns an error, our page will be unlocked and its not safe to call it again. This bug goes all the way back to 2011, and it's not something commonly hit. While we're here, add a more explicit check for the page being truncated away. The bare lock_page() alone is protected only by good thoughts and i_mutex, which we're sure to regret eventually. Reported-by: Dave Jones Signed-off-by: Chris Mason Reviewed-by: Liu Bo Reviewed-by: Filipe Manana --- fs/btrfs/file.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 72e7346..0f09526 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -1291,7 +1291,8 @@ out: * on error we return an unlocked page and the error value * on success we return a locked page and 0 */ -static int prepare_uptodate_page(struct page *page, u64 pos, +static int prepare_uptodate_page(struct inode *inode, + struct page *page, u64 pos, bool force_uptodate) { int ret = 0; @@ -1306,6 +1307,10 @@ static int prepare_uptodate_page(struct page *page, u64 pos, unlock_page(page); return -EIO; } + if (page->mapping != inode->i_mapping) { + unlock_page(page); + return -EAGAIN; + } } return 0; } @@ -1324,6 +1329,7 @@ static noinline int prepare_pages(struct inode *inode, struct page **pages, int faili; for (i = 0; i < num_pages; i++) { +again: pages[i] = find_or_create_page(inode->i_mapping, index + i, mask | __GFP_WRITE); if (!pages[i]) { @@ -1333,13 +1339,17 @@ static noinline int prepare_pages(struct inode *inode, struct page **pages, } if (i == 0) - err = prepare_uptodate_page(pages[i], pos, + err = prepare_uptodate_page(inode, pages[i], pos, force_uptodate); - if (i == num_pages - 1) - err = prepare_uptodate_page(pages[i], + if (!err && i == num_pages - 1) + err = prepare_uptodate_page(inode, pages[i], pos + write_bytes, false); if (err) { page_cache_release(pages[i]); + if (err == -EAGAIN) { + err = 0; + goto again; + } faili = i - 1; goto fail; }