From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bob Peterson Date: Fri, 6 Apr 2018 10:44:13 -0400 (EDT) Subject: [Cluster-devel] [PATCH v3 5/8] gfs2: Implement iomap buffered write support In-Reply-To: <20180323191728.23819-6-agruenba@redhat.com> References: <20180323191728.23819-1-agruenba@redhat.com> <20180323191728.23819-6-agruenba@redhat.com> Message-ID: <807083870.16612494.1523025853631.JavaMail.zimbra@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hi, ----- Original Message ----- (snip) > +static int gfs2_iomap_begin_write(struct inode *inode, loff_t pos, loff_t > length, > + unsigned flags, struct iomap *iomap) (snip) > +{ > + struct metapath mp = { .mp_aheight = 1, }; > + struct gfs2_inode *ip = GFS2_I(inode); > + struct gfs2_sbd *sdp = GFS2_SB(inode); > + unsigned int data_blocks = 0, ind_blocks = 0, rblocks; > + bool unstuff, alloc_required; > + int ret; > + > + ret = gfs2_write_lock(inode); > + if (ret) > + return ret; > + > + unstuff = gfs2_is_stuffed(ip) && > + pos + length > gfs2_max_stuffed_size(ip); > + > + ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp); > + if (ret) > + goto out_release; > + > + alloc_required = unstuff || iomap->type != IOMAP_MAPPED; > + > + if (alloc_required || gfs2_is_jdata(ip)) > + gfs2_write_calc_reserv(ip, iomap->length, &data_blocks, &ind_blocks); It would be ideal if the iomap_get could tell us how many blocks are mapped and how many are unmapped, if it wouldn't cause unnecessary IO. If we could use the number of unmapped blocks rather than iomap->length, it could potentially save us from reserving unnecessary journal space, for example, on rewrites (iow where the metadata for the writes already exists). (snip) Bob Peterson From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:52472 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752248AbeDFOoO (ORCPT ); Fri, 6 Apr 2018 10:44:14 -0400 Date: Fri, 6 Apr 2018 10:44:13 -0400 (EDT) From: Bob Peterson To: Andreas Gruenbacher Cc: cluster-devel@redhat.com, Christoph Hellwig , linux-fsdevel@vger.kernel.org Message-ID: <807083870.16612494.1523025853631.JavaMail.zimbra@redhat.com> In-Reply-To: <20180323191728.23819-6-agruenba@redhat.com> References: <20180323191728.23819-1-agruenba@redhat.com> <20180323191728.23819-6-agruenba@redhat.com> Subject: Re: [Cluster-devel] [PATCH v3 5/8] gfs2: Implement iomap buffered write support MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Hi, ----- Original Message ----- (snip) > +static int gfs2_iomap_begin_write(struct inode *inode, loff_t pos, loff_t > length, > + unsigned flags, struct iomap *iomap) (snip) > +{ > + struct metapath mp = { .mp_aheight = 1, }; > + struct gfs2_inode *ip = GFS2_I(inode); > + struct gfs2_sbd *sdp = GFS2_SB(inode); > + unsigned int data_blocks = 0, ind_blocks = 0, rblocks; > + bool unstuff, alloc_required; > + int ret; > + > + ret = gfs2_write_lock(inode); > + if (ret) > + return ret; > + > + unstuff = gfs2_is_stuffed(ip) && > + pos + length > gfs2_max_stuffed_size(ip); > + > + ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp); > + if (ret) > + goto out_release; > + > + alloc_required = unstuff || iomap->type != IOMAP_MAPPED; > + > + if (alloc_required || gfs2_is_jdata(ip)) > + gfs2_write_calc_reserv(ip, iomap->length, &data_blocks, &ind_blocks); It would be ideal if the iomap_get could tell us how many blocks are mapped and how many are unmapped, if it wouldn't cause unnecessary IO. If we could use the number of unmapped blocks rather than iomap->length, it could potentially save us from reserving unnecessary journal space, for example, on rewrites (iow where the metadata for the writes already exists). (snip) Bob Peterson