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 94A39C61DA3 for ; Fri, 24 Feb 2023 09:04:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2B6EA6B0081; Fri, 24 Feb 2023 04:04:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 23E9D6B0082; Fri, 24 Feb 2023 04:04:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0E10A6B0083; Fri, 24 Feb 2023 04:04:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id ED0036B0081 for ; Fri, 24 Feb 2023 04:04:56 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id C1401121727 for ; Fri, 24 Feb 2023 09:04:56 +0000 (UTC) X-FDA: 80501600592.26.4038579 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf14.hostedemail.com (Postfix) with ESMTP id A39B5100002 for ; Fri, 24 Feb 2023 09:04:54 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=DhIbLSwi; spf=pass (imf14.hostedemail.com: domain of dhowells@redhat.com designates 170.10.133.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=1677229495; a=rsa-sha256; cv=none; b=b6vIhi/5b1Qj7TcKGQy2Y6MvWalLmYib5BxxOYbYxOAtl+loqSX1U1NBkP1jd96jgp+6/F ox6eXDG+ZxOTgGmjhr2Ix1vjxil2vLYYqu1hUVPFwtAx5X1fCirTq9Zwgdge+hfNtanEoD XLKaq9GP6AIJRJkcw5K7iyhsSy9cg1c= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=DhIbLSwi; spf=pass (imf14.hostedemail.com: domain of dhowells@redhat.com designates 170.10.133.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=1677229495; 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=6+0eD77pSON4ctupQ+Q2sz1nh9boGw6I9xn8BCA/uFs=; b=WZIvwZF4resEuoH7U/cPGgjN7W5LpAhaUiuW59L3itStTuetLkUGw2mva0qLWUDn6vGL4p Piqcyr/TvhSBzjMGqgv7simt0X2tOK3H3F4QwwK0OCpGkOFg/n1kVBLbxOk+n4kLMf30lO CcArKPFn/rbLE0/FlaszYfa7h9vWVCE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1677229493; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=6+0eD77pSON4ctupQ+Q2sz1nh9boGw6I9xn8BCA/uFs=; b=DhIbLSwi6+xWh387Z4e/cE3gPt9doE8x9xzp4zODW+0VepYEBZgrIF3rt/d5WfDIvvmkV6 1oaasEfF0DPKjW+DLfNxgpIVr/imb/wek1hof11W+fGyJWOPgyiOgvEdHfKhvzMGbNDpaS 8paTU0pKUYkDF9QUi9GLgG15UMmYBkI= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-244-GcPkmjlFMVeXtquMioak-g-1; Fri, 24 Feb 2023 04:04:51 -0500 X-MC-Unique: GcPkmjlFMVeXtquMioak-g-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3637E38012E8; Fri, 24 Feb 2023 09:04:51 +0000 (UTC) Received: from warthog.procyon.org.uk (unknown [10.33.36.18]) by smtp.corp.redhat.com (Postfix) with ESMTP id 43BF4C15BA0; Fri, 24 Feb 2023 09:04:49 +0000 (UTC) Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 From: David Howells In-Reply-To: References: <20230220135225.91b0f28344c01d5306c31230@linux-foundation.org> To: Linus Torvalds , Vishal Moola , Steve French Cc: dhowells@redhat.com, Andrew Morton , Jan Kara , Paulo Alcantara , Matthew Wilcox , Huang Ying , Baolin Wang , Xin Hao , linux-mm@kvack.org, mm-commits@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [GIT PULL] MM updates for 6.3-rc1 MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <2009824.1677229488.1@warthog.procyon.org.uk> Date: Fri, 24 Feb 2023 09:04:48 +0000 Message-ID: <2009825.1677229488@warthog.procyon.org.uk> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.8 X-Rspam-User: X-Rspamd-Queue-Id: A39B5100002 X-Rspamd-Server: rspam01 X-Stat-Signature: zgrkwddytthrcb3mbggjkxyiy3iucn86 X-HE-Tag: 1677229494-291759 X-HE-Meta: U2FsdGVkX18rUnrBaunjsMr2RO/m2TVanZ0ZUEj+3rxBXg27UUqHZGGKyweS1iyjfwcvANrGVYMYmeGcIcf4bnLbssIkFlK9H/2y4REI+LizBu18qi5ZRhABtdxz2YrZiFKIKnEWmPutssGUAbdNpk3RVIetFL/Gv0WfxbvwywsNSrIzgqKVVUVndcvhznLuJNi8KjjGii/9hFn4CjQOej2O/BomAzfmpN1+R96TqaQrlIplZA5DU0eyKjCS4ti3cNoHnbAkyQdpDczgx4mWxrLxD3lyldf9T3URA74PTd5lylZG/ImXx0Xx30iR/BZQ4cpnq24CPvgM+4QGbZapaxFLkvmuokeCCkAM+70RdTSWOK049o05lVwtE93kCbvLjPTMNZ/epaNhUa+c+Z9btWX+Zeq3dk6eGMnlBxhAYnVYvG4Tu8wcYmmMwg9AFhxprObO2fgyPOI2Ulm9nx2I8i5RPU0FYZVkkF9dYJw21N7q/UKHMepR3uubjxmSMqBXNg0WYP8YyMc4TV8156Su4W4pXn7ApZrRmghTe51FPUiJ342FnJeYPSXEcRI7r38MbybtJSQ00s/XoCuRKQ8iJZYFerW+qMou09y1+WQPV7ZZbJ0SLowUyZgwEimXBSRYIbLBgUdSfmUIc9DeIKxuNwc3JEKV28T3J+tm13xtrNYPg3xO4YvFWCiOanNWBTBp01dFzM55Rg0QaZQAGJfc/XLfZsQ/1QmcNgT6ax1ykAuP9gdxP5+lHC4UF+sVLl+CsVLeDnLU8rfAUN3qv5gGN9uOT0/OD+6Jq+dD1r0Fg48AQyrWd2JQv6EbEb90k9TUtacU/4WoY2QWXfgCh8BpSwMwfE7g/FFeA/5mq/4hnS5EAzVjoMTc+0ATc4ch05SKuKJ9AHPPMA1kbAoa4vFn5UuN2PWZNF8lTyaI2uVLGS43NC05BP85UMxuHL8ZMWVZFsNXjO7dJkHCoCGkSLo qI6+dMJQ kWOb6OrIMRh8B+A1Mi0IIepQ1zYUM3qRm9eaiFq6vkZJ/rvnAvdWtSsctjC4/TYiDgTc+cJx70XQvHrx6mjyN0jbZuGJNHgPxF3tCuSBtqZq5p8KFritsiINz8zdv7mS7Mb1oEDsuR8eoDPkswzjykKD5JMylilDbhgyRr40uBwEufi7Jky83JK+CFTz14dBeENYcvmfHZf8O/O8AfqZXfnUKyyzbxKzairsLtQ8YgJbwvEAXUJcvHUn+JCOALiUeMpu0vF0jah9pYvfCOX5gcePT0mhR8Q1dMmqeT6VVsITKzAUj1fQk/oTpug== 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: Linus Torvalds wrote: > Yes, I saw David Howells resolution suggestion. I think that one > was buggy. It would wait for a page under writeback, and then go on to > the *next* one without writing it back. I don't thin kthat was right. You're right. Vishal's patch introduced it into afs and I copied it across and didn't notice it either then or on review of Vishal's patch. He inserted the extra for-loop as he's now extracting a batch, but kept the continue that used to repeat the extraction - except now it continues the wrong loop. So afs will need fixing too. The simplest ways I think are to just decrement the loop counter before continuing or to stick a goto in back to the beginning of the loop (which is what you did in cifs). But I'm not sure that's the correct thing to do. The previous code dropped the found folio and then repeated the search in case the folio got truncated, migrated or punched. I suspect that's probably what we should do. Also, thinking about it again, I'm not sure whether fetching a batch with filemap_get_folios_tag() like this in {afs,cifs}_writepages_region() is necessarily the right thing to do. There are three cases I'm thinking of: (1) A single folio is returned. This is trivial. (2) A run of contiguous folios are returned - {afs,cifs}_extend_writeback() is likely to write them back, in which case the batch is probably not useful. Note that *_extend_writeback() walks the xarray directly itself as it wants contiguous folios and doesn't want to extract any folio it's not going to use. (3) A list of scattered folios is returned. Granted this is more efficient if nothing else interferes - but there could be other writes in the gaps that we then skip over, other flushes that render some of our list clean or page invalidations. This is a change in behaviour, but I'm not sure that matters too much since a flush/sync can only be expected to write back what's modified at the time it is initiated. Further, processing each entry in the list is potentially very slow because we're doing a write across the network for each one (cifs might bump this into the background, but it might also have to (re)open a file handle on the server and wait for credits first to even begin the transaction). Which means all of the folios in the batch may then get pinned for a long period of time - up to 14x for the last folio in the batch - which could prevent things like page migration. Further, we might not get to write out all the folios in the batch as *_extend_writeback() might hit the wbc limit first. > That said, I'm not at all convinced my version is right either. I > can't test it, and that means I probably messed up. It looked sane to > me when I did it, and it builds cleanly, but I honestly doubt myself. It doesn't seem to work. A write seems to end in lots of: CIFS: VFS: No writable handle in writepages rc=-9 being emitted. I'll poke further into it - there's always the possibility that some other patch is interfering. David