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 7792DC48260 for ; Fri, 16 Feb 2024 07:40:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DEC558D0006; Fri, 16 Feb 2024 02:40:47 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D9CE48D0001; Fri, 16 Feb 2024 02:40:47 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C64268D0006; Fri, 16 Feb 2024 02:40:47 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id B6D5C8D0001 for ; Fri, 16 Feb 2024 02:40:47 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 4E255C0239 for ; Fri, 16 Feb 2024 07:40:47 +0000 (UTC) X-FDA: 81796870134.02.696A806 Received: from mail-yw1-f170.google.com (mail-yw1-f170.google.com [209.85.128.170]) by imf03.hostedemail.com (Postfix) with ESMTP id A936820013 for ; Fri, 16 Feb 2024 07:40:45 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=y0F3Ub3e; spf=pass (imf03.hostedemail.com: domain of hughd@google.com designates 209.85.128.170 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708069245; 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=ovLXWtZtTydmOuJYQBdia+RLmYItmrIBSwt8zg/YWmk=; b=N4HFOoAfdC5Zk6pfhl216nIFVC8Y/AUa9HXzY+ZCro1GQ6rGl49CZhKtrZHXg8a++E5YCW dox+8ivG3Gmm/5fSUKkWHEjl3MYDm4SYPiDSvA6gPV6+MV8wnnLxKocQIWaH1Mq0+hWszj P/C7k49wCAuDbTdsaglv4JJ4pHZZ+qQ= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=y0F3Ub3e; spf=pass (imf03.hostedemail.com: domain of hughd@google.com designates 209.85.128.170 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708069245; a=rsa-sha256; cv=none; b=2eDmYkaEzSgZoLCLSjHy++6hTI3Uh2o7prSC+GQ3xyPsOPakagsudSTXgL2AXxQlnz5XwA bATaeoEfU7OqWz1a2HnvNezOt9G9ftS4q/EIqCCRrarxyeFmDetaPw7MQejx3bOSOSXwAm Y7a8rhApVluyWM2Kpo6Djm7Xv4FfS4U= Received: by mail-yw1-f170.google.com with SMTP id 00721157ae682-607fe8cc6d8so1477307b3.1 for ; Thu, 15 Feb 2024 23:40:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1708069245; x=1708674045; darn=kvack.org; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=ovLXWtZtTydmOuJYQBdia+RLmYItmrIBSwt8zg/YWmk=; b=y0F3Ub3eYySkFb/wq6Oy30U9x6JrhEbN2Tu6PagzddUBs2oo3uJaB8mgN9Y8jA9/G0 BwRXDIhsRUlM6lnmAJ4p/Aa7XmuHwvnAymLAQJ6fU/XFQ/npaM/7j3dUAbjF1r5uQxIG av3Df4S9NEnXOmBvsbsrv0r5xzG2YwCWyOWTLUlt7WP8KVHXJOIDa7jvrrSgcGbqrOs2 QWKxBQQw1emxxtC26Vvi/6nIoN1VzGi6S3jXuXIvAzoICSLWdX9j1xyk24b+vhbiJnps A6hIZ8HhakUUJzW554bjTLcnHcjojUtqx2Nr9I+WLeIRn2tg5BCZYCZsob+gGt62gS0X Fh0w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708069245; x=1708674045; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ovLXWtZtTydmOuJYQBdia+RLmYItmrIBSwt8zg/YWmk=; b=rS2ecFmX2jQwiSnCb78NfUSjB1ZIVCzhjq1kIPVUJMWfEpkmL0jNcXmjvKJc2+/PNs Vt2R5Kgo3uU2rZXG1vOh67RQ+Ur8iwBozjE1fXcognOPrBuG+tI+rDUEz4x1Qx0uWBCe o+C0FRvrZOB0XKwrjfi5anSt5rHoVykD0T48PMWgrhs6Mgl3Ebn1+Dzf3/UaGTEB6ZI7 8D0Fe4iATHf1jUxk2CKxh3wNJd4Yg12cQVu5I8p1mPP03qf5ltOjiA8u9hqEgauHivY+ FUwBsfACKetmys8j/cORe7wezR3f5K9zBjllzlHWzUP6Nq/UeKvrcaznaHeY7kPM7PUq 9FRw== X-Forwarded-Encrypted: i=1; AJvYcCUQ/QvXZNxNp4R9DcwRGss0vXqViK30mx9jMYqZTsvJg0hvBwqIk9wNwbaQOuof7Hf4eRN67VUxamprWDGreRYmGns= X-Gm-Message-State: AOJu0YxHes8Dt8blDx6QIpjnkknc79ZHkvfiWNw74avBQ3RDP79FqwEE VhB6wnleHxkboWc3pdJos7F8ijNCAfPvzU09oGJezwxQzjMnFux2TN7JdBOuuQ== X-Google-Smtp-Source: AGHT+IFRP/V0/OL+0SVHgVYfcJFzbqM573teIpsV9n40jRjl5FF1m2E4UHS7hsCWKPYXdbboHzRICA== X-Received: by 2002:a0d:d511:0:b0:607:7d11:ea2 with SMTP id x17-20020a0dd511000000b006077d110ea2mr4511710ywd.37.1708069244342; Thu, 15 Feb 2024 23:40:44 -0800 (PST) Received: from darker.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id t4-20020a815f04000000b00607b37e2c74sm245013ywb.118.2024.02.15.23.40.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Feb 2024 23:40:43 -0800 (PST) Date: Thu, 15 Feb 2024 23:40:41 -0800 (PST) From: Hugh Dickins To: Christoph Hellwig cc: Chandan Babu R , "Darrick J. Wong" , Hugh Dickins , Andrew Morton , Matthew Wilcox , linux-xfs@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 14/20] xfs: use shmem_get_folio in xfile_obj_store In-Reply-To: <20240129143502.189370-15-hch@lst.de> Message-ID: <8eba6e1d-3e93-9586-17f3-86dcf25f5ae4@google.com> References: <20240129143502.189370-1-hch@lst.de> <20240129143502.189370-15-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspamd-Queue-Id: A936820013 X-Rspam-User: X-Stat-Signature: f9prhsea31ogeqd6k5cot3jmnwpkn7fw X-Rspamd-Server: rspam01 X-HE-Tag: 1708069245-618733 X-HE-Meta: U2FsdGVkX1+K94CZQegIW8IJuFIVhQ6rcbHRestGhlmkVyj0Kqf0yCg+mL6cRjaCSpUUIm9iK+WTaud4CRjIUa792dwWa8ealV3ecW23YwedgV1dRiXpjles3ujZS1H8fxD4eRBQjiGBloIwoDnc1izcU9M77zOscnZ/IzIhLSHFA03OnyjI/hAidFj3FGcZ/IauJ9WPm8c72L7jgkKhxf8zj7Ue8bIHBLWA+gdmjcoHP1NuylVAXSsMPrAmA1NAT/X/lvJHh91mK7acgtayhwxGBgpyFDrAtGgOqBkVMz33UkkZiVUwUhw0V7ncXCMfHeCO5A93vh8hHiIXXp0tR4y4aNAbBOug+r1r/NncwPK7SfpZZ6RA1Hf19+tQy+LOI+SAqyd6GNxJd+MH47lviOctH9JEyjx3RWMELw6IZnvpA5YZ0V5i8sjtiSKHz0SlJMiACwP0rLG78qAL/wFIltYnAa0VIHSmnOsNOaxDFu3zqEZ7v9OH0puZomS5rArOIO3bWvvokbIjxT5OF7CNiZXz1aAwKjAkszpbVbNRgM44rOmlMKKg1QhpUIRwO1zobbD7ie5+yONPdc/V6461hDLLnltCzES+5J7ElQIJQoAFmvMMWEYAAnMlHChrP/jo5XkXZl7ehpVOum94W//B50ZGt+WvHd3WSowotsiz8b8Hscjh7jyjCESP5O+HssncVgnF9svTHKVWVyJ9wfVxp3IZwbsBKHUUmcSncDeFAJPddulBeVUwtc9/yc9Nz8F1553IS1dUlObhEeGXayUpAEzSepxZK9iiOcyEdFdc5+sSpIZBoRd1C9tgxAUzjo+NLG/32Sw9S0jag9j4/K2fZmWio1kUK0xb581/gUiv7mVgTL4RcADiyohwTKI23DmWirNMl83yRe9MSq68DF0WSmuYUcEDBB5vVMnO0bgGKZR1XLQJyvJNKcxjWE15icjlV1i6ZrN9Qq+N+9aTuL5 Hs97arQ0 IQpcY6Vg2ZSxa606vsf2ZO5SuE8DqcRDDq9xBa6SbxdlD2PuxsEeqDq4gxW/z0E73JL10HHEJABxY/adYm9DlnvGyD/cbn6yehLOGMC44QEfYSPvpKXKKWyo8FyNVAc5adjQt 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 Mon, 29 Jan 2024, Christoph Hellwig wrote: > Switch to using shmem_get_folio and manually dirtying the page instead > of abusing aops->write_begin and aops->write_end in xfile_get_page. > > This simplifies the code by not doing indirect calls of not actually > exported interfaces that don't really fit the use case very well, and > happens to get us large folio support for free. > > Signed-off-by: Christoph Hellwig > Reviewed-by: Darrick J. Wong > --- > fs/xfs/scrub/xfile.c | 73 ++++++++++++++++---------------------------- > 1 file changed, 27 insertions(+), 46 deletions(-) > > diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c > index a669ebbbc02d1d..2b4819902b4cc3 100644 > --- a/fs/xfs/scrub/xfile.c > +++ b/fs/xfs/scrub/xfile.c > @@ -183,11 +183,7 @@ xfile_store( > loff_t pos) > { > struct inode *inode = file_inode(xf->file); > - struct address_space *mapping = inode->i_mapping; > - const struct address_space_operations *aops = mapping->a_ops; > - struct page *page = NULL; > unsigned int pflags; > - int error = 0; > > if (count > MAX_RW_COUNT) > return -ENOMEM; > @@ -196,60 +192,45 @@ xfile_store( > > trace_xfile_store(xf, pos, count); > > + /* > + * Increase the file size first so that shmem_get_folio(..., SGP_CACHE), > + * actually allocates a folio instead of erroring out. > + */ > + if (pos + count > i_size_read(inode)) > + i_size_write(inode, pos + count); > + > pflags = memalloc_nofs_save(); > while (count > 0) { > - void *fsdata = NULL; > - void *p, *kaddr; > + struct folio *folio; > unsigned int len; > - int ret; > + unsigned int offset; > > - len = min_t(ssize_t, count, PAGE_SIZE - offset_in_page(pos)); > - > - /* > - * We call write_begin directly here to avoid all the freezer > - * protection lock-taking that happens in the normal path. > - * shmem doesn't support fs freeze, but lockdep doesn't know > - * that and will trip over that. > - */ > - error = aops->write_begin(NULL, mapping, pos, len, &page, > - &fsdata); > - if (error) { > - error = -ENOMEM; > + if (shmem_get_folio(inode, pos >> PAGE_SHIFT, &folio, > + SGP_CACHE) < 0)o SGP_CACHE is the safest choice, yes. It will tend to do an unnecessary clear_highpage() which you immediately overwrite with the actual data; but saves calculating exactly what needs to be zeroed above and below the data - not worth bothering with, unless shaving off CPU cycles. > break; > - } > - > - /* > - * xfile pages must never be mapped into userspace, so we skip > - * the dcache flush. If the page is not uptodate, zero it > - * before writing data. > - */ > - kaddr = page_address(page); > - if (!PageUptodate(page)) { > - memset(kaddr, 0, PAGE_SIZE); > - SetPageUptodate(page); > - } > - p = kaddr + offset_in_page(pos); > - memcpy(p, buf, len); > - > - ret = aops->write_end(NULL, mapping, pos, len, len, page, > - fsdata); > - if (ret < 0) { > - error = -ENOMEM; > + if (filemap_check_wb_err(inode->i_mapping, 0)) { Hah. I was sceptical what that could ever achieve on shmem (the "wb" is misleading); but it's an ingenious suggestion from Matthew, to avoid our current horrid folio+page HWPoison handling. I followed it up a bit, and it does look as if this filemap_check_wb_err() technique should work for that; but it's not something I've tried at all. And that's all I've got to say on the series (I read on, but certainly did not delve into the folio sorting stuff): looks good, but the VM_NORESERVE question probably needs attention (and that docbook comment to mention "locked"). XFS tree stil seems to me the right home for it all. Hugh > + folio_unlock(folio); > + folio_put(folio); > break; > } > > - if (ret != len) { > - error = -ENOMEM; > - break; > - } > + offset = offset_in_folio(folio, pos); > + len = min_t(ssize_t, count, folio_size(folio) - offset); > + memcpy(folio_address(folio) + offset, buf, len); > + > + folio_mark_dirty(folio); > + folio_unlock(folio); > + folio_put(folio); > > - count -= ret; > - pos += ret; > - buf += ret; > + count -= len; > + pos += len; > + buf += len; > } > memalloc_nofs_restore(pflags); > > - return error; > + if (count) > + return -ENOMEM; > + return 0; > } > > /* Find the next written area in the xfile data for a given offset. */ > -- > 2.39.2 > >