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 C1521C43334 for ; Tue, 5 Jul 2022 14:23:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 544AD6B0071; Tue, 5 Jul 2022 10:23:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4F44E6B0073; Tue, 5 Jul 2022 10:23:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3E4296B0074; Tue, 5 Jul 2022 10:23:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 301E06B0071 for ; Tue, 5 Jul 2022 10:23:43 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 0378035889 for ; Tue, 5 Jul 2022 14:23:42 +0000 (UTC) X-FDA: 79653264726.29.1F1F2AF Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf24.hostedemail.com (Postfix) with ESMTP id 67DE918005C for ; Tue, 5 Jul 2022 14:23:42 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 1A887229AA; Tue, 5 Jul 2022 14:23:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1657031020; h=from:from:reply-to: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=jcuDAT42MRN08+ai7zVZVZ2mWppuivEezCE/Bu5mhtU=; b=owi3RO93Ow5n5L73HHdTiQn2hfqPSvdUleh8KgUxZseP/ZBphU+H2KIcgWp0yOuyd/tPNX tA91jLVfbnO4kHORFIEqLMH2ak5lnHc1iKigU1DIvSOghhPNyDORiB3mSrrCg0C5fUSqJp ynPIt+3GTWOOn7TKZHxuFbmAfJ6ma0w= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1657031020; h=from:from:reply-to: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=jcuDAT42MRN08+ai7zVZVZ2mWppuivEezCE/Bu5mhtU=; b=T6AyjA5QRILuUAb7gk4isa/t+rjA2Qg9uB5wx/oL5bTShCnXOn0HwI9w9azt/RA5Dsn0N5 jvH/ojhAEBRt/8Aw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 095FC1339A; Tue, 5 Jul 2022 14:23:37 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id MHsBCWlJxGI7fQAAMHmgww (envelope-from ); Tue, 05 Jul 2022 14:23:37 +0000 Date: Tue, 5 Jul 2022 09:23:34 -0500 From: Goldwyn Rodrigues To: Josef Bacik Cc: Jens Axboe , "Darrick J. Wong" , Al Viro , Stefan Roesch , io-uring@vger.kernel.org, kernel-team@fb.com, linux-mm@kvack.org, linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, david@fromorbit.com, jack@suse.cz, hch@infradead.org, Christoph Hellwig Subject: Re: [PATCH v7 15/15] xfs: Add async buffered write support Message-ID: <20220705142243.hlevsj4pfpahjcdv@fiona> References: <20220601210141.3773402-1-shr@fb.com> <20220601210141.3773402-16-shr@fb.com> <0a75a0c4-e2e5-b403-27bc-e43872fecdc1@kernel.dk> <47dd9e6a-4e08-e562-12ff-5450fc42da77@kernel.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1657031022; a=rsa-sha256; cv=none; b=tXt11TioZEkCjNNTmPDEJ4GzIC1sKaFCtNBIdf9zqqPU1ef0sBDLKKIZ7Pq+DQKtOvQ0Ih kcNtcn8zgumH8LPMDufg4/yW3O4HKU1Np46uP+UDQP+hVh3+liDQtdPEVDCJj+RBsKSxg6 2vUDXyCoAZdcnL25yoc7sboJBqCx2c0= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=suse.de header.s=susede2_rsa header.b=owi3RO93; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=T6AyjA5Q; dmarc=pass (policy=none) header.from=suse.de; spf=pass (imf24.hostedemail.com: domain of rgoldwyn@suse.de designates 195.135.220.28 as permitted sender) smtp.mailfrom=rgoldwyn@suse.de ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1657031022; 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=jcuDAT42MRN08+ai7zVZVZ2mWppuivEezCE/Bu5mhtU=; b=bMAKx8+jD3+cNOQeRiIeKrtzgJyTQg1IcJq13IP9vZiatAO2WTZ6GWOd1oQxs8G/ytWCQd HJ4GOG1MyUQBLRRv8Lnw5+wSmN4zZO9dJ+dtSxtOJjj4pTqtmWzypktcer/OCf3lJHg4nX /ZCZJwJO4NBbz5+fH8gL2XGPVrsje7k= Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=suse.de header.s=susede2_rsa header.b=owi3RO93; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=T6AyjA5Q; dmarc=pass (policy=none) header.from=suse.de; spf=pass (imf24.hostedemail.com: domain of rgoldwyn@suse.de designates 195.135.220.28 as permitted sender) smtp.mailfrom=rgoldwyn@suse.de X-Rspam-User: X-Stat-Signature: mos7p9uyeiyj94jymdbdbttprz78ic51 X-Rspamd-Queue-Id: 67DE918005C X-Rspamd-Server: rspam04 X-HE-Tag: 1657031022-888481 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: On 9:47 05/07, Josef Bacik wrote: > On Fri, Jul 01, 2022 at 12:14:41PM -0600, Jens Axboe wrote: > > On 7/1/22 12:05 PM, Darrick J. Wong wrote: > > > On Fri, Jul 01, 2022 at 08:38:07AM -0600, Jens Axboe wrote: > > >> On 7/1/22 8:30 AM, Jens Axboe wrote: > > >>> On 7/1/22 8:19 AM, Jens Axboe wrote: > > >>>> On 6/30/22 10:39 PM, Al Viro wrote: > > >>>>> On Wed, Jun 01, 2022 at 02:01:41PM -0700, Stefan Roesch wrote: > > >>>>>> This adds the async buffered write support to XFS. For async buffered > > >>>>>> write requests, the request will return -EAGAIN if the ilock cannot be > > >>>>>> obtained immediately. > > >>>>> > > >>>>> breaks generic/471... > > >>>> > > >>>> That test case is odd, because it makes some weird assumptions about > > >>>> what RWF_NOWAIT means. Most notably that it makes it mean if we should > > >>>> instantiate blocks or not. Where did those assumed semantics come from? > > >>>> On the read side, we have clearly documented that it should "not wait > > >>>> for data which is not immediately available". > > >>>> > > >>>> Now it is possible that we're returning a spurious -EAGAIN here when we > > >>>> should not be. And that would be a bug imho. I'll dig in and see what's > > >>>> going on. > > >>> > > >>> This is the timestamp update that needs doing which will now return > > >>> -EAGAIN if IOCB_NOWAIT is set as it may block. > > >>> > > >>> I do wonder if we should just allow inode time updates with IOCB_NOWAIT, > > >>> even on the io_uring side. Either that, or passed in RWF_NOWAIT > > >>> semantics don't map completely to internal IOCB_NOWAIT semantics. At > > >>> least in terms of what generic/471 is doing, but I'm not sure who came > > >>> up with that and if it's established semantics or just some made up ones > > >>> from whomever wrote that test. I don't think they make any sense, to be > > >>> honest. > > >> > > >> Further support that generic/471 is just randomly made up semantics, > > >> it needs to special case btrfs with nocow or you'd get -EAGAIN anyway > > >> for that test. > > >> > > >> And it's relying on some random timing to see if this works. I really > > >> think that test case is just hot garbage, and doesn't test anything > > >> meaningful. > > > > > > I had thought that NOWAIT means "don't wait for *any*thing", > > > which would include timestamp updates... but then I've never been all > > > that clear on what specifically NOWAIT will and won't wait for. :/ > > > > Agree, at least the read semantics (kind of) make sense, but the ones > > seemingly made up by generic/471 don't seem to make any sense at all. > > > > Added Goldwyn to the CC list for this. > > This appears to be just a confusion about what we think NOWAIT should mean. > Looking at the btrfs code it seems like Goldwyn took it as literally as possible > so we wouldn't do any NOWAIT IO's unless it was into a NOCOW area, meaning we > literally wouldn't do anything other than wrap the bio up and fire it off. When I introduced NOWAIT, it was only meant for writes and for those which would block on block allocations or locking. Over time it was included for buffered reads as well. > > The general consensus seems to be that NOWAIT isn't that strict, and that > BTRFS's definition was too strict. I wrote initial patches to give to Stefan to > clean up the Btrfs side to allow us to use NOWAIT under a lot more > circumstances. BTRFS COW path would allocate blocks and the reason it returns -EAGAIN. > > Goldwyn, this test seems to be a little specific to our case, and can be flakey > if the timing isn't just right. I think we should just remove it? Especially > since how we define NOWAIT isn't quite right. Does that sound reasonable to > you? Yes, I agree. It was based on the initial definition and now can be removed. -- Goldwyn