Received: by 2002:a25:8b12:0:0:0:0:0 with SMTP id i18csp1438621ybl; Wed, 28 Aug 2019 14:55:00 -0700 (PDT) X-Google-Smtp-Source: APXvYqwAI8azLxSEueQ0JIy0IVkkWZFRphLi7GbnXWHRepvCg6JeKyptUmEpHv2zg9vaxEDjKX6a X-Received: by 2002:a63:506:: with SMTP id 6mr5298740pgf.434.1567029300266; Wed, 28 Aug 2019 14:55:00 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1567029300; cv=none; d=google.com; s=arc-20160816; b=lPwckzO/Fs2pGlQBzf/eiNth7xjkPP1RWyvPDlxnVgtSKRfw+ORopDk9+4vckKxLUZ fiR54uuR66YcHILR55N/4RWHQkUXOEcuhkfH/g/6mmU7BWIJf8w2mXYxJ0DHsSjmBnaf WAsCmPavOBwhFx0FMJlNwA/f5tM3CVIPLSGS4x6iyX31+MUClm9wNOz7jWq1b1oG0XDa Zbi3NAamCMDPJpQ3iAmfxqSZLEwsApWl7Fwz0DwQSEWslfw2BnEMmZu63ngM2uw+nuWG w9IbPuQzAxbO2wgGC4uHZMND7aG90ns1D+UyOcIDAEHIWZiyVD1+OHo7+QaABCMaK8sG 8hmw== 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=9AFvDPdMFXF3Oi+pi/ljks9u7VZds22rpfzOC3/TilY=; b=mdgGXRHqK5RzZFJXOvEQmFl3xr8aTFybv8AC0AI4WPcKiMEvgr1v2N6jgjM7mrvAK2 InJnsA8XbbdfegFFOwISxCnbKCq+7MmZwiz1He8uZ850e5KL0ovAZS02V65kz7FDa/v9 6WpSmNETIYD28hU93vhbjaId2md8U0wuzdXQWyXN/RoP7LzMgzR9hFPe8XNL1ArzakY6 Cp5NeKUxxSh1EElnnTRkB1TCk6zI7xb1TU10FSizsjbA114VYe7TR7Fnb39WGPElcQ1P PMH/09C7mz2/JJnAk/sg6q67NWaapXrpJ0DEIPr3ycZlYvnROr17jcq5xezuZhW5674c SBhA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@mbobrowski-org.20150623.gappssmtp.com header.s=20150623 header.b=fBUiQzof; 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 t5si270743pgr.172.2019.08.28.14.54.36; Wed, 28 Aug 2019 14:55:00 -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=fBUiQzof; 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 S1726944AbfH1Vya (ORCPT + 99 others); Wed, 28 Aug 2019 17:54:30 -0400 Received: from mail-pl1-f193.google.com ([209.85.214.193]:46275 "EHLO mail-pl1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726921AbfH1Vya (ORCPT ); Wed, 28 Aug 2019 17:54:30 -0400 Received: by mail-pl1-f193.google.com with SMTP id o3so553410plb.13 for ; Wed, 28 Aug 2019 14:54:29 -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=9AFvDPdMFXF3Oi+pi/ljks9u7VZds22rpfzOC3/TilY=; b=fBUiQzofqeIX+1GAyoU7Al8WZfc+JCKB4NVcOGNJRp/tSzRJmpVSkBVFY/VDiuMZUJ fQ5nW7Jj7SJJ5JAv1zEZzj7016mrwWeWpCjhv+VcOnZBx4BFl+nv0o5rryC+yf8sNLaa O8rSwOnwgZC8QAEIfvNaiARRQW/gmJKG3lhOn3Wl3jS2rACx1hbYsk/bilW9a4KNE9LE 3hkMRD81yKAIF6j7DQn1nG0AHv51lxuu/XZJPQ74AnJit7lC7dKlUfbn4qJNRTSZavWM YcG89xKJ0An3nIUXDGN1BRtpcarkuIoH9jWXQTresfZgxcD+eLHLBcVpzpybyL8SeWR2 2vMg== 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=9AFvDPdMFXF3Oi+pi/ljks9u7VZds22rpfzOC3/TilY=; b=PNysQyBmTSclK5DC616e4YEv7W2OLdkyj49jNZlyhXoXeObICFV7grh1dBk/p+VRJj Rtdf05E6CnL4hW3msc7e5cVwCuz/9OqtdqR/19nIvtxtmeS/sZOieXjwTwHScfnc4t6b dBQ3JcXT13pCUVEmojt9r6+tOsbpkYW6uN8GNDg2CBVEyTJpA/KMmvJIxh0HBzXDw4vs uMjIijd4HF8H7+cj1nkvhIr8ZeAob4A2DN6zuRzzWE04VCpuzeaLycf/h563+lkXbDqi Tm7c9wtbEz5XdnxWOuvxzYYVyT/M14GZngFcBVqmtdaO3RYHuxzW6BUXgfu1EtwDy44/ ku/Q== X-Gm-Message-State: APjAAAWlEjL3gmr7GH3bYA0rueauBbybRNTnS1VtLjtCUHvvsSsQKgya xAH/DErwqH0nA5GU14h1uJ3o X-Received: by 2002:a17:902:3064:: with SMTP id u91mr6600966plb.244.1567029268996; Wed, 28 Aug 2019 14:54:28 -0700 (PDT) Received: from athena.bobrowski.net ([120.18.194.154]) by smtp.gmail.com with ESMTPSA id a128sm353474pfb.185.2019.08.28.14.54.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Aug 2019 14:54:28 -0700 (PDT) Date: Thu, 29 Aug 2019 07:54:21 +1000 From: Matthew Bobrowski To: Jan Kara Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, tytso@mit.edu, riteshh@linux.ibm.com Subject: Re: [PATCH 2/5] ext4: move inode extension/truncate code out from ext4_iomap_end() Message-ID: <20190828215421.GA9221@athena.bobrowski.net> References: <774754e9b2afc541df619921f7743d98c5c6a358.1565609891.git.mbobrowski@mbobrowski.org> <20190828195914.GF22343@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190828195914.GF22343@quack2.suse.cz> 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 Wed, Aug 28, 2019 at 09:59:14PM +0200, Jan Kara wrote: > On Mon 12-08-19 22:52:53, Matthew Bobrowski wrote: > > +static int ext4_handle_inode_extension(struct inode *inode, loff_t size, > > + size_t count) > > +{ > > + handle_t *handle; > > + bool truncate = false; > > + ext4_lblk_t written_blk, end_blk; > > + int ret = 0, blkbits = inode->i_blkbits; > > + > > + handle = ext4_journal_start(inode, EXT4_HT_INODE, 2); > > + if (IS_ERR(handle)) { > > + ret = PTR_ERR(handle); > > + goto orphan_del; > > + } > > + > > + if (ext4_update_inode_size(inode, size)) > > + ext4_mark_inode_dirty(handle, inode); > > + > > + /* > > + * We may need truncate allocated but not written blocks > > + * beyond EOF. > > + */ > > + written_blk = ALIGN(size, 1 << blkbits); > > + end_blk = ALIGN(size + count, 1 << blkbits); > > So this seems to imply that 'size' is really offset where IO started but > ext4_update_inode_size(inode, size) above suggests 'size' is really where > IO has ended and that's indeed what you pass into > ext4_handle_inode_extension(). So I'd just make the calling convention for > ext4_handle_inode_extension() less confusing and pass 'offset' and 'len' > and fixup the math inside the function... Agree, that will help with making things more transparent. Also, one other thing while looking at this patch again. See comment below. > > @@ -257,6 +308,13 @@ ext4_dax_write_iter(struct kiocb *iocb, struct iov_iter *from) > > goto out; > > > > ret = dax_iomap_rw(iocb, from, &ext4_iomap_ops); > > + > > + if (ret > 0 && iocb->ki_pos > i_size_read(inode)) { > > + err = ext4_handle_inode_extension(inode, iocb->ki_pos, > > + iov_iter_count(from)); > > + if (err) > > + ret = err; > > + } I noticed that within ext4_dax_write_iter() we're not accommodating for error cases. Subsequently, there's no clean up code that goes with that. So, for example, in the case where we've added the inode onto the orphan list as a result of an extension and we bump into any error i.e. -ENOSPC, we'll be left with inconsistencies. Perhaps it might be worthwhile to introduce a helper here i.e. ext4_dax_handle_failed_write(), which would be the path taken to perform any necessary clean up routines in the case of a failed write? I think it'd be better to have this rather than polluting ext4_dax_write_iter(). What do you think? --M