Received: by 2002:a25:8b12:0:0:0:0:0 with SMTP id i18csp831828ybl; Wed, 21 Aug 2019 06:17:33 -0700 (PDT) X-Google-Smtp-Source: APXvYqy0mUh3Ihr9h/del/1yad3fKSe5f5/hL9TdLOZuoBotS3nBUTQFQBfZlK/cZAv1FdvqcWLb X-Received: by 2002:a17:902:8543:: with SMTP id d3mr34672009plo.80.1566393453674; Wed, 21 Aug 2019 06:17:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1566393453; cv=none; d=google.com; s=arc-20160816; b=WN7D8T5YKEkuN4zbYyhxTEFSQz40iOzCHfQG+G9uf9l12/LRSgIixZWeek4GUzMhco COhQxaaU/779cbt6flh7FdoMMy6jpsv2NP5bEj0zGEkxcJQB6JZSgIU+0coltI1SlQT3 piSyp9E6wevO9jNJ/5Ptq8Pgpapowdyn9M33pH9Mlu2K+3gowUX7A2WuwSLVQNaRmmwT vozUa5iJjs7jx+rsPdq/cFnOY8IouTX+pPPPwVXVM0j7MQCM0TgFm0u6ZJOzLWGvgglw MdUyzEVHKOUawp53bhBL4Rag7nQSk5dtQ9NlIGmWlK3REqAu9P0i/O3/Hq7+CFEi3t4X Nseg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=+FTVPQJIElgo/eBaJWXuUPChwGI7d5OiYHXZ+WGlwrs=; b=rx++u0a7HDrlsRd7t4zPeX2tEciG+UHyHcOyMlhCPWEYKucqcWSFZa1748IsZ4r4aY FXQ2dt4JsZMsL951/Vn4PfNcKTeziHgI7NC2p1Bt5lnZpYhnLlcKLQYpuJgRajhYL77h JNzS7M8eB8AE1jaLrvKoJs4yC9BmxmLax0GOYPZ/UissJlOoEJHLXGZ/gu9n2Mji5C/M i3lmwdz5S/BEJ/WOXiPtlOS+aDb5pes3IVSdFOzC7Vriq+vKJ1CloxTpu2JWbL6F2Hod GDw/zkr2T1AyphM3rbk/c+ElaQfH92f93BtQY6XRjDeVmPX0NJ8glxxnclnOXdYR8WY2 CD7A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@mbobrowski-org.20150623.gappssmtp.com header.s=20150623 header.b=pv7Lv0nk; spf=pass (google.com: best guess record for domain of linux-ext4-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-ext4-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id b24si14523858pgh.41.2019.08.21.06.17.10; Wed, 21 Aug 2019 06:17:33 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-ext4-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@mbobrowski-org.20150623.gappssmtp.com header.s=20150623 header.b=pv7Lv0nk; spf=pass (google.com: best guess record for domain of linux-ext4-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-ext4-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726371AbfHUNOV (ORCPT + 99 others); Wed, 21 Aug 2019 09:14:21 -0400 Received: from mail-pf1-f194.google.com ([209.85.210.194]:36749 "EHLO mail-pf1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728339AbfHUNOU (ORCPT ); Wed, 21 Aug 2019 09:14:20 -0400 Received: by mail-pf1-f194.google.com with SMTP id w2so1429032pfi.3 for ; Wed, 21 Aug 2019 06:14:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mbobrowski-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=+FTVPQJIElgo/eBaJWXuUPChwGI7d5OiYHXZ+WGlwrs=; b=pv7Lv0nkvGtPx9sY7uJXRk6DeNkxZVidTa9axqCI8TsPkpGAtyBqFeZh1Y5Qt76QVO LE7EHSmgpSIYkVC0X+TBiYBVx6/m3GNkXY4HLlSbORfeZ5i2FF/R2gSdPyLnYYGsLMgO Z+bfz916hUGJEkJWQmoWsw6oIVLRtEc70MpSNpYRytxK1bMYr5BD1dP2uxlWWuWgOHpm b8XRyA3rbpx1UBrsWpl9YY5E3tsPYovOIHmTg+bc8CPO91doAzsg2kvIi1b30LbqhNst ritcgFBSQp6bc+fXkljxq7SweSg816s85RHdMbrlBbgq3fTQtyUmHcWhi2v4ZbHOBRrn ustw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=+FTVPQJIElgo/eBaJWXuUPChwGI7d5OiYHXZ+WGlwrs=; b=OxsRo3FqMP8mJWMwx4sWRRBfDxaakAKt5c45y0qTZItF9H+SHLjmqW83w7Om1Smbo4 94g4RNEPCXrQW4N8kQHesi2i6XyLJX/dffTzC1O+H1g8tCHnRRCh7egRV5pLiyt1aTUe Z290TqsM1tknUoKSr7RUk99dILU7w1Ls/nvJ85ZUO7NiJJlK37MaruNXRb/y6146hexb K1HAIDORwF9khdRbIJTgGYowtvqGxyI7HWqWKAXtseTKKLp2UEqMuq2RMNgkezt2gHEM y29iZ5T4PdJMxfj6vMRLq6YyiY6uiwVnWD/dyT2dtPT0XkzERHCirkTUoxjfYVbUf+Q3 b/qg== X-Gm-Message-State: APjAAAV6krHi35h1FLvaVs4Iuytp3eP6FqZKHwDB00s21X6Ns1HcvFeZ oRsltszSN3xFKC8efWEz9aRN X-Received: by 2002:a63:4b02:: with SMTP id y2mr28565129pga.135.1566393254090; Wed, 21 Aug 2019 06:14:14 -0700 (PDT) Received: from poseidon.bobrowski.net ([114.78.226.167]) by smtp.gmail.com with ESMTPSA id g2sm44391425pfq.88.2019.08.21.06.14.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Aug 2019 06:14:13 -0700 (PDT) Date: Wed, 21 Aug 2019 23:14:07 +1000 From: Matthew Bobrowski To: tytso@mit.edu, jack@suse.cz, adilger.kernel@dilger.ca Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, riteshh@linux.ibm.com, hch@infradead.org Subject: Re: [PATCH 0/5] ext4: direct IO via iomap infrastructure Message-ID: <20190821131405.GC24417@poseidon.bobrowski.net> References: <20190812173150.AF04F5204F@d06av21.portsmouth.uk.ibm.com> <20190813111004.GA12682@poseidon.bobrowski.net> <20190813122723.AE6264C040@d06av22.portsmouth.uk.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190813122723.AE6264C040@d06av22.portsmouth.uk.ibm.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Tue, Aug 13, 2019 at 05:57:22PM +0530, RITESH HARJANI wrote: > But what I meant was this (I may be wrong here since I haven't really looked > into it), but for my understanding I would like to discuss this - > > So earlier with this flag(EXT4_STATE_DIO_UNWRITTEN) we were determining on > whether a newextent can be merged with ex1 in function > ext4_extents_can_be_merged. But now since we have removed that flag we have > no way of knowing that whether this inode has any unwritten extents or not > from any DIO path. > > What I meant is isn't this removal of setting/unsetting of > flag(EXT4_STATE_DIO_UNWRITTEN) changing the behavior of this func - > ext4_extents_can_be_merged? OK, I'm stuck and looking for either clarity, revalidation of my thought process, or any input on how to solve this problem for that matter. In the current ext4 direct IO implementation, the dynamic state flag EXT4_STATE_DIO_UNWRITTEN is set/unset for synchronous direct IO writes. On the other hand, the flag EXT4_IO_END_UNWRITTEN is set/unset for ext4_io_end->flag, and the value of i_unwritten is incremented/decremented for asynchronous direct IO writes. All mechanisms by which are used to track and determine whether the inode, or an IO in flight against a particular inode have any pending unwritten extents that need to be converted after the IO has completed. In addition to this, we have ext4_can_extents_be_merged() performing explicit checks against both EXT4_STATE_DIO_UNWRITTEN and i_unwritten and using them to determine whether it can or cannot merge a requested extent into an existing extent. This is all fine for the current direct IO implementation. However, while switching the direct IO code paths over to make use of the iomap infrastructure, I believe that we can no longer simply track whether an inode has unwritten extents needing to be converted by simply setting and checking the EXT4_STATE_DIO_UNWRITTEN flag on the inode. The reason being is that there can be multiple direct IO operations to unwritten extents running against the inode and we don't particularly distinguish synchronous from asynchronous writes within ext4_iomap_begin() as there's really no need. So, the only way to accurately determine whether extent conversion is deemed necessary for an IO operation whether it'd be synchronous or asynchronous is by checking the IOMAP_DIO_UNWRITTEN flag within the ->end_io() callback. I'm certain that this portion of the logic is correct, but we're still left with some issues when it comes to the checks that I previously mentioned in ext4_can_extents_be_merged(), which is the part I need some input on. I was doing some thinking and I don't believe that making use of the EXT4_STATE_DIO_UNWRITTEN flag is the solution at all here. This is not only for reasons that I've briefly mentioned above, but also because of the fact that it'll probably lead to a lot of inaccurate judgements while taking particular code paths and some really ugly code that creeps close to the definition of insanity. Rather, what if we solve this problem by continuing to just use i_unwritten to keep track of all the direct IOs to unwritten against running against an inode? Within ext4_iomap_begin() post successful creation of unwritten extents we'd call atomic_inc(&EXT4_I(inode)->i_unwritten) and subsequently within the ->end_io() callback whether we take the success or error path we'd call atomic_dec(&EXT4_I(inode)->i_unwritten) accordingly? This way we can still rely on this value to be used in the check within ext4_can_extents_be_merged(). Open for alternate suggestions if anyone has any... --M