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 A20EAC02192 for ; Mon, 3 Feb 2025 14:04:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 165146B0083; Mon, 3 Feb 2025 09:04:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0EDB26B0085; Mon, 3 Feb 2025 09:04:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EA9CB6B0088; Mon, 3 Feb 2025 09:04:43 -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 C9A6E6B0083 for ; Mon, 3 Feb 2025 09:04:43 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id C00D8C090C for ; Mon, 3 Feb 2025 14:00:04 +0000 (UTC) X-FDA: 83078792328.11.E86336B Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf30.hostedemail.com (Postfix) with ESMTP id 14C5C8001A for ; Mon, 3 Feb 2025 14:00:02 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=HgvYe2rP; spf=pass (imf30.hostedemail.com: domain of mcgrof@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=mcgrof@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738591203; 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=LEoaPzXWEKlKMZ+UAoBezKxhtXzIvIVTknPKduQ6qfM=; b=ICye4Hj6kTULEjndQqcFbv6b8vO9zz1OEDgE7T5sggHFHxiiltKfoA1ajpdycqAVKzYLEm h6eJVnF8Ikwf1LSDW7CnOmq7Jt+2ssxI+PzmA8GxZAvUaCc1T/JgQPsj/Rxd/C+VLq6zaQ dWBepUrQWpMKckjNh9qMSbWCd3+MdZs= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=HgvYe2rP; spf=pass (imf30.hostedemail.com: domain of mcgrof@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=mcgrof@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738591203; a=rsa-sha256; cv=none; b=jFqcaQij8UtZQdYTCe07KzILgp4PkhXN6o6r2Jioscnz/l3aZt9pXDJZ2xSDqpuEiwLF+R 5G9wkxUs13oNY4YJVwXuual+YI60veJmADKv4q20gRUa8FtpFTqpYLLQhGJCO2Q8uW1pMG B12+oklOMads2H31jV3bIkcs9QcjJgk= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 0AA3EA4123A; Mon, 3 Feb 2025 13:58:16 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 81BBBC4CED2; Mon, 3 Feb 2025 14:00:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1738591202; bh=DodJHTwvmvoQJhOboL7veVJCLIW/qyqGwOMGQqc+N4I=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=HgvYe2rPWcMjSyAu3Eokhg0NfcNuRWANm4Gyz/4hOOBktMJSf8tin3RUlguKJqOk4 bEgKeU/PSciJwSbVdm+qO/peNzrP+IoB9+9Opsuh9POZy7jilaHSxoGkCMYHtaUNan E1eQtyzugdwz57zepOXLKhHZvZNCnWGn/8+e9Od8224p/I9D5sTNS9ve7HWAlZ3nAN bYEqYGuuOk+Y44bUThSnihWPSNVGLIOQksmEe65mV/BNjt7Zo3tQ5Vts3dzpogMa+5 2juoCX6Yg03qW7uTt9yZ2o6FWyEdnAipDTyI7/rlKj1/6SBiMVgl4aYefXniuy7c1Q yNcWcJlhI7Eig== Date: Mon, 3 Feb 2025 06:00:00 -0800 From: Luis Chamberlain To: Matthew Wilcox 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-Rspamd-Queue-Id: 14C5C8001A X-Stat-Signature: 1haztiiep5kmfmartjjg448mgpo3hzax X-Rspamd-Server: rspam08 X-Rspam-User: X-HE-Tag: 1738591202-61802 X-HE-Meta: U2FsdGVkX18KTH5sjYnGOtMqb48dhEJYoJUVMPIptj4Yq0olHwZ07svoSkXOhjV7aoQmPZnMJiEO8MY8iqTgeJ1qdmOGDYGy6Q9pDDamNpZ0S68AiQ9ArrqChUPBQuTmhrg5S5+agzkedMiyCX2WqWqo/lO2ULlnijmiOY/6Oc78EDPCU82RqJuCFbcRUh1wi2gK1dwXCutkTP4T2gBVkT7PfWQhgYXZZOnZUpvaV9i/W2tp5Fqq6+aBT9P6R9HQiikPx6MppsETOA/WmENqo9QO2j2m305+9rtaRqlP0ZlVRk3n7nfkayCyw4Av1jvtx4060ouGHuh3wu4mILVUoFat9A9zjM1+GNvztYQOdp2y174CCPUmm6wAK3fwEHrk3f55OigeuvsZ0Qjg1WOAHx5aIsGkfyhnNq2rH4lPLZrAaPwp/N3+j4Ib32yOfm8IJdFPvm+7AEmaJW78BYWkQHe+t6FUW8JGQAjDBEF9mSGhClMiT78mnjsWZ/J999WhFiKm5oIyfHG3eK1kbbD4c2/blLdWhEzxhrURDF1xRkBP8uPKL/ohVBAZDIC4XKvmcQsXFO6xNkK7kAjQ95EzMhgYTEIzZrHrjj+x/+CgRMgwIee2pqAeYpMz/fdGkoLMliWIgdRUZKMF4teZFjDBgnUZm6DaK4qyc5wgJA8/4EWK7+HGpcsT0fJMnza4uWdrx7tjwWfSYZ1ub0xcGST0ovyBjo8IT9/t9VW30sul/ZMeoEEt+N34h+mRRSK010tRRUdNVE3sU8wflMX9jhSEKnwXsC8xGD6eAUmEm+ZR4gezx6WKFDbTIMpyTiIIV41ifK64pcrkXvIUrOa520aRlcBXqmfk+a8nilcQsuG6Bek1XVwBnZ27/hujIV5sPnlm9QIntCgQfpz4Z7PSrc8I3rh6ybB5NU/h2wn2z/uxn8KpWuzNrraBMOInrWXlAk6PvDUCsX7EyzrLo1hGMFv NyXF9oK/ CrkSMGaRmnZRSxxQlKnrxWQC0fKstr+53AnQa+9spSB3IwLi0zkkIuAP28tSRHTWGrZj3hB/yo9EtaT7jtPMi29UI4F/LUEE6hYJDtkBt6cftyOsdXpc+SVr/EvAsRMkexkaW4lYJQ13SqvANajfO2tBh5dCuUeMxYhiZEw+L+QZgUxBNbpV0gty1Ot8mWw1ovUnztpCEBRHyGbeneza2FWA5LQn4FASBznBV5bhOXj7cUtOUMTvAPIPG9T5MTtfYgGF2ehLri1cqJbJN/PqhE6lm033kNF6A7GJa 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 10:01:46PM +0000, Matthew Wilcox wrote: > 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: Yes that is what I meant, sorry. > > 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. Indeed, also, its not just every buffer on the page, since we can call end_buffer_async_read() on every buffer in the page we can end up calling end_buffer_async_read() on every buffer in the worst case, and on each loop above we start from the pivot buffer up to the end of the page. > 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(). Ah yes, thanks, this clarifies to me what you meant! Luis