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 72810C56201 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 2B10E2158C for ; Tue, 24 Nov 2020 21:47:22 +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 S2387594AbgKXVrA (ORCPT ); Tue, 24 Nov 2020 16:47:00 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54850 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732603AbgKXVrA (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 20853C061A4E for ; Tue, 24 Nov 2020 13:47:00 -0800 (PST) Received: by mail-oi1-x244.google.com with SMTP id a130so355273oif.7 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=A2QcNW1Q5OsF6p+0r2ZZ0A84or5Uv7TlmXsSuclOqqSwL7SfvBVr0lkVhKQTntUKSk fixKxb9jRgJDXXEa+S1fWoUWmo/SZHTOJy5AdSo+CU1rVGUPK+mHqhwjd6/ZYc3cbO9i 243uFV3Hy+HrOrxqI3tJJyM5zhrZS0z5gFSKkC5unLZItAB/D7OUbFyNTt1imNRUZdPm UnXBAeXxFmV65vgEpOUYbzj4VKWwXsSLcXHF6m2suKBCglZCf/1g47z0x2BzuKfGdogl NHrlg3NPsz6jQmWJdn/LGWQyB72iS3det3juduoNf/ZNQR6lo62/1ILGobUNtRv8Q6tD 2SEQ== X-Gm-Message-State: AOAM5331bZiOMbJl7EjnNJbQBrTR8PAGpc0EAMSoDwdjf+tLrFZ2vGv7 624kjfjqO9omjjlUNaatRNAjFQ== 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-ext4@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