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 X-Spam-Level: X-Spam-Status: No, score=-14.8 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1, USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2BDD9C56202 for ; Tue, 24 Nov 2020 21:47:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AEDCE20872 for ; Tue, 24 Nov 2020 21:47:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="VzQAooJ4" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387615AbgKXVrB (ORCPT ); Tue, 24 Nov 2020 16:47:01 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54846 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731557AbgKXVrA (ORCPT ); Tue, 24 Nov 2020 16:47:00 -0500 Received: from mail-oi1-x244.google.com (mail-oi1-x244.google.com [IPv6:2607:f8b0:4864:20::244]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 16498C0613D6 for ; Tue, 24 Nov 2020 13:47:00 -0800 (PST) Received: by mail-oi1-x244.google.com with SMTP id s18so391512oih.1 for ; Tue, 24 Nov 2020 13:47:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:in-reply-to:message-id:references :user-agent:mime-version; bh=04YZn/oOi9cwlQjeUz6wAhKNr23VM3kG20feokh0qeg=; b=VzQAooJ4hoEvvjBCc+1Qt2u70siZAJm6h9WtKGpP0ywmkQ1vspYXHydmfQoTf4ym3I oqKJzcR5krrs+o2BlUzYp3kcBIpfa6E6ew96qdIC2DTZU5N5pj5QjitRF0O5vAP3DLm/ Rw71YTkXEZBmOi1/VFYN3Po6znE+JQDeN5AQ5GT8PpDyb4DUQYVH9RDLVPxXCtlzpqGL NLPgS8k2q9XJZCcEqp0jixScddihjCkoUiurydf3GmXYEZPzwqkjXKYbEuCjos3F0Dgj X3M8gEHfAlkVbtkko2WkcUF0RRIGEvajB3ukYdimzdhZa9OdqeypZLEs6CnYwqCUGZF7 gNSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version; bh=04YZn/oOi9cwlQjeUz6wAhKNr23VM3kG20feokh0qeg=; b=d84hUq0JcenZYbDP7USN2aWY21j3/zNh+P88/D785iOqa4/eJDSrrbXPKtbJjljJJ7 rrhRavD0CSMILIgtShgux97/4BaWu1dsJ6RkbvtfBFbJ5SrwxM9l47OC1Jqx7yrQiqnU o51/8OGaGWM5kt/dzYEHSL2y3+CioR7ZdVJp37j9jRzVomvQYehrR3iihfWwc+B3IZaZ 6jQMfd5pDJLGmUcSkLTy4BXk7Xf9/2nNbfcwahvteBEOUCqmwoJKYw3qciaHwYbZJQOq GkourcXDQPGuc1Rhcds9zZeduGVA73AKBqOoiq+BdKpiyp/ey0Hw+TgOj79vx/j/X2r+ AcXw== X-Gm-Message-State: AOAM532kxoES3WdY7CYZC9zOWkPTM4Z6K/mfV7zXNN3jXr2mkQ+RiK6q y55Kjv6jDwSCtNl6vq+ABe9Wqw== X-Google-Smtp-Source: ABdhPJxOVg+Bowm05sagrPqp3xv+YibTG16pRL9z0qkeXz1sv96JtkxPJaBWAFfkjlhlZQfRI/zZYw== X-Received: by 2002:aca:f5c8:: with SMTP id t191mr213734oih.40.1606254419120; Tue, 24 Nov 2020 13:46:59 -0800 (PST) Received: from eggly.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id a4sm139138otj.29.2020.11.24.13.46.56 (version=TLS1 cipher=ECDHE-ECDSA-AES128-SHA bits=128/128); Tue, 24 Nov 2020 13:46:58 -0800 (PST) Date: Tue, 24 Nov 2020 13:46:44 -0800 (PST) From: Hugh Dickins X-X-Sender: hugh@eggly.anvils To: Linus Torvalds cc: Matthew Wilcox , Hugh Dickins , Jan Kara , syzbot , Andreas Dilger , Ext4 Developers List , Linux Kernel Mailing List , syzkaller-bugs , Theodore Ts'o , Linux-MM , Oleg Nesterov , Andrew Morton , "Kirill A. Shutemov" , Nicholas Piggin , Alex Shi , Qian Cai , Christoph Hellwig , "Darrick J. Wong" , William Kucharski , Jens Axboe , linux-fsdevel , linux-xfs Subject: Re: kernel BUG at fs/ext4/inode.c:LINE! In-Reply-To: Message-ID: References: <000000000000d3a33205add2f7b2@google.com> <20200828100755.GG7072@quack2.suse.cz> <20200831100340.GA26519@quack2.suse.cz> <20201124121912.GZ4327@casper.infradead.org> <20201124183351.GD4327@casper.infradead.org> <20201124201552.GE4327@casper.infradead.org> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 24 Nov 2020, Linus Torvalds wrote: > On Tue, Nov 24, 2020 at 12:16 PM Matthew Wilcox wrote: > > > > So my s/if/while/ suggestion is wrong and we need to do something to > > prevent spurious wakeups. Unless we bury the spurious wakeup logic > > inside wait_on_page_writeback() ... > > We can certainly make the "if()" in that loop be a "while()'. > > That's basically what the old code did - simply by virtue of the > wakeup not happening if the writeback bit was set in > wake_page_function(): > > if (test_bit(key->bit_nr, &key->page->flags)) > return -1; > > of course, the race was still there - because the writeback bit might > be clear at that point, but another CPU would reallocate and dirty it, > and then autoremove_wake_function() would happen anyway. > > But back in the bad old days, the wait_on_page_bit_common() code would > then double-check in a loop, so it would catch that case, re-insert > itself on the wait queue, and try again. Except for the DROP case, > which isn't used by writeback. > > Anyway, making that "if()" be a "while()" in wait_on_page_writeback() > would basically re-introduce that old behavior. I don't really care, > because it was the lock bit that really mattered, the writeback bit is > not really all that interesting (except from a "let's fix this bug" > angle) > > I'm not 100% sure I like the fragility of this writeback thing. > > Anyway, I'm certainly happy with either model, whether it be an added > while() in wait_on_page_writeback(), or it be the page reference count > in end_page_writeback(). > > Strong opinions? Responding to "Strong opinions?" before having digested Matthew's DMA sequence (no, not his DNA sequence). I think it comes down to whether my paranoia (about accessing an unreferenced struct page) is realistic or not: since I do hold that paranoia, I do prefer (whatever variant of) my patch. I'm not a memory hotremove guy. I did search mm/memory_hotplug.c for references to rcu or stop_machine(), but found none. I can imagine that the memory containing the struct pages would be located elsewhere than the memory itself, with some strong barrier in between removals; but think there were patches posted just a few days ago, with intent to allocate struct pages from the same memory block. It would be easy to forget this writeback issue when hotremove advances, if we don't fix it properly now. Another problem with the s/if/while/ solution: I think Matthew pointed to another patch needed, to prevent wake_up_page_bit() from doing an inappropriate ClearPageWaiters (I've not studied that patch); and would also need a further patch to deal with my PF_ONLY_HEAD VM_BUG_ON(PageTail). More? I think the unreferenced struct page asks for trouble. Hugh 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 X-Spam-Level: X-Spam-Status: No, score=-14.8 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1, USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CBB78C2D0E4 for ; Tue, 24 Nov 2020 21:47:02 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 376322083E for ; Tue, 24 Nov 2020 21:47:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="VzQAooJ4" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 376322083E Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 3CA8A6B006E; Tue, 24 Nov 2020 16:47:01 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 379AA6B0070; Tue, 24 Nov 2020 16:47:01 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 28F836B0071; Tue, 24 Nov 2020 16:47:01 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0077.hostedemail.com [216.40.44.77]) by kanga.kvack.org (Postfix) with ESMTP id 124886B006E for ; Tue, 24 Nov 2020 16:47:01 -0500 (EST) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id C3C6D3633 for ; Tue, 24 Nov 2020 21:47:00 +0000 (UTC) X-FDA: 77520647400.27.line55_15031a427371 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin27.hostedemail.com (Postfix) with ESMTP id A074B3D668 for ; Tue, 24 Nov 2020 21:47:00 +0000 (UTC) X-HE-Tag: line55_15031a427371 X-Filterd-Recvd-Size: 7272 Received: from mail-oi1-f193.google.com (mail-oi1-f193.google.com [209.85.167.193]) by imf22.hostedemail.com (Postfix) with ESMTP for ; Tue, 24 Nov 2020 21:47:00 +0000 (UTC) Received: by mail-oi1-f193.google.com with SMTP id s18so391515oih.1 for ; Tue, 24 Nov 2020 13:47:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:in-reply-to:message-id:references :user-agent:mime-version; bh=04YZn/oOi9cwlQjeUz6wAhKNr23VM3kG20feokh0qeg=; b=VzQAooJ4hoEvvjBCc+1Qt2u70siZAJm6h9WtKGpP0ywmkQ1vspYXHydmfQoTf4ym3I oqKJzcR5krrs+o2BlUzYp3kcBIpfa6E6ew96qdIC2DTZU5N5pj5QjitRF0O5vAP3DLm/ Rw71YTkXEZBmOi1/VFYN3Po6znE+JQDeN5AQ5GT8PpDyb4DUQYVH9RDLVPxXCtlzpqGL NLPgS8k2q9XJZCcEqp0jixScddihjCkoUiurydf3GmXYEZPzwqkjXKYbEuCjos3F0Dgj X3M8gEHfAlkVbtkko2WkcUF0RRIGEvajB3ukYdimzdhZa9OdqeypZLEs6CnYwqCUGZF7 gNSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version; bh=04YZn/oOi9cwlQjeUz6wAhKNr23VM3kG20feokh0qeg=; b=NmWE13A4lPaOHk2rs9CKtBJ6IgkJdG9R08lVPyrMJqUzlGIdKY20YUMA20/jdk9ZhY fcILSsMBAmduG/XQ/o1wFTJSdXQxiUxiHcc5G5aNphuXJ/tmYDNM7MM6MlPlLeamIZaX Fr8/kdzvI9u9ke0krBVmv+ZySeKvt7QnDVkQwoFrw3RxZ+lxSIh2RMYf+HMNBB4Dexhi rz3E+EXd7zV58D580z0hrQ+6+3hBIWN2KD4CF0k9eD8KZ8dL4V0HSR2fGwPcJzo3NjCl e1mIbhHAoB6JlRHiPWLz4kjTegE5KmFRO7F5i55felUmWk3teSIwxNmWxV//AUYuLB60 WpsA== X-Gm-Message-State: AOAM531t9ySABLnRHbwwKSIH0TYnMq7WZ6PwquN7s4jI/SWoF9dEMFJq ONArTilb8t3aT7LpcR9bfEK3Eg== X-Google-Smtp-Source: ABdhPJxOVg+Bowm05sagrPqp3xv+YibTG16pRL9z0qkeXz1sv96JtkxPJaBWAFfkjlhlZQfRI/zZYw== X-Received: by 2002:aca:f5c8:: with SMTP id t191mr213734oih.40.1606254419120; Tue, 24 Nov 2020 13:46:59 -0800 (PST) Received: from eggly.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id a4sm139138otj.29.2020.11.24.13.46.56 (version=TLS1 cipher=ECDHE-ECDSA-AES128-SHA bits=128/128); Tue, 24 Nov 2020 13:46:58 -0800 (PST) Date: Tue, 24 Nov 2020 13:46:44 -0800 (PST) From: Hugh Dickins X-X-Sender: hugh@eggly.anvils To: Linus Torvalds cc: Matthew Wilcox , Hugh Dickins , Jan Kara , syzbot , Andreas Dilger , Ext4 Developers List , Linux Kernel Mailing List , syzkaller-bugs , Theodore Ts'o , Linux-MM , Oleg Nesterov , Andrew Morton , "Kirill A. Shutemov" , Nicholas Piggin , Alex Shi , Qian Cai , Christoph Hellwig , "Darrick J. Wong" , William Kucharski , Jens Axboe , linux-fsdevel , linux-xfs Subject: Re: kernel BUG at fs/ext4/inode.c:LINE! In-Reply-To: Message-ID: References: <000000000000d3a33205add2f7b2@google.com> <20200828100755.GG7072@quack2.suse.cz> <20200831100340.GA26519@quack2.suse.cz> <20201124121912.GZ4327@casper.infradead.org> <20201124183351.GD4327@casper.infradead.org> <20201124201552.GE4327@casper.infradead.org> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, 24 Nov 2020, Linus Torvalds wrote: > On Tue, Nov 24, 2020 at 12:16 PM Matthew Wilcox wrote: > > > > So my s/if/while/ suggestion is wrong and we need to do something to > > prevent spurious wakeups. Unless we bury the spurious wakeup logic > > inside wait_on_page_writeback() ... > > We can certainly make the "if()" in that loop be a "while()'. > > That's basically what the old code did - simply by virtue of the > wakeup not happening if the writeback bit was set in > wake_page_function(): > > if (test_bit(key->bit_nr, &key->page->flags)) > return -1; > > of course, the race was still there - because the writeback bit might > be clear at that point, but another CPU would reallocate and dirty it, > and then autoremove_wake_function() would happen anyway. > > But back in the bad old days, the wait_on_page_bit_common() code would > then double-check in a loop, so it would catch that case, re-insert > itself on the wait queue, and try again. Except for the DROP case, > which isn't used by writeback. > > Anyway, making that "if()" be a "while()" in wait_on_page_writeback() > would basically re-introduce that old behavior. I don't really care, > because it was the lock bit that really mattered, the writeback bit is > not really all that interesting (except from a "let's fix this bug" > angle) > > I'm not 100% sure I like the fragility of this writeback thing. > > Anyway, I'm certainly happy with either model, whether it be an added > while() in wait_on_page_writeback(), or it be the page reference count > in end_page_writeback(). > > Strong opinions? Responding to "Strong opinions?" before having digested Matthew's DMA sequence (no, not his DNA sequence). I think it comes down to whether my paranoia (about accessing an unreferenced struct page) is realistic or not: since I do hold that paranoia, I do prefer (whatever variant of) my patch. I'm not a memory hotremove guy. I did search mm/memory_hotplug.c for references to rcu or stop_machine(), but found none. I can imagine that the memory containing the struct pages would be located elsewhere than the memory itself, with some strong barrier in between removals; but think there were patches posted just a few days ago, with intent to allocate struct pages from the same memory block. It would be easy to forget this writeback issue when hotremove advances, if we don't fix it properly now. Another problem with the s/if/while/ solution: I think Matthew pointed to another patch needed, to prevent wake_up_page_bit() from doing an inappropriate ClearPageWaiters (I've not studied that patch); and would also need a further patch to deal with my PF_ONLY_HEAD VM_BUG_ON(PageTail). More? I think the unreferenced struct page asks for trouble. Hugh