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 44B94C6379D 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 E57442083E 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 S2387605AbgKXVrB (ORCPT ); Tue, 24 Nov 2020 16:47:01 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54852 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387573AbgKXVrA (ORCPT ); Tue, 24 Nov 2020 16:47:00 -0500 Received: from mail-oi1-x243.google.com (mail-oi1-x243.google.com [IPv6:2607:f8b0:4864:20::243]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2A658C061A4F for ; Tue, 24 Nov 2020 13:47:00 -0800 (PST) Received: by mail-oi1-x243.google.com with SMTP id v202so345504oia.9 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=hiuotxrJdavwaBnDjl8Jnym4WpvDyrCGBqQ/4Qj4/b7kDGK6zojTGzKiTDdPhqpT6B gPmKcaxq9F11pZNqtbLHu0uAFl3ktdJdG2a7WMQN2LF2YwG1TkVeAaKp+eTogvwtG8q3 rIwFSfZrqh5XYrQa+90RPqmqqdloRvnLKhD4o2rdz47FQ+H8OxPQLN5YSj/v8v7Yay+/ /kNtuziYE/rm6iefFpZQlJ4QJcuJ6FzsQ/04yLd5OdgYgEZxqKPopjuEHlOYkWfgemQx 7yxxK8P5FzCwuoTfuogfJmrY6Q5sgdLEQBL0zbP9noVs0vnMOkN1TqEovP4gQX3TGh8+ Aq9w== X-Gm-Message-State: AOAM531JLb3LnfHgP9NKgdx+AIGc4RQgmoA9b8ZRN3mgg0vSoukj4J3/ wTQSbCCykST/tLjXLe22LoXqHg== 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-xfs@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