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 B9C20C3DA64 for ; Thu, 1 Aug 2024 18:53:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 25CF56B007B; Thu, 1 Aug 2024 14:53:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1E7456B0082; Thu, 1 Aug 2024 14:53:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 062606B0083; Thu, 1 Aug 2024 14:53:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id DA8C36B007B for ; Thu, 1 Aug 2024 14:53:27 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 62C4E8100A for ; Thu, 1 Aug 2024 18:53:27 +0000 (UTC) X-FDA: 82404574854.23.BDBF73C Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf04.hostedemail.com (Postfix) with ESMTP id 9AE3D4001C for ; Thu, 1 Aug 2024 18:53:25 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ZaOYwW9B; spf=pass (imf04.hostedemail.com: domain of nathan@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=nathan@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722538377; 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=72jbEyYdcMeUgcNXsbhYMfCuzyWoqPMc0qp6nOTicIk=; b=j6F/8qKBAzvMP5ISDHSU2qu4gE2+OKvHrGgOr7Ne6YbdesRTHslPDAN8lsuOG2pLvRgl21 RgQZ+OsbFXIfm936ut4nEDdjBBuv9NJWfq33dl8MVTZ3N5czp3jr2WyeSZWsYoksPUOSjY Dnl7eDIhZp0/zhgnkSsT92kLA1ftJVQ= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ZaOYwW9B; spf=pass (imf04.hostedemail.com: domain of nathan@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=nathan@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722538377; a=rsa-sha256; cv=none; b=rON6MRja18GaCunlMuU+xKuiHbejTr9Rzo4QCUpzW/BvriBh/sGse6aZvC7OpZdZ8LaM1k jZjezvYiJ2HpVjjoptAZde0qznq5Pzu/21zuTZqZeVWPBFIn8vAXGdaTj4qWMooc5K1o1t jWU+CKpcS/8c4sLgKsOtVq1DV8A55pI= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 9509962946; Thu, 1 Aug 2024 18:53:24 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id DEE6AC32786; Thu, 1 Aug 2024 18:53:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1722538404; bh=8fSyU2sb8kmX3R6gndd/1GgRwRBUKnHGxw9g8uAGMbo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZaOYwW9BfCu9i44CDKEZGhyyXPNNch8oGDnGletL0Un/ouU/aRaMFsLFkmv3lEuCA +7ja6mmwZph8X7iGE+xEYRD3mNq6ZdnIoJFT4jomwOyfbEBDmUzTmFTH0mXMMNip/R +cC7yytW8K1ClqfzSE1o2mXpikLIp+I4oTE0sAjOwIlFrdSp79VYgVL/f2ZMw/6xmz ILYvtEswuHcHI/Jn94fctSTX7WysarchHqvcFLklSEBcoGtNXCh6wBvvk6VL/1kd1y km1sHZPziIGOyZai3aJNrYlTlJmtXlDnhKVDU3TwqIXepvzZYzfLSAIko0Ar2py1d8 sdzQc78+tt0PQ== Date: Thu, 1 Aug 2024 11:53:21 -0700 From: Nathan Chancellor To: Simon Horman , David Howells , Christian Brauner Cc: Steve French , Matthew Wilcox , Jeff Layton , Gao Xiang , Dominique Martinet , Marc Dionne , Paulo Alcantara , Shyam Prasad N , Tom Talpey , Eric Van Hensbergen , Ilya Dryomov , netfs@lists.linux.dev, linux-afs@lists.infradead.org, linux-cifs@vger.kernel.org, linux-nfs@vger.kernel.org, ceph-devel@vger.kernel.org, v9fs@lists.linux.dev, linux-erofs@lists.ozlabs.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 18/24] netfs: Speed up buffered reading Message-ID: <20240801185321.GA2534812@thelio-3990X> References: <20240729162002.3436763-1-dhowells@redhat.com> <20240729162002.3436763-19-dhowells@redhat.com> <20240731190742.GS1967603@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240731190742.GS1967603@kernel.org> X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: 9AE3D4001C X-Stat-Signature: da3r8h1sr54u3tcdrkr1e6zwsciit34o X-HE-Tag: 1722538405-563923 X-HE-Meta: U2FsdGVkX1/SskH/Ue90vOaQK/TWMV1RGP0/BZi7JnD+FlQe2sbb3yh7iOFvk/2CG57pN17sm0of1NTDJf51t7fFQq7rdQGaU+zGAdPRoKiUkQkcAGZ5cl+Kiej3jasp5Gy7r4uFEhFBU5oKLItJir6/Tzaa6LNeEbmU+i7cM1ukuDLMZ1cdnOKPvrI7wxsTWipPMV/Mxu2yj7VXiOeyWBAim2kGrvL1SjF9B3WaDVfQnGx7UOqHAUIg/jJGusahyXgNCpeWkqhbK5g3e3i/i+SKtwuBqyVelqCQWGDiYMgsV7EXEruZbkfXsaM3EikaBXvfxpRsLyQ6ja4DzcqVsksxIp2XOPevh79REEcrbnbaO2b3LdaHp8ztJxgSCcpqPrb9jbi/CeR6u/2eDlt4jkZ6XncfnLvdsb/7ldsDRgji/pMphZBalIqcROshSBLdAbi4mxdXC3w6NHzykcnUqNzUxcTYXjKbWHjpHMSGpThMuc1TRFQBcmD5weXkQ5mlWnYnuz8YCnEgvXPp7QAeyXNXQEfn0HaPDPFzUdMlIDdTJ22LFmNzyZfXLorZiYYoypnOIqF+D2WFDiVyaTP7rvVPgn58gcV8WrRdFK7r+jWzCXXR5trY8aFPudJ0ho3FBx4aQNMCc4rneqrHTaoR7wfv66i5IrGsLfNoOZBForWE8qbqUuidyeRrHrV+XDuZ20sDBJjf7/uLXFv7XQ3pmTcNTJeSQUjzN9nDFC4GIP+DhUdWD/VERw01ldRp1+uN9YewrRXkNx5+iRltrM+6Ja44vnIAMqbAzkEfumcLuZ1PNCNmgr7GGvujlt0urCPzWxRvpLo/abvzxE+F29gOiSKTqpCAZlIDs7Nvhu0IyyewQKAVxvjX0qxIGZ8+mHt69BAcmehrGx8eySsHTVICFbPFCElGHPoMg4JkX77+bSwqi9raLdH/omYnhmVB3HpqbLaMAzFlrFduZmV2omz /l0pWmBK 5IiVpO387/TrGIu835Tb4fylbScg+d1fnqhzvtCX9rC7gQN42IejT79KEgKFblG0FpIF4eG9u6a5UDUl5x9hO+H+A3UTVw2PpRbU1WKZPRe31pUvVBqM164wBWPg1eFDZz7y9eOTulCBYg6hOtC9V9N2HdVueDPszOuMb7JetO+qNJNWRQE3NI/w6bF1Gjk12KKYTBIVdCSNDtzfZT9BH/7IbaoKlvrKhrKb2q2xWfFaBY3GjFzoVENXfKsvT464IxDK2Ym7YlIrer24= 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 Wed, Jul 31, 2024 at 08:07:42PM +0100, Simon Horman wrote: > On Mon, Jul 29, 2024 at 05:19:47PM +0100, David Howells wrote: > > ... > > > diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c > > ... > > > +/* > > + * Perform a read to the pagecache from a series of sources of different types, > > + * slicing up the region to be read according to available cache blocks and > > + * network rsize. > > + */ > > +static void netfs_read_to_pagecache(struct netfs_io_request *rreq) > > +{ > > + struct netfs_inode *ictx = netfs_inode(rreq->inode); > > + unsigned long long start = rreq->start; > > + ssize_t size = rreq->len; > > + int ret = 0; > > + > > + atomic_inc(&rreq->nr_outstanding); > > + > > + do { > > + struct netfs_io_subrequest *subreq; > > + enum netfs_io_source source = NETFS_DOWNLOAD_FROM_SERVER; > > + ssize_t slice; > > + > > + subreq = netfs_alloc_subrequest(rreq); > > + if (!subreq) { > > + ret = -ENOMEM; > > + break; > > + } > > + > > + subreq->start = start; > > + subreq->len = size; > > + > > + atomic_inc(&rreq->nr_outstanding); > > + spin_lock_bh(&rreq->lock); > > + list_add_tail(&subreq->rreq_link, &rreq->subrequests); > > + subreq->prev_donated = rreq->prev_donated; > > + rreq->prev_donated = 0; > > + trace_netfs_sreq(subreq, netfs_sreq_trace_added); > > + spin_unlock_bh(&rreq->lock); > > + > > + source = netfs_cache_prepare_read(rreq, subreq, rreq->i_size); > > + subreq->source = source; > > + if (source == NETFS_DOWNLOAD_FROM_SERVER) { > > + unsigned long long zp = umin(ictx->zero_point, rreq->i_size); > > + size_t len = subreq->len; > > + > > + if (subreq->start >= zp) { > > + subreq->source = source = NETFS_FILL_WITH_ZEROES; > > + goto fill_with_zeroes; > > + } > > + > > + if (len > zp - subreq->start) > > + len = zp - subreq->start; > > + if (len == 0) { > > + pr_err("ZERO-LEN READ: R=%08x[%x] l=%zx/%zx s=%llx z=%llx i=%llx", > > + rreq->debug_id, subreq->debug_index, > > + subreq->len, size, > > + subreq->start, ictx->zero_point, rreq->i_size); > > + break; > > + } > > + subreq->len = len; > > + > > + netfs_stat(&netfs_n_rh_download); > > + if (rreq->netfs_ops->prepare_read) { > > + ret = rreq->netfs_ops->prepare_read(subreq); > > + if (ret < 0) { > > + atomic_dec(&rreq->nr_outstanding); > > + netfs_put_subrequest(subreq, false, > > + netfs_sreq_trace_put_cancel); > > + break; > > + } > > + trace_netfs_sreq(subreq, netfs_sreq_trace_prepare); > > + } > > + > > + slice = netfs_prepare_read_iterator(subreq); > > + if (slice < 0) { > > + atomic_dec(&rreq->nr_outstanding); > > + netfs_put_subrequest(subreq, false, netfs_sreq_trace_put_cancel); > > + ret = slice; > > + break; > > + } > > + > > + rreq->netfs_ops->issue_read(subreq); > > + goto done; > > + } > > + > > + fill_with_zeroes: > > + if (source == NETFS_FILL_WITH_ZEROES) { > > + subreq->source = NETFS_FILL_WITH_ZEROES; > > + trace_netfs_sreq(subreq, netfs_sreq_trace_submit); > > + netfs_stat(&netfs_n_rh_zero); > > + slice = netfs_prepare_read_iterator(subreq); > > + __set_bit(NETFS_SREQ_CLEAR_TAIL, &subreq->flags); > > + netfs_read_subreq_terminated(subreq, 0, false); > > + goto done; > > + } > > + > > + if (source == NETFS_READ_FROM_CACHE) { > > + trace_netfs_sreq(subreq, netfs_sreq_trace_submit); > > + slice = netfs_prepare_read_iterator(subreq); > > + netfs_read_cache_to_pagecache(rreq, subreq); > > + goto done; > > + } > > + > > + if (source == NETFS_INVALID_READ) > > + break; > > Hi David, > > I feel a sense of deja vu here. So apologies if this was already > discussed in the past. > > If the code ever reaches this line, then slice will be used > uninitialised below. > > Flagged by W=1 allmodconfig builds on x86_64 with Clang 18.1.8. which now breaks the build in next-20240801: fs/netfs/buffered_read.c:304:7: error: variable 'slice' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] 304 | if (source == NETFS_INVALID_READ) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/netfs/buffered_read.c:308:11: note: uninitialized use occurs here 308 | size -= slice; | ^~~~~ fs/netfs/buffered_read.c:304:3: note: remove the 'if' if its condition is always true 304 | if (source == NETFS_INVALID_READ) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 305 | break; fs/netfs/buffered_read.c:221:16: note: initialize the variable 'slice' to silence this warning 221 | ssize_t slice; | ^ | = 0 1 error generated. If source has to be one of these values, perhaps switching to a switch statement and having a default with a WARN_ON() or something would convey that to the compiler? Cheers, Nathan