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 E84EAC43334 for ; Tue, 5 Jul 2022 13:47:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4EAC16B0071; Tue, 5 Jul 2022 09:47:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4999C6B0073; Tue, 5 Jul 2022 09:47:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 363AF6B0074; Tue, 5 Jul 2022 09:47:29 -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 26AB16B0071 for ; Tue, 5 Jul 2022 09:47:29 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 03A646083E for ; Tue, 5 Jul 2022 13:47:28 +0000 (UTC) X-FDA: 79653173418.28.ECE555B Received: from mail-qt1-f178.google.com (mail-qt1-f178.google.com [209.85.160.178]) by imf02.hostedemail.com (Postfix) with ESMTP id 6EBCA8000C for ; Tue, 5 Jul 2022 13:47:28 +0000 (UTC) Received: by mail-qt1-f178.google.com with SMTP id i11so13682744qtr.4 for ; Tue, 05 Jul 2022 06:47:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=toxicpanda-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=aC7tiWGDUmjG4E+Vlf1HDuDZFXAFOR+FnjCqRmORTyc=; b=HPc5a7APrqmVXvuv5diow+iSYd8P0GBEUcygVWpOkE9Y4My7LL+qpTLid+Vu13E7kq ZMzttA9/ceVExR+3wOxr9nXJMAmqRpxVRSpYfEVN/whAoVKGpdd+2bnjuO0hZ0cKt6HL PE8bLwSxxoYkiuJytIdQBdmnQCnoNlzIxkyB6hT8uKG6uENRnBCSh3qQyDDRIdtq2kcf m9+dqK82EAbZwoLIeAqHobNHiyQHxfPQ7zF+aST8mPzSQbGYsOJURJqVNtVPmusK+qqN +vQQA4O2OJyk6d3OD+pLnWLLy70RYnuD+ox2bgcmMRbq9cPOmSHZmkCam8OCt8A598JW ZQxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=aC7tiWGDUmjG4E+Vlf1HDuDZFXAFOR+FnjCqRmORTyc=; b=tIZNrEJVD3V9w431zZrO5Bk+f1pGcsFIXnRs9eYEKpwovgyNzxQ8i8xPscRzlNQGjS RfqhUUp5dWTZUe5rQQs3IiY2fipfhgT+QmQN+lJ5Fae1iBrJwfuTqAlhpahTJbNyVQMs kkzphuTE0JYSkcKP0L8TxE/dz7J/F1DzTh7F8NYxHVo7/fAYo5MUD4zTiw5r88iToRvT /U4sp+SCkmTdPogASI6OYIrZi3ub40MieyK1NYjpXVdwqQpabZD4EN4Jxf1p0dx+TXTK rjZfgLqUIg1uJHuRvm79pQ7s81gZ35WNnlH7n1z+U7ghfe8MsLdsYuzpNU74u+vFP/Ld xpqg== X-Gm-Message-State: AJIora+IhX8H5+TRRCHzw8Gz7Jf8DvpJAGvvOdxjgYF5MwgKsehAiBLw KJMMsS6ZQ0xBxNGBKx003/a/HQ== X-Google-Smtp-Source: AGRyM1vvib7F+uTcsONBGb8V+lPuSuzNrIyahf3+BZ177JeEEhvuW5nixoUCeLrPQ5VLMB5h+kPVNQ== X-Received: by 2002:a05:6214:5099:b0:473:5d:d29f with SMTP id kk25-20020a056214509900b00473005dd29fmr5025227qvb.55.1657028847407; Tue, 05 Jul 2022 06:47:27 -0700 (PDT) Received: from localhost (cpe-174-109-172-136.nc.res.rr.com. [174.109.172.136]) by smtp.gmail.com with ESMTPSA id c4-20020a05620a268400b006aee03a95dfsm26905631qkp.124.2022.07.05.06.47.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Jul 2022 06:47:26 -0700 (PDT) Date: Tue, 5 Jul 2022 09:47:25 -0400 From: Josef Bacik To: Jens Axboe Cc: "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 , rgoldwyn@suse.com Subject: Re: [PATCH v7 15/15] xfs: Add async buffered write support Message-ID: 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: <47dd9e6a-4e08-e562-12ff-5450fc42da77@kernel.dk> ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1657028848; a=rsa-sha256; cv=none; b=wc8Oba5Hq58e85KfyMIXJfeY7Ux3KsjTNK7Vq2t3CKOKLpodq9UUJ3YQS6+nuGxBJeteU7 OYTRnuKpIvErDWRJPEBg9Sr2B0FVT6bTuIuqwIWTplbOZ4h3dU2Ricg5STyr40q4M+F6m9 a9dr4yiZ+fvH+Eo6QC2Vjv+bIp8U1nc= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=toxicpanda-com.20210112.gappssmtp.com header.s=20210112 header.b=HPc5a7AP; dmarc=none; spf=none (imf02.hostedemail.com: domain of josef@toxicpanda.com has no SPF policy when checking 209.85.160.178) smtp.mailfrom=josef@toxicpanda.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1657028848; 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=aC7tiWGDUmjG4E+Vlf1HDuDZFXAFOR+FnjCqRmORTyc=; b=fllTVZZ+lDFVRZyh+rkvlYtI/mZ0iIODVB9H4gZE9N++X1yUrU1FNdl+KmLw6VTaRIiCWa LmurlINv+96D+pQrXzH1gLmZJ0wrI6LeS7nhXt0dt+RG/x5GvfdWjTpDBitiXCmVMghZkV 7V+SmnUiiQvRqJwG87mfTNKxwzL7VOc= X-Rspam-User: X-Rspamd-Server: rspam07 Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=toxicpanda-com.20210112.gappssmtp.com header.s=20210112 header.b=HPc5a7AP; dmarc=none; spf=none (imf02.hostedemail.com: domain of josef@toxicpanda.com has no SPF policy when checking 209.85.160.178) smtp.mailfrom=josef@toxicpanda.com X-Stat-Signature: uor97fqtztce1ujxo6ri354s3gg49tt3 X-Rspamd-Queue-Id: 6EBCA8000C X-HE-Tag: 1657028848-305789 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 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. 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. 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? I think a decent followup would be to add a NOWAIT specific fio test to fsperf so we still can catch any NOWAIT related regressions, without trying to test for specific behavior for something that can fail under a whole lot of conditions unrelated to our implementation. Thanks, Josef