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 E2738D32D81 for ; Tue, 12 Nov 2024 19:08:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 065266B00B8; Tue, 12 Nov 2024 14:08:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id F30276B00B9; Tue, 12 Nov 2024 14:08:50 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DA80E6B00BA; Tue, 12 Nov 2024 14:08:50 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id B92A86B00B8 for ; Tue, 12 Nov 2024 14:08:50 -0500 (EST) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 5D7B8C0531 for ; Tue, 12 Nov 2024 19:08:50 +0000 (UTC) X-FDA: 82778379600.28.A5BDDBF Received: from mail-oi1-f180.google.com (mail-oi1-f180.google.com [209.85.167.180]) by imf06.hostedemail.com (Postfix) with ESMTP id 6A47418000B for ; Tue, 12 Nov 2024 19:08:18 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=kernel-dk.20230601.gappssmtp.com header.s=20230601 header.b=wFO+DAZr; spf=pass (imf06.hostedemail.com: domain of axboe@kernel.dk designates 209.85.167.180 as permitted sender) smtp.mailfrom=axboe@kernel.dk; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1731438294; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=D2L7Qc7cWvztIAJNQU15qiNixT/BI8zR+cXMMeIYHYo=; b=Zl5EjGYkk95WmNZE5VyMpoUSfJSeaHfsBB2M4Wl0obtrrY6eTTrs6Gax70Ngmt68k5uy0Y Wjmb0fmCPtAFfA3wZOyZXPP2A+D2aH8L35bx3zAyGujuQo6RyJLa5BoTosC1QP1PUTZh7l /Jdx272eGFCDwWDXiY6G4FvIvBZAfTw= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=kernel-dk.20230601.gappssmtp.com header.s=20230601 header.b=wFO+DAZr; spf=pass (imf06.hostedemail.com: domain of axboe@kernel.dk designates 209.85.167.180 as permitted sender) smtp.mailfrom=axboe@kernel.dk; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1731438294; a=rsa-sha256; cv=none; b=KRfNOXd/ESpc94cBrkrG0O+oGUbMhwAmpyUHMWbNL65dXW6NxiB6laaPW3n3LgXrY7yv8n qJI5iFB40HvQTGJo6LokZIl6uRC+iC1v7vjyHxTCcH6Fgo4kem80F3ISASdjgoah3WWXXG wGMokOmABzGDM2wkE78YWrOThb3yOek= Received: by mail-oi1-f180.google.com with SMTP id 5614622812f47-3e5f533e1c2so3786915b6e.3 for ; Tue, 12 Nov 2024 11:08:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20230601.gappssmtp.com; s=20230601; t=1731438527; x=1732043327; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=D2L7Qc7cWvztIAJNQU15qiNixT/BI8zR+cXMMeIYHYo=; b=wFO+DAZrOBg0pRBdsGI2moc9o8wcOkl06yCIoNw3DhTRpsInz/8r55ZgtYBRyxpZjs MDSJACxRmPfdVENc5cs20nXu8whRTDZlfZV0ZLXIhPt8q6erX0QuwL45/UUkFpBLjoMc n67t7Ov1xz25fqy0zqb14EAlIF3iz90Z1E9+ZC79fULeBfWX2rN4KOcS9AOJKmlRv3hK V0+9XvvylRxh3O8OF82kMQtodu7DE9o76ubpYPDS/UPnLKSLNjY73141YsRwfk9n+ebo bo9ypN0cR2P2OOt7uoT6z2bGzxkEVII5Hl3szgynyGcM6tYjqqVski/HZbFpnfBzm4/r yMHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731438527; x=1732043327; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=D2L7Qc7cWvztIAJNQU15qiNixT/BI8zR+cXMMeIYHYo=; b=wUsz9vsAOes6cyW3PKwm9VBMjYXKihwZY3tMDuPGRwwc/l4oK6E46LZGb/FblkRvyc ZUy4U7dm0ocS+tLIQqII2eoudC1ShkLqNjaJM6DZSnMsLzBLV9cr/ncXD2VWWl2Czjw8 ryvH4g9q9Tv7C8cSXpyH2+o0OyssUkCDNPmyPefo6OarMOq4AYsRJt0q6yLXxGvPKb1Q 8QFZ3yO1AZW51w5xiVjnNrggm1rn1Oo8TuqoEJl3eaP2doxJ8rhVu5HjJEl4BD786Emd Fa1eCCFAN/NCtBeAyGkk9tFN5esyavY5E0CgSRQeKrnRnmp6BUbZHW/KZBm/2l4ex2KC gSkg== X-Forwarded-Encrypted: i=1; AJvYcCV4LY7hqPsmVPmtCTuJEU780KOlcl5vZmykhOB3GCEP+Xy1s2jLFYOsLH5mTHAriyZcjgYDox76Fg==@kvack.org X-Gm-Message-State: AOJu0Yzjbfo/oaKYz13S0gAp/NiViJ+v5brAIMxAYHehJZBdPAcZRYwz yJX6wo4LjnuPShYAeua7f+h9cbk6Mt/Mv6FbNpT+2Pjwl+opPprF+F3zB4oUSBE= X-Google-Smtp-Source: AGHT+IHoD/uu9DK4NnbrmBtV1ufstZJ55RPP5+LQ3aAZ0f5dnSCso4iUv4jnAvRgbKIUMZBidnJMoA== X-Received: by 2002:a05:6808:1790:b0:3e6:263b:9108 with SMTP id 5614622812f47-3e7946adb92mr15187907b6e.22.1731438527201; Tue, 12 Nov 2024 11:08:47 -0800 (PST) Received: from [192.168.1.116] ([96.43.243.2]) by smtp.gmail.com with ESMTPSA id 5614622812f47-3e7b0955af5sm22182b6e.4.2024.11.12.11.08.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 12 Nov 2024 11:08:46 -0800 (PST) Message-ID: Date: Tue, 12 Nov 2024 12:08:45 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 08/15] mm/filemap: add read support for RWF_UNCACHED To: Brian Foster 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 References: <221590fa-b230-426a-a8ec-7f18b74044b8@kernel.dk> <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> Content-Language: en-US From: Jens Axboe In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 6A47418000B X-Stat-Signature: o48yo8se6fpfmxsmgtgqgztnoog65637 X-Rspam-User: X-HE-Tag: 1731438498-41704 X-HE-Meta: U2FsdGVkX1/OWMdWMYRlmBSMNJ/vNpsQt+gzI1Qdex7wfYi6hQMRyppTCCk2rQVdVQOHdv5krj0oOxrGCorNwZtml8SnVMdZAWXN1j9EoHHfcVwKo3yF0+mZOmvcj+c8S8m3Q6Z65gS6keIw4MHW3HO6POSUITiKFz6OuDFUfyhW7TaoeH2BcVu8t1A7hcoYnOPMTuA2hNAn2YO0ogSkFUUkoKgHYwEYr4dy7o9KBxKzy1Vo9NUakDo+zfVLLm9PW9mLh51rVla8x/XtYD/3YVtj6ssqZY78/gu+KZ0rQcNMpmTCR1ifkGdyFaPz8Tmsa9U0NiW9V0gJ6kJkQebPG8SMyycGVgtOB5boEd4r2F6c2y3AuSH8zlo/BBqZeqqBVNZrYWUBzPq70UMLl8aIxGmmAQQEP16YD8bhkmwSc9XQSCy8RGJ9Eq19jE5+F+xngtsBkJ46rRbU9+DfoQWK5SZQtCPC7fBBOp1erpMIAcx1m+BtxycWJNZ5p468OLYZbQe/O93M9TwtxJmi0dMPaarlfkIrI84jcthUUWL99cSKpJSUl12LjUdwbrdWR7A9Su1lmEWjaYlU73U2aMvyxZippEehbVukV1X87q+Bvz91+bifT6gW8h0y+iDH7Z9XHKs64bRKrDjy+ORICMPHKH2LWZ+GRpnLm5zpFzN7QkIWiuE7gduGtFyDaSWBY4p/amCnknXWdY+Lw1lEjHd/aOwoNucpSuV2dINYEwfG7QHHsMlBnNweF059bBkkpdljYFv0WQ/Q72/BdsO1FjwDVO9ue/6sUDO6KghvNHA9/7n8jnN8fG1hSBlt7jdcJ63nY8sYNgU6qNU9/QyU9ygcfO7NZ8HID6BC75/ArMT0F/Uq3yqp6Zn86FnZ6vvI3I4P1ZkM17uUjGqRq0w26GjB/IvDAF7jaomxnKQIrN2g4dy6/KaxHIR2nA5mSK80ySZ2JY5nlPqh1U/PdCMyz5G 8TQkjl+J n7ASPfrrL/cPOWeJ+vRfL0SM+0M6n+R1kFu4YIp2AqX+8Q+42h8MTfLsYiYTpQKoH6xwq1S7+/0gZqHX2fDvMslStkeKRmok26ozGdAPI+LAX2f9kx6LYDdfyNN0yIxpo09miJsSbYniLKoyTrLFFzuDfDf/6mJHIaq3aHLcxA4fL2+gGv9rZGPsvmivzlWRWw3olLSYVaTaHif9njT0qyvQrGMrtcvF9oQYvLscJUzwSyQ4R8TYzBP3ztQ85/i7WPeJPWjHgjbJcY7PQHJM3aqFRC5AK1xEn5cvfPjfCBgo9TXeIwFG7ocAlBwNgapf3MS3qfCeicy2kOoBDy6fPIdtrTbhthgOhAUHg9/OclaIerdkXMNiazzuHbjRZgo1067l95aLnQto2FkYTnQXKFfPQAg== 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 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? >> #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