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 868F8C02198 for ; Wed, 12 Feb 2025 22:24:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1FE296B008C; Wed, 12 Feb 2025 17:24:46 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1D5476B0092; Wed, 12 Feb 2025 17:24:46 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 09D996B0093; Wed, 12 Feb 2025 17:24:46 -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 DE68B6B008C for ; Wed, 12 Feb 2025 17:24:45 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 8EFA2160F79 for ; Wed, 12 Feb 2025 22:24:45 +0000 (UTC) X-FDA: 83112723330.05.A31D65E Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf27.hostedemail.com (Postfix) with ESMTP id BED8F40003 for ; Wed, 12 Feb 2025 22:24:42 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=aiBYxE4+; spf=pass (imf27.hostedemail.com: domain of dhowells@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=dhowells@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1739399082; 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-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=EHAXmEebuXfGaW+Cbp856q5t3oORgVUeZdmoxZNWAV4=; b=6IlNqNgTEjZpgDr9bg1+7nbvNiT6PMQVEJFvK7upQm4w+DeyjatXRoXaXPLkjXo6gGEM2C CPIcRCBLmX1E0NJVHTsYJNiAIxt4+rNuWucDTs7hOVamRvwvUivGjpK/Hn1qQDAx8SLCyt qfBjT4tI0KjCPqeRMjY2vhdzVfF8nCQ= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=aiBYxE4+; spf=pass (imf27.hostedemail.com: domain of dhowells@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=dhowells@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1739399082; a=rsa-sha256; cv=none; b=npP/+IStMTW7ZYadTkEgzu7QrudbXWVgc1aWQZ5v+9gWOvSTx846Cgpdsd+q8879iaSkG5 Zyfr5/1HxRmLVNKqmyMotQXmNG9y+hNgIIYNTBaHkF+KnGJcueNc9YNA34IeCQKj51abph uygf70NgtlBjAP5a2CPZg/8Qe9a6fd0= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1739399082; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EHAXmEebuXfGaW+Cbp856q5t3oORgVUeZdmoxZNWAV4=; b=aiBYxE4+3AwDN6BNoRz5wg6pga7+qYWqH0KazYjVP+1SC5kSL+0LKPG3opBhTPL3Eaw/iQ ASHVy2UJmWtK8+KBfoDSnCHer692Hrha75Xm2xeHvbGUFRFayb417GWs3ulnxGY/p/AOZk GQcEG5pYKuuUpmJ3Oi3aAJPzFCTmCDs= Received: from mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-117-0JhFd5MBMq-N6YBSpy5hVg-1; Wed, 12 Feb 2025 17:24:40 -0500 X-MC-Unique: 0JhFd5MBMq-N6YBSpy5hVg-1 X-Mimecast-MFC-AGG-ID: 0JhFd5MBMq-N6YBSpy5hVg_1739399077 Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.15]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id B56B7195608B; Wed, 12 Feb 2025 22:24:36 +0000 (UTC) Received: from warthog.procyon.org.com (unknown [10.42.28.92]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id B79951955F21; Wed, 12 Feb 2025 22:24:29 +0000 (UTC) From: David Howells To: Christian Brauner Cc: David Howells , Ihor Solodrai , Max Kellermann , Steve French , Marc Dionne , Jeff Layton , Paulo Alcantara , Tom Talpey , Eric Van Hensbergen , Dominique Martinet , 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, linux-kernel@vger.kernel.org, Ihor Solodrai , Latchesar Ionkov , Christian Schoenebeck , Steve French Subject: [PATCH 3/3] netfs: Fix setting NETFS_RREQ_ALL_QUEUED to be after all subreqs queued Date: Wed, 12 Feb 2025 22:24:01 +0000 Message-ID: <20250212222402.3618494-4-dhowells@redhat.com> In-Reply-To: <20250212222402.3618494-1-dhowells@redhat.com> References: <20250212222402.3618494-1-dhowells@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.0 on 10.30.177.15 X-Rspam-User: X-Stat-Signature: gjxxqicu58xbefrrfmp8ouwqjb1aokgz X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: BED8F40003 X-HE-Tag: 1739399082-383218 X-HE-Meta: U2FsdGVkX19QcTNKa//1oLwAzRyS1N/slYci8NXpZBWBr85ue8jU/KNceD6i6PC/3IVaVcas68oZR/Xyp6pCuZf3dwvIF/yXnCv2xom+BJnQqMX8M+8Wu8oEeJ78x3cIfY5zHArrF8QQYxa/jpmXs7YusA/oALrbHy5CXEFBiKWFP9EMoOV3cF8vpZ8Q7kkdprcEuPzWIVoYtqBprdoI4OPbYSif44h/srM7y4uQxohyXIDoAuNjo9Fg8zkQp54EZFvvQehTip/nZ0G6R/miT6vAZupkgPAUxMikVBxtqCczMEDSKM7GFOe3UWurhi31aUCBuKf/ECnnxcIz1AW4f/ckKCdupNTXVxmDxM0GmzIk/m9jPAfisyILFJC+IV7Ag6MyEb9ZaWYD9d5xzWYrSyVVq7fgQUW+p77wmySVzMAebxSh4cc+oVfv45d0o+5iu+62hqcH9IwbB/Y6SrR+oH2eWIbma3Rsa+iBGwZFAttibAxOXqpWuS3mKJuopzzO7Nurdk1wA1XiBKWE04+e/CTeeeGVTizDOMK3n8o62B4CrW6aiUaMsH5U379rHkUCbutNnygz6sv5jzz8c+VWJ6huwdV8wRpy0VXdF2m94r8dzPb4ndttIQRrPSkznJCbweH6RiNh5jOhCJKRooWUmb+FJHK69P2nzCFRJ08JqrIN6q2NFdqGiEOY96nMczpltc2mhmFpPp/p+v+WEO5FsGi9n06j0TzDqem7KuM05O0fzmSd0Q4voVuM41dnGpI36pBqnrZjB8CUlLMfGqCSSgYaoUBYgezKdaPzMNt0qu7lbZTK2520xPDmG8HYkVgLu/eO+TVXZLoGAzN6KnACnGKMHWWF0s2DOlcDlzZcLA5Lr4LURs1rY5dgGD9JYwwn0Df4eFIsOorb30F055L9CZYvwTS6eJggNNq+C7R7XDoL18HOEQuOPPYm0uGZ/I1ubogZ1atbmFIdOs2x6bf i0U4nLyy GQeZLGlhnBfg3bE/9h7lKK+CtEKff2j3MHoeNa8MflZ97+sFrXs0h9zge4A3uE1s/8J4Fl+mKjNpGh19oWOmFpE2XP5kbr5LmziWNYV2I4NvJwBBseZR7gbhjf7m5qVYKMNJtIK7fOw2zZvESKudto2/WQIfIpiHjL/XZhGfp7t1wflNNrQY8CbXpsmay/7ReNd4YpUVnZrFCdCMJy8uc1HBR+IeDBCmfOOGA3mvy6nXvYakf9FyB5O8tN7Lfzc8Y+plU8kh+HGu9KM+2kEb+DMHFkpJrRziqL6TzfpxyBlah79fZgbjCwecr0bG06WyQqsNIAtKT8UaQKChIC6qeeAoNSywUgfHL7HM14zgIudns+DyTGaJpR/MrZB1kRly0+vkZSjxitfooi0IpdTqocQrDUtzny6zyIKdYGeyC1qASQZ+jDJ1rhC9KqonmNPHBJdYbwZK1ad7Oorja7nqopC77rzwaXvnCsXnsNYdVZQEKij2K0wvzzjx5H+tnkrIWUqNfnDqm5edgCiXK0cb5YqdTsvL5HF8f9SnYLzsMRs78ev8r7S/GTXZn7tneO/7Wu9NZakV2xYCkNOhx+7WmirCclJVEBoUPIQtaaYnIep0375AHi2JW2yLTHzTcXsVBdIfXXfYl72+B1KiPb6wnI0Q/GlKaIXBnUHjx+0rI3L7M8ybFqF324geU0Aw/R5CfXuB49rZa5NO1BAsyWB96Qejx1g== 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: Due to the code that queues a subreq on the active subrequest list getting moved to netfs_issue_read(), the NETFS_RREQ_ALL_QUEUED flag may now get set before the list-add actually happens. This is not a problem if the collection worker happens after the list-add, but it's a race - and, for 9P, where the read from the server is synchronous and done in the submitting thread, this is a lot more likely. The result is that, if the timing is wrong, a ref gets leaked because the collector thinks that all the subreqs have completed (because it can't see the last one yet) and clears NETFS_RREQ_IN_PROGRESS - at which point, the collection worker no longer goes into the collector. This can be provoked with AFS by injecting an msleep() right before the final subreq is queued. Fix this by splitting the queuing part out of netfs_issue_read() into a new function, netfs_queue_read(), and calling it separately. The setting of NETFS_RREQ_ALL_QUEUED is then done by netfs_queue_read() whilst it is holding the spinlock (that's probably unnecessary, but shouldn't hurt). It might be better to set a flag on the final subreq, but this could be a problem if an error occurs and we can't queue it. Fixes: e2d46f2ec332 ("netfs: Change the read result collector to only use one work item") Reported-by: Ihor Solodrai Closes: https://lore.kernel.org/r/a7x33d4dnMdGTtRivptq6S1i8btK70SNBP2XyX_xwDAhLvgQoPox6FVBOkifq4eBinfFfbZlIkMZBe3QarlWTxoEtHZwJCZbNKtaqrR7PvI=@pm.me/ Signed-off-by: David Howells Tested-by: Ihor Solodrai cc: Eric Van Hensbergen cc: Latchesar Ionkov cc: Dominique Martinet cc: Christian Schoenebeck cc: Marc Dionne cc: Steve French cc: Paulo Alcantara cc: Jeff Layton cc: v9fs@lists.linux.dev cc: linux-cifs@vger.kernel.org cc: netfs@lists.linux.dev cc: linux-fsdevel@vger.kernel.org --- fs/netfs/buffered_read.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c index f761d44b3436..0d1b6d35ff3b 100644 --- a/fs/netfs/buffered_read.c +++ b/fs/netfs/buffered_read.c @@ -155,8 +155,9 @@ static void netfs_read_cache_to_pagecache(struct netfs_io_request *rreq, netfs_cache_read_terminated, subreq); } -static void netfs_issue_read(struct netfs_io_request *rreq, - struct netfs_io_subrequest *subreq) +static void netfs_queue_read(struct netfs_io_request *rreq, + struct netfs_io_subrequest *subreq, + bool last_subreq) { struct netfs_io_stream *stream = &rreq->io_streams[0]; @@ -177,8 +178,17 @@ static void netfs_issue_read(struct netfs_io_request *rreq, } } + if (last_subreq) { + smp_wmb(); /* Write lists before ALL_QUEUED. */ + set_bit(NETFS_RREQ_ALL_QUEUED, &rreq->flags); + } + spin_unlock(&rreq->lock); +} +static void netfs_issue_read(struct netfs_io_request *rreq, + struct netfs_io_subrequest *subreq) +{ switch (subreq->source) { case NETFS_DOWNLOAD_FROM_SERVER: rreq->netfs_ops->issue_read(subreq); @@ -293,11 +303,8 @@ static void netfs_read_to_pagecache(struct netfs_io_request *rreq) } size -= slice; start += slice; - if (size <= 0) { - smp_wmb(); /* Write lists before ALL_QUEUED. */ - set_bit(NETFS_RREQ_ALL_QUEUED, &rreq->flags); - } + netfs_queue_read(rreq, subreq, size <= 0); netfs_issue_read(rreq, subreq); cond_resched(); } while (size > 0);