From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-f195.google.com ([209.85.223.195]:37292 "EHLO mail-io0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751914AbeF0BFH (ORCPT ); Tue, 26 Jun 2018 21:05:07 -0400 Received: by mail-io0-f195.google.com with SMTP id s26-v6so298415ioj.4 for ; Tue, 26 Jun 2018 18:05:07 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20180624190028.4166-2-agruenba@redhat.com> References: <20180624190028.4166-1-agruenba@redhat.com> <20180624190028.4166-2-agruenba@redhat.com> From: Andreas Gruenbacher Date: Wed, 27 Jun 2018 03:05:05 +0200 Message-ID: Subject: Re: [PATCH v10 1/5] gfs2: Further iomap cleanups To: cluster-devel , Christoph Hellwig Cc: linux-fsdevel , Andreas Gruenbacher Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 24 June 2018 at 21:00, Andreas Gruenbacher wrote: > In gfs2_iomap_alloc, set the type of newly allocated extents to > IOMAP_MAPPED so that iomap_to_bh will set the bh states correctly: > otherwise, the bhs would not be marked as mapped, confusing > __mpage_writepage. This means that we need to check for the IOMAP_F_NEW > flag in fallocate_chunk now. > > Further clean up gfs2_iomap_get and implement gfs2_stuffed_iomap here > directly. For reads beyond the end of the file, return holes instead of > failing with -ENOENT so that we can get rid of that special case in > gfs2_block_map. > > Signed-off-by: Andreas Gruenbacher > --- > fs/gfs2/bmap.c | 74 ++++++++++++++++++++++++++++---------------------- > fs/gfs2/file.c | 2 +- > 2 files changed, 43 insertions(+), 33 deletions(-) > > diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c > index ed6699705c13..4341683015bf 100644 > --- a/fs/gfs2/bmap.c > +++ b/fs/gfs2/bmap.c > @@ -750,6 +750,7 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap, > } > } while (iomap->addr == IOMAP_NULL_ADDR); > > + iomap->type = IOMAP_MAPPED; > iomap->length = (u64)dblks << inode->i_blkbits; > ip->i_height = mp->mp_fheight; > gfs2_add_inode_blocks(&ip->i_inode, alloced); > @@ -759,17 +760,6 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap, > return ret; > } > > -static void gfs2_stuffed_iomap(struct inode *inode, struct iomap *iomap) > -{ > - struct gfs2_inode *ip = GFS2_I(inode); > - > - iomap->addr = (ip->i_no_addr << inode->i_blkbits) + > - sizeof(struct gfs2_dinode); > - iomap->offset = 0; > - iomap->length = i_size_read(inode); > - iomap->type = IOMAP_INLINE; > -} > - > #define IOMAP_F_GFS2_BOUNDARY IOMAP_F_PRIVATE > > /** > @@ -789,37 +779,61 @@ static int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length, > { > struct gfs2_inode *ip = GFS2_I(inode); > struct gfs2_sbd *sdp = GFS2_SB(inode); > + loff_t size = i_size_read(inode); > __be64 *ptr; > sector_t lblock; > sector_t lblock_stop; > int ret; > int eob; > u64 len; > - struct buffer_head *bh; > + struct buffer_head *dibh, *bh; > u8 height; > > if (!length) > return -EINVAL; > > + down_read(&ip->i_rw_mutex); > + > + ret = gfs2_meta_inode_buffer(ip, &dibh); > + if (ret) > + goto unlock; > + > if (gfs2_is_stuffed(ip)) { > - if (flags & IOMAP_REPORT) { > - if (pos >= i_size_read(inode)) > - return -ENOENT; > - gfs2_stuffed_iomap(inode, iomap); > - return 0; > + if (flags & IOMAP_WRITE) { > + loff_t max_size = gfs2_max_stuffed_size(ip); > + > + if (pos + length > max_size) > + goto unstuff; > + iomap->length = max_size; > + } else { > + if (pos >= size) { > + if (flags & IOMAP_REPORT) { > + ret = -ENOENT; > + goto unlock; > + } else { > + /* report a hole */ > + iomap->offset = pos; > + iomap->length = length; > + goto do_alloc; > + } > + } > + iomap->length = size; > } > - BUG_ON(!(flags & IOMAP_WRITE)); > + iomap->addr = (ip->i_no_addr << inode->i_blkbits) + > + sizeof(struct gfs2_dinode); > + iomap->type = IOMAP_INLINE; > + goto unlock; To get things right even with the "iomap: direct I/O for inline data" patch just posted, we want to change the above to "goto out". > } > + > +unstuff: > lblock = pos >> inode->i_blkbits; > iomap->offset = lblock << inode->i_blkbits; > lblock_stop = (pos + length - 1) >> inode->i_blkbits; > len = lblock_stop - lblock + 1; > + iomap->length = len << inode->i_blkbits; > > - down_read(&ip->i_rw_mutex); > - > - ret = gfs2_meta_inode_buffer(ip, &mp->mp_bh[0]); > - if (ret) > - goto unlock; > + get_bh(dibh); > + mp->mp_bh[0] = dibh; > > height = ip->i_height; > while ((lblock + 1) * sdp->sd_sb.sb_bsize > sdp->sd_heightsize[height]) > @@ -853,21 +867,23 @@ static int gfs2_iomap_get(struct inode *inode, loff_t pos, loff_t length, > iomap->bdev = inode->i_sb->s_bdev; > unlock: > up_read(&ip->i_rw_mutex); > + if (dibh) > + brelse(dibh); > return ret; > > do_alloc: > iomap->addr = IOMAP_NULL_ADDR; > - iomap->length = len << inode->i_blkbits; > iomap->type = IOMAP_HOLE; > - iomap->flags = 0; > if (flags & IOMAP_REPORT) { > - loff_t size = i_size_read(inode); > if (pos >= size) > ret = -ENOENT; > else if (height == ip->i_height) > ret = gfs2_hole_size(inode, lblock, len, mp, iomap); > else > iomap->length = size - pos; > + } else if (!(flags & IOMAP_WRITE)) { > + if (pos < size && height == ip->i_height) > + ret = gfs2_hole_size(inode, lblock, len, mp, iomap); > } > goto out; > } > @@ -941,12 +957,6 @@ int gfs2_block_map(struct inode *inode, sector_t lblock, > } else { > ret = gfs2_iomap_get(inode, pos, length, 0, &iomap, &mp); > release_metapath(&mp); > - > - /* Return unmapped buffer beyond the end of file. */ > - if (ret == -ENOENT) { > - ret = 0; > - goto out; > - } > } > if (ret) > goto out; > diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c > index 7137db7b0119..6f6bbfbff13d 100644 > --- a/fs/gfs2/file.c > +++ b/fs/gfs2/file.c > @@ -754,7 +754,7 @@ static int fallocate_chunk(struct inode *inode, loff_t offset, loff_t len, > if (error) > goto out; > offset = iomap.offset + iomap.length; > - if (iomap.type != IOMAP_HOLE) > + if (!(iomap.flags & IOMAP_F_NEW)) > continue; > error = sb_issue_zeroout(sb, iomap.addr >> inode->i_blkbits, > iomap.length >> inode->i_blkbits, > -- > 2.17.1 > Andreas