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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 88CE4C0218F for ; Fri, 31 Jan 2025 22:01:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D8A9F6B007B; Fri, 31 Jan 2025 17:01:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D3C796B0082; Fri, 31 Jan 2025 17:01:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C02F96B0083; Fri, 31 Jan 2025 17:01:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id A1D346B007B for ; Fri, 31 Jan 2025 17:01:57 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 4E233141182 for ; Fri, 31 Jan 2025 22:01:57 +0000 (UTC) X-FDA: 83069120274.17.74295C6 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf09.hostedemail.com (Postfix) with ESMTP id EDEC8140011 for ; Fri, 31 Jan 2025 22:01:54 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=cqC6Lzbi; dmarc=none; spf=none (imf09.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738360915; a=rsa-sha256; cv=none; b=nvtE6TDOy8APRFknVWDoqItkFvMJR+mN1RfuSxmmNVhjPV2Qv+aUda2lI2ZkgwRPKz0xho Uo6FJWxK4VVqG3uzHI1HqYPOgKCjv6rcaj8DvCy046e1fjXj3P8YBLiPAnML1gKsi77zyn YzV/Swh4I7N5P5Ycbuz41NihUb/z3eY= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=cqC6Lzbi; dmarc=none; spf=none (imf09.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738360915; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=R5v3Vp8tsw1LE/lxKen0G1myY0wqF26imHFfjT4SmXY=; b=7E91jciceNellW0EBUgFbBiZVAEduh4a7hc23HfabnbyU/WzW/pezdl61aV+yB90Jzqf+E l3/+v2Tug12M3RSxAloiBHQMHvdXReayFtpnlf8QYlHs9vJI8cmWfqLRU+pebWdri+g+C6 dene+wpG58Q6qzyif4jZPl3WMclrEso= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=R5v3Vp8tsw1LE/lxKen0G1myY0wqF26imHFfjT4SmXY=; b=cqC6LzbiijGTbh/tgJBpGzZD6w RgljOVnsa/poOlHqwhg+1K5UbvltHwAVrRsp/XzoAgwPctDvlqRtcIU3Vt13pryrPmMqBOwB5VRTK 8niKO0DUEbLSO/gmesV4IPh4O/MiwfMe02g82j6NnIEVOYpOcVXEYAoFSw9t/NbVakrMof+NdjO2t 4Ze1IxToitidCy8cCcW+qQcq8H01itmjNMqpULSP1V00XQqKqV4coLdnKOs9hbO6Zy2d2KhT+Lqz0 vtzFPCHrdozxjxEUbUVsbRMxZr3lHac0gKWehVCRGYcpwel2qgK/MNwUV3YnpkHB9mkOEjhTLB+ky CmjRPJSg==; Received: from willy by casper.infradead.org with local (Exim 4.98 #2 (Red Hat Linux)) id 1tdz58-0000000FCc3-1I5f; Fri, 31 Jan 2025 22:01:46 +0000 Date: Fri, 31 Jan 2025 22:01:46 +0000 From: Matthew Wilcox To: Luis Chamberlain Cc: hare@suse.de, dave@stgolabs.net, david@fromorbit.com, djwong@kernel.org, kbusch@kernel.org, john.g.garry@oracle.com, hch@lst.de, ritesh.list@gmail.com, linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-mm@kvack.org, linux-block@vger.kernel.org, gost.dev@samsung.com, p.raghav@samsung.com, da.gomez@samsung.com, kernel@pankajraghav.com Subject: Re: [PATCH 0/5] fs/buffer: strack reduction on async read Message-ID: References: <20241218022626.3668119-1-mcgrof@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Queue-Id: EDEC8140011 X-Rspamd-Server: rspam10 X-Stat-Signature: d73qq3dc43sy894546mgcp65xipimshn X-HE-Tag: 1738360914-566145 X-HE-Meta: U2FsdGVkX1/3C57fHOP5n+/4lF50b37gnHRIiCy5Qgs4cX9JtxjsvpkMx+hEeOCs3EHsYOYaj7WgbUVwt0XF3ss7E6ql3x+WTJDDenWBuUbaHrUtdb+jk0b6RxWOIRHEWahgQlfWixosItxsRZ7ew1TzFaO5Cpy2QiWLZ8lV3GGl09GvRLOEEfOwadnR/n6aq0ydxXHww3QRL7Pfy7EoA1M9eCI+NGZYEN4XITIG++NqoUDR+nswnH3qK7IvxQTYzmUfzQdZnfMUMDS5E3dheNMJDC+3eEFpuBIzu2SUlnCPUzMvKl+izL0bdBCnTLlIIIZW8nwZqjkM6SEQRxeyESRIwjQ3ht/0WLO6U58hk7ic6QT5NAiaTNJAzVO18xujx12wSJ8vjmRk7vL3n5lbP2Sc3e0PzP5PMXU8Qu/35+C146aKWymmRHQ35pVIyFoLOOZA56bUwXirulzbDNJBb34HZBqyEc750BZKWMCSRL7I+E9D37fqGGp1KLxp0m9LmYi0aNiAZKzicdCB3DQRAcXEL+56+xRRthtZziJSSWtARneDXHyztwtcDfaCFB3nHxUL5evK0FI/5NQP6bI69c1qnub+LkPqIQyveY8KgKAYPS2GOPGMBVJ0rEJ2ghwHuj5B9ddfRqn44n5qIEIHfSh5fOo8uMlyh3snRxQI8XCkKQcwy5rgm69Zvac8XAajGiCrhJGo40O0D/33OSIonQLHob4KFK3izbYzcjtmM7Ttq87pOcBU+TRGwoKiIYxseu8HChYykLoA+AFtyT2C9TIryEGn1/42RnHxT1ToFc9Ro/7Be+AryTLhTGy4q0SakuWj4XaB26gAXj4nS3LqqNm2xhOevZVR9J5imnVipfqeJjKdpl9LCYextjPTv68HiO8xAJqHtl0+WNGYvmFrssYdeCjjD8ndV/wA2JprxzSQYPv+TZltGNxEvbx9tkGoCjmZ2CPvnyD8z3SVC0U GtaieN3+ Ig4GvWOUVl82MGUtHQddVjJIL4rvnOsW6YIikDeNG9kIF9bGbHcCXxMVW683YUVsRUVuxdYRPunXg4Tj4CqRwXHqZz5/TPs/t9QS2yd+IS9rV0U6Fl3DDIC7IMOkHMUuAXC/hn9AblZm3r0tANRRd4Sm3jkOEd3a1s9Gj/B1pM6oOdUyGYnUbjDZEJq+K+CmKGDV3nSZwogYjxtKXs37TmpxJfAGkD4U6G1wlsAD6eJGBtf43DRYYyvybdSPfFRExLWsH8RplN5Ao67HkFroFPzvviULhwFE8rkZwoOlIUUEHUZY= 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: List-Subscribe: List-Unsubscribe: On Fri, Jan 31, 2025 at 08:54:31AM -0800, Luis Chamberlain wrote: > On Thu, Dec 19, 2024 at 03:51:34AM +0000, Matthew Wilcox wrote: > > On Wed, Dec 18, 2024 at 06:27:36PM -0800, Luis Chamberlain wrote: > > > On Wed, Dec 18, 2024 at 08:05:29PM +0000, Matthew Wilcox wrote: > > > > On Tue, Dec 17, 2024 at 06:26:21PM -0800, Luis Chamberlain wrote: > > > > > This splits up a minor enhancement from the bs > ps device support > > > > > series into its own series for better review / focus / testing. > > > > > This series just addresses the reducing the array size used and cleaning > > > > > up the async read to be easier to read and maintain. > > > > > > > > How about this approach instead -- get rid of the batch entirely? > > > > > > Less is more! I wish it worked, but we end up with a null pointer on > > > ext4/032 (and indeed this is the test that helped me find most bugs in > > > what I was working on): > > > > Yeah, I did no testing; just wanted to give people a different approach > > to consider. > > > > > [ 106.034851] BUG: kernel NULL pointer dereference, address: 0000000000000000 > > > [ 106.046300] RIP: 0010:end_buffer_async_read_io+0x11/0x90 > > > [ 106.047819] Code: f2 ff 0f 1f 80 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 53 48 8b 47 10 48 89 fb 48 8b 40 18 <48> 8b 00 f6 40 0d 40 74 0d 0f b7 00 66 25 00 f0 66 3d 00 80 74 09 > > > > That decodes as: > > > > 5: 53 push %rbx > > 6: 48 8b 47 10 mov 0x10(%rdi),%rax > > a: 48 89 fb mov %rdi,%rbx > > d: 48 8b 40 18 mov 0x18(%rax),%rax > > 11:* 48 8b 00 mov (%rax),%rax <-- trapping instruction > > 14: f6 40 0d 40 testb $0x40,0xd(%rax) > > > > 6: bh->b_folio > > d: b_folio->mapping > > 11: mapping->host > > > > So folio->mapping is NULL. > > > > Ah, I see the problem. end_buffer_async_read() uses the buffer_async_read > > test to decide if all buffers on the page are uptodate or not. So both > > having no batch (ie this patch) and having a batch which is smaller than > > the number of buffers in the folio can lead to folio_end_read() being > > called prematurely (ie we'll unlock the folio before finishing reading > > every buffer in the folio). > > But: > > a) all batched buffers are locked in the old code, we only unlock > the currently evaluated buffer, the buffers from our pivot are locked > and should also have the async flag set. That fact that buffers ahead > should have the async flag set should prevent from calling > folio_end_read() prematurely as I read the code, no? I'm sure you know what you mean by "the old code", but I don't. If you mean "the code in 6.13", here's what it does: tmp = bh; do { if (!buffer_uptodate(tmp)) folio_uptodate = 0; if (buffer_async_read(tmp)) { BUG_ON(!buffer_locked(tmp)); goto still_busy; } tmp = tmp->b_this_page; } while (tmp != bh); folio_end_read(folio, folio_uptodate); so it's going to cycle around every buffer on the page, and if it finds none which are marked async_read, it'll call folio_end_read(). That's fine in 6.13 because in stage 2, all buffers which are part of this folio are marked as async_read. In your patch, you mark every buffer _in the batch_ as async_read and then submit the entire batch. So if they all complete before you mark the next bh as being uptodate, it'll think the read is complete and call folio_end_read().