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 E880EC433F5 for ; Mon, 29 Nov 2021 21:23:54 +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:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=FY8rfjq7SWpjkMwWm9ks6f9ER+t17IQrmqMXcNn8wYE=; b=h0erW7q91OjB7x GP/bm6Urd7cl/gGu7Jp7cFUgUnzHBppdvIHFwBc3v72PTG6Z4eNvORO1xnsT2TNIKqve8I5fucE6O MEu1A2tn2IKXUTaQaFcKzDaikEt4XRMqrb+rIDodiflafkVdV8dTTf+8x1mVEdRMzREhW801T6Cd0 zCCysT6KaUQw9bgoymQQ6ETP+RM7Nw8A/F7PWSFq1I/W6UyyUImdMjvwziFHM78Q6X+shT8FiEFV+ oipMLJKZJvtsg+yEDZgSJgbc935Fw0g3D0T4OfHj+y4sFoQAqBjH5XxxhnA8uIuVSDV0mLUxMIdoP N/p7PE0SF+43+PvhT9iQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mro6S-002hev-K0; Mon, 29 Nov 2021 21:22:25 +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 1mrnhA-002XpZ-Dp for linux-arm-kernel@lists.infradead.org; Mon, 29 Nov 2021 20:56:18 +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 1FB8FB815FE; Mon, 29 Nov 2021 20:56:15 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B26FAC53FAD; Mon, 29 Nov 2021 20:56:11 +0000 (UTC) Date: Mon, 29 Nov 2021 20:56:08 +0000 From: Catalin Marinas To: Linus Torvalds Cc: Andreas Gruenbacher , Matthew Wilcox , Josef Bacik , David Sterba , Al Viro , Andrew Morton , Will Deacon , linux-fsdevel , LKML , Linux ARM , linux-btrfs Subject: Re: [PATCH 3/3] btrfs: Avoid live-lock in search_ioctl() on hardware with sub-page faults Message-ID: References: <20211127123958.588350-1-agruenba@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211129_125616_787390_B71A8CD9 X-CRM114-Status: GOOD ( 30.16 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Nov 29, 2021 at 10:40:38AM -0800, Linus Torvalds wrote: > On Mon, Nov 29, 2021 at 7:36 AM Catalin Marinas wrote: > > That's what this series does when it probes the whole range in > > fault_in_writeable(). The main reason was that it's more efficient to do > > a read than a write on a large range (the latter dirtying the cache > > lines). > > The more this thread goes on, the more I'm starting to think that we > should just make "fault_in_writable()" (and readable, of course) only > really work on the beginning of the area. > > Not just for the finer-granularity pointer color probing, but for the > page probing too. I have patches for the finer-granularity checking of the beginning of the buffer. They need a bit of testing, so probably posting them tomorrow. > I'm looking at our current fault_in_writeable(), and I'm going > > (a) it uses __put_user() without range checks, which is really not great For arm64 at least __put_user() does the access_ok() check. I thought only unsafe_put_user() should skip the checks. If __put_user() can write arbitrary memory, we may have a bigger problem. > (b) it looks like a disaster from another standpoint: essentially > user-controlled loop size with no limit checking, no preemption, and > no check for fatal signals. Indeed, the fault_in_*() loop can get pretty long, bounded by how much memory can be faulted in the user process. My patches for now only address the outer loop doing the copy_to_user() as that can be unbounded. > Now, (a) should be fixed with a access_ok() or similar. > > And (b) can easily be fixed multiple ways, with one option simply just > being adding a can_resched() call and checking for fatal signals. > > But faulting in the whole region is actually fundamentally wrong in > low-memory situations - the beginning of the region might be swapped > out by the time we get to the end. That's unlikely to be a problem in > real life, but it's an example of how it's simply not conceptually > sensible. > > So I do wonder why we don't just say "fault_in_writable will fault in > _at_most_ X bytes", and simply limit the actual fault-in size to > something reasonable. > > That solves _all_ the problems. It solves the lack of preemption and > fatal signals (by virtue of just limiting the amount of work we do). > It solves the low memory situation. And it solves the "excessive dirty > cachelines" case too. I think that would be useful, though it doesn't solve the potential livelock with sub-page faults. We still need the outer loop to handle the copy_to_user() for the whole user buffer and the sub-page probing of the beginning of such buffer (or whenever copy_to_user() failed). IOW, you still fault in the whole buffer eventually. Anyway, I think the sub-page probing and limiting the fault-in are complementary improvements. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel