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 67CECD32D97 for ; Tue, 12 Nov 2024 19:38:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B9F506B00CB; Tue, 12 Nov 2024 14:38:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B29A66B00CC; Tue, 12 Nov 2024 14:38:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9C64C6B00CE; Tue, 12 Nov 2024 14:38:09 -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 7864D6B00CB for ; Tue, 12 Nov 2024 14:38:09 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 0857A1605BA for ; Tue, 12 Nov 2024 19:38:09 +0000 (UTC) X-FDA: 82778453478.29.DBA6651 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf24.hostedemail.com (Postfix) with ESMTP id 9CD1718000D for ; Tue, 12 Nov 2024 19:38:01 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=IbABtO5N; spf=pass (imf24.hostedemail.com: domain of bfoster@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bfoster@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=1731440144; 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=+pEWGwDV42qZ605Ox/qRGT5wCc1jBMka21/Df9UKjYs=; b=7+bLy/JR54V9sH695xaQiUOrPI5mLN2aB7GBHIfomttOju79yHeL+G6g2EMZhWY9LX0eUf fsWgvWv34IR7pi9hkb5dlcgIEgSI/T567NFFfAw/+uuf8V5/NPh8b0ViWh7szvBDssiEFu rYN0bnmpBnYCAzEOnS3NzQyw7L3qh9g= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1731440144; a=rsa-sha256; cv=none; b=qnWIb7557D/lA17W/j4yDh1hYKEUs3Brv0qtznqAVfAxSi3S3OYRpjmg1sLgE2HOO5zrZb tDdJ/gZG5hQMOaz2IESCP5dKvZg/jiJbHpZieqf3QwiCEnWOhreRdSOgx6b5nKv8DhQhhl hfYlSsPvDBf3nyGe14KaT43/xsowp6Q= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=IbABtO5N; spf=pass (imf24.hostedemail.com: domain of bfoster@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (policy=none) header.from=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1731440286; 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=+pEWGwDV42qZ605Ox/qRGT5wCc1jBMka21/Df9UKjYs=; b=IbABtO5NqDIwWKUbKUAJDS/42cUiCWJr9uMtwsfuoo/rB3s8RFKGCCbBfwXicfzjRLIOo7 /4pHHohf2I6iD9ylCY2dKe8k+K2mlpwUshTyGyVzPOZbHok3s3SwjBqgLLNr5JORjc8kKm rjyU9n9D9D1tfSjFU5xkzrVSkmR1ybY= Received: from mx-prod-mc-05.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-113-sQXeUOb5NqWfBZxRfASG6Q-1; Tue, 12 Nov 2024 14:38:04 -0500 X-MC-Unique: sQXeUOb5NqWfBZxRfASG6Q-1 X-Mimecast-MFC-AGG-ID: sQXeUOb5NqWfBZxRfASG6Q Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (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-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 64D541955F2C; Tue, 12 Nov 2024 19:38:02 +0000 (UTC) Received: from bfoster (unknown [10.22.80.120]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 8E80D19560A3; Tue, 12 Nov 2024 19:37:59 +0000 (UTC) Date: Tue, 12 Nov 2024 14:39:32 -0500 From: Brian Foster To: Jens Axboe Cc: Christoph Hellwig , "Kirill A. Shutemov" , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, hannes@cmpxchg.org, clm@meta.com, linux-kernel@vger.kernel.org, willy@infradead.org Subject: Re: [PATCH 08/15] mm/filemap: add read support for RWF_UNCACHED Message-ID: References: <04fd04b3-c19e-4192-b386-0487ab090417@kernel.dk> <31db6462-83d1-48b6-99b9-da38c399c767@kernel.dk> <3da73668-a954-47b9-b66d-bb2e719f5590@kernel.dk> <7a4ef71f-905e-4f2a-b3d2-8fd939c5a865@kernel.dk> <3f378e51-87e7-499e-a9fb-4810ca760d2b@kernel.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 9CD1718000D X-Stat-Signature: se5my3q9k1pe3rgosxgmn5yn1saszdoz X-HE-Tag: 1731440281-160681 X-HE-Meta: U2FsdGVkX19pHhLq9tpa0rqHZuqDyrgkU3KtdxE/IkfVpusiBhYFORDj6BLBnCQYKRgJ+G9U0TjnaHVEMpeOHdA9LDheyDdhJX0iQVAT3jNV/jSwIFuFUGaXovrRtSQytCDzuNveKLq1CPMZOzaVCXjyi/sTUu/5fAoTbRKzSTKQdlXbWNdpdcWOHTuvjJnlNCBORT/DiTN3WlASs50Y7aPZZiMggImAiUFAYlHwhlnv7c62XpZFHqf8h5Yc/sy6JBYh2xkPVVmJo3dSeqNrBiFh22NghCFLD/70F1q76LFUYPSDdFIwK85Z1sXkK4sC+2rvObRVB05SQGSTxJZledHft0UhcgS3SWxCIusCVVOWeNjazA8l28VBe1Brd8SwBNjpgySQDTyx4Tm3iY1WVnpT7TaNyJRKNdrnhNGvxt+3MHLjwHB4Cm9U/zsTTkPnPPT5wqvzmlpbnlKwOnbdxz2CPmLoHXT1bFNlaD+BUhjchLByRylgB/ZEy+7ZHU5M5a694G7cAK1cJke50BQrQcuUMf87smXX3v4f4EX0JSLfkRdmvfc+Gnr1yzVB/1M2Kasd1q4eYSUi81/RWcb1xsP0CaR2DeVqMkamvz6owYjUs7DSEBlc3CckpPU/w/Ll95gjn9G4L9tvLU2R+Q7cOYnN2nsKkP00fdicMtRCelIYRWCeIOVNXAadcRRPDmmyWgsHsnu3QsiqistmOAdU6SB1ErShYsTOIk8lphqJsOGhoAR0lgRUGABgS+knxWUgUjycMqn15D910uPhY2DEizhQ3kOUSeS4KsnbxQvsq62CemfzXvXW8X7TTlHNeTRE49a2ZEGO9orhNQrj4ZhIKagUI4gdUNEeKSnYt2erVOq0HKeghacAmxhHSOTbjYYesDMREAoFU9lejdzPDb9a9Ff94Lbsvfl91j017uBH8k6wb2FiYQfE4EVVOlfWsQlWDpzbuWwvXyEbd7AiS/o kgykopXW eFuS9kVJX/GaAMGNcpnbWCb3KhhKdq5Sy4fDcWBCWSpU0/5QjR/GvNT9rFPbC8rtoEFnScD4HMOak0fpP3ekcM5+LEh9oG7U9fVJjEykKSwNIMKDumcizm+qdLVrrzxmLxe+VUDjOsumcYBOYZecSt9RCQFTvWk4yO4yRi1lImkm8bFKtP+flmmDKGjL1/9WxYE/hGiUhcZY8X4irL3bD4IlC+7jl/6wdulKqLckRtnS7Wm4oSaE8LrC5fzbLftHd1K5UmpfVbWmnoQM5t4d8/qSyHXt56yXKN9KO73qvvps0mjCTJXdaBxY4jKa03MrQkE206IveJrlRMAatgj12zJ2X6v4iUpLyvUXif1bRzsVijt/lpy+l6veNEVxHzjcnOQjns/BvyBuE4L8FtjAfUYKzCg== 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 Tue, Nov 12, 2024 at 12:08:45PM -0700, Jens Axboe wrote: > On 11/12/24 11:44 AM, Brian Foster wrote: > > On Tue, Nov 12, 2024 at 10:19:02AM -0700, Jens Axboe wrote: > >> On 11/12/24 10:06 AM, Jens Axboe wrote: > >>> On 11/12/24 9:39 AM, Brian Foster wrote: > >>>> On Tue, Nov 12, 2024 at 08:14:28AM -0700, Jens Axboe wrote: > >>>>> On 11/11/24 10:13 PM, Christoph Hellwig wrote: > >>>>>> On Mon, Nov 11, 2024 at 04:42:25PM -0700, Jens Axboe wrote: > >>>>>>> Here's the slightly cleaned up version, this is the one I ran testing > >>>>>>> with. > >>>>>> > >>>>>> Looks reasonable to me, but you probably get better reviews on the > >>>>>> fstests lists. > >>>>> > >>>>> I'll send it out once this patchset is a bit closer to integration, > >>>>> there's the usual chicken and egg situation with it. For now, it's quite > >>>>> handy for my testing, found a few issues with this version. So thanks > >>>>> for the suggestion, sure beats writing more of your own test cases :-) > >>>>> > >>>> > >>>> fsx support is probably a good idea as well. It's similar in idea to > >>>> fsstress, but bashes the same file with mixed operations and includes > >>>> data integrity validation checks as well. It's pretty useful for > >>>> uncovering subtle corner case issues or bad interactions.. > >>> > >>> Indeed, I did that too. Re-running xfstests right now with that too. > >> > >> Here's what I'm running right now, fwiw. It adds RWF_UNCACHED support > >> for both the sync read/write and io_uring paths. > >> > > > > Nice, thanks. Looks reasonable to me at first glance. A few randomish > > comments inlined below. > > > > BTW, I should have also mentioned that fsx is also useful for longer > > soak testing. I.e., fstests will provide a decent amount of coverage as > > is via the various preexisting tests, but I'll occasionally run fsx > > directly and let it run overnight or something to get the op count at > > least up in the 100 millions or so to have a little more confidence > > there isn't some rare/subtle bug lurking. That might be helpful with > > something like this. JFYI. > > Good suggestion, I can leave it running overnight here as well. Since > I'm not super familiar with it, what would be a good set of parameters > to run it with? > Most things are on by default, so I'd probably just go with that. -p is useful to get occasional status output on how many operations have completed and you could consider increasing the max file size with -l, but usually I don't use more than a few MB or so if I increase it at all. Random other thought: I also wonder if uncached I/O should be an exclusive mode more similar to like how O_DIRECT or AIO is implemented. But I dunno, maybe it doesn't matter that much (or maybe others will have opinions on the fstests list). Brian > >> #define READ 0 > >> #define WRITE 1 > >> -#define fsxread(a,b,c,d) fsx_rw(READ, a,b,c,d) > >> -#define fsxwrite(a,b,c,d) fsx_rw(WRITE, a,b,c,d) > >> +#define fsxread(a,b,c,d,f) fsx_rw(READ, a,b,c,d,f) > >> +#define fsxwrite(a,b,c,d,f) fsx_rw(WRITE, a,b,c,d,f) > >> > > > > My pattern recognition brain wants to see an 'e' here. ;) > > This is a "check if reviewer has actually looked at it" check ;-) > > >> @@ -266,7 +273,9 @@ prterr(const char *prefix) > >> > >> static const char *op_names[] = { > >> [OP_READ] = "read", > >> + [OP_READ_UNCACHED] = "read_uncached", > >> [OP_WRITE] = "write", > >> + [OP_WRITE_UNCACHED] = "write_uncached", > >> [OP_MAPREAD] = "mapread", > >> [OP_MAPWRITE] = "mapwrite", > >> [OP_TRUNCATE] = "truncate", > >> @@ -393,12 +402,14 @@ logdump(void) > >> prt("\t******WWWW"); > >> break; > >> case OP_READ: > >> + case OP_READ_UNCACHED: > >> prt("READ 0x%x thru 0x%x\t(0x%x bytes)", > >> lp->args[0], lp->args[0] + lp->args[1] - 1, > >> lp->args[1]); > >> if (overlap) > >> prt("\t***RRRR***"); > >> break; > >> + case OP_WRITE_UNCACHED: > >> case OP_WRITE: > >> prt("WRITE 0x%x thru 0x%x\t(0x%x bytes)", > >> lp->args[0], lp->args[0] + lp->args[1] - 1, > >> @@ -784,9 +795,8 @@ doflush(unsigned offset, unsigned size) > >> } > >> > >> void > >> -doread(unsigned offset, unsigned size) > >> +__doread(unsigned offset, unsigned size, int flags) > >> { > >> - off_t ret; > >> unsigned iret; > >> > >> offset -= offset % readbdy; > >> @@ -818,23 +828,39 @@ doread(unsigned offset, unsigned size) > >> (monitorend == -1 || offset <= monitorend)))))) > >> prt("%lld read\t0x%x thru\t0x%x\t(0x%x bytes)\n", testcalls, > >> offset, offset + size - 1, size); > >> - ret = lseek(fd, (off_t)offset, SEEK_SET); > >> - if (ret == (off_t)-1) { > >> - prterr("doread: lseek"); > >> - report_failure(140); > >> - } > >> - iret = fsxread(fd, temp_buf, size, offset); > >> + iret = fsxread(fd, temp_buf, size, offset, flags); > >> if (iret != size) { > >> - if (iret == -1) > >> - prterr("doread: read"); > >> - else > >> + if (iret == -1) { > >> + if (errno == EOPNOTSUPP && flags & RWF_UNCACHED) { > >> + rwf_uncached = 1; > > > > I assume you meant rwf_uncached = 0 here? > > Indeed, good catch. Haven't tested this on a kernel without RWF_UNCACHED > yet... > > > If so, check out test_fallocate() and friends to see how various > > operations are tested for support before the test starts. Following that > > might clean things up a bit. > > Sure, I can do something like that instead. fsx looks pretty old school > in its design, was not expecting a static (and single) fd. But since we > have that, we can do the probe and check. Just a basic read would be > enough, with RWF_UNCACHED set. > > > Also it's useful to have a CLI option to enable/disable individual > > features. That tends to be helpful to narrow things down when it does > > happen to explode and you want to narrow down the cause. > > I can add a -U for "do not use uncached". > > -- > Jens Axboe >