From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Thu, 20 Oct 2016 18:05:35 +0100 Subject: [Cluster-devel] [PATCH 3/4] GFS2: Implement iomap for block_map In-Reply-To: <1476980065-10663-4-git-send-email-rpeterso@redhat.com> References: <1476980065-10663-1-git-send-email-rpeterso@redhat.com> <1476980065-10663-4-git-send-email-rpeterso@redhat.com> Message-ID: <5808F95F.4070402@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, On 20/10/16 17:14, Bob Peterson wrote: > This patch implements iomap for block mapping, and switches the > block_map function to use it under the covers. Definitely going in the right direction now... a few comments below. > Signed-off-by: Bob Peterson > --- > fs/gfs2/bmap.c | 244 ++++++++++++++++++++++++++++++++++++++++++--------------- > fs/gfs2/bmap.h | 4 + > 2 files changed, 185 insertions(+), 63 deletions(-) > > diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c > index 774bdb8..b1bcdd6 100644 > --- a/fs/gfs2/bmap.c > +++ b/fs/gfs2/bmap.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > > #include "gfs2.h" > #include "incore.h" > @@ -594,104 +595,221 @@ static int gfs2_bmap_alloc(struct inode *inode, const sector_t lblock, > } > > /** > - * gfs2_block_map - Map a block from an inode to a disk block > - * @inode: The inode > - * @lblock: The logical block number > - * @bh_map: The bh to be mapped > - * @create: True if its ok to alloc blocks to satify the request > + * hole_size - figure out the size of a hole > + * @ip: The inode > + * @lblock: The logical starting block number > + * @mp: The metapath > * > - * Sets buffer_mapped() if successful, sets buffer_boundary() if a > - * read of metadata will be required before the next block can be > - * mapped. Sets buffer_new() if new blocks were allocated. > + * Returns: The hole size in bytes > * > - * Returns: errno > */ > +static u64 hole_size(struct inode *inode, sector_t lblock, struct metapath *mp) > +{ > + struct gfs2_inode *ip = GFS2_I(inode); > + struct gfs2_sbd *sdp = GFS2_SB(inode); > + struct metapath mp_eof; > + unsigned int end_of_metadata = ip->i_height - 1; > + u64 factor = 1; > + int hgt = end_of_metadata; > + u64 holesz = 0, holestep; > + const __be64 *first, *end, *ptr; > + const struct buffer_head *bh; > + u64 isize_blks = (i_size_read(inode) - 1) >> inode->i_blkbits; > + int zeroptrs; > + bool done = false; > + > + /* Get another metapath, to the very last byte */ > + find_metapath(sdp, isize_blks, &mp_eof, ip->i_height); > + for (hgt = end_of_metadata; hgt >= 0 && !done; hgt--) { > + bh = mp->mp_bh[hgt]; > + if (bh) { > + zeroptrs = 0; > + first = metapointer(hgt, mp); > + end = (const __be64 *)(bh->b_data + bh->b_size); > + > + for (ptr = first; ptr < end; ptr++) { > + if (*ptr) { > + done = true; > + break; > + } else { > + zeroptrs++; > + } > + } > + } else { > + zeroptrs = sdp->sd_inptrs; > + } > + holestep = min(factor * zeroptrs, > + isize_blks - (lblock + (zeroptrs * holesz))); > + holesz += holestep; > + if (lblock + holesz >= isize_blks) { > + holesz = isize_blks - lblock; > + break; > + } > > -int gfs2_block_map(struct inode *inode, sector_t lblock, > - struct buffer_head *bh_map, int create) > + factor *= sdp->sd_inptrs; > + if (hgt && (mp->mp_list[hgt - 1] < mp_eof.mp_list[hgt - 1])) > + (mp->mp_list[hgt - 1])++; > + } > + return holesz << inode->i_blkbits; > +} > + > +/** > + * gfs2_get_iomap - Map blocks from an inode to disk blocks > + * @mapping: The address space > + * @pos: Starting position in bytes > + * @length: Length to map, in bytes > + * @iomap: The iomap structure > + * @mp: An uninitialized metapath structure > + * @release_mpath: if true, we need to release the metapath when done > + * > + * Returns: errno > + */ > +static int _gfs2_get_iomap(struct inode *inode, loff_t pos, ssize_t length, > + struct iomap *iomap, struct metapath *mp, > + bool release_mpath) > { > struct gfs2_inode *ip = GFS2_I(inode); > struct gfs2_sbd *sdp = GFS2_SB(inode); > unsigned int bsize = sdp->sd_sb.sb_bsize; > - const size_t maxlen = bh_map->b_size >> inode->i_blkbits; > const u64 *arr = sdp->sd_heightsize; > __be64 *ptr; > - u64 size; > - struct metapath mp; > + sector_t lblock = pos >> sdp->sd_sb.sb_bsize_shift; > int ret; > int eob; > unsigned int len; > struct buffer_head *bh; > u8 height; > - bool zero_new = false; > - sector_t dblock; > - unsigned int dblks = 0; > - > - BUG_ON(maxlen == 0); > - > - memset(mp.mp_bh, 0, sizeof(mp.mp_bh)); > - bmap_lock(ip, create); > - clear_buffer_mapped(bh_map); > - clear_buffer_new(bh_map); > - clear_buffer_boundary(bh_map); > - trace_gfs2_bmap(ip, bh_map, lblock, create, 1); > + loff_t isize = i_size_read(inode); > + > + if (length == 0) > + return -EINVAL; > + > + iomap->offset = pos; > + iomap->blkno = IOMAP_NULL_BLOCK; > + iomap->type = IOMAP_HOLE; > + iomap->length = length; > + mp->mp_aheight = 1; > + memset(&mp->mp_bh, 0, sizeof(mp->mp_bh)); > + bmap_lock(ip, 0); > if (gfs2_is_dir(ip)) { > bsize = sdp->sd_jbsize; > arr = sdp->sd_jheightsize; > } > > - ret = gfs2_meta_inode_buffer(ip, &mp.mp_bh[0]); > + ret = gfs2_meta_inode_buffer(ip, &(mp->mp_bh[0])); > if (ret) > goto out; > > height = ip->i_height; > - size = (lblock + 1) * bsize; > - while (size > arr[height]) > + while ((lblock + 1) * bsize > arr[height]) > height++; > - find_metapath(sdp, lblock, &mp, height); > - mp.mp_aheight = 1; > + find_metapath(sdp, lblock, mp, height); > if (height > ip->i_height || gfs2_is_stuffed(ip)) > - goto do_alloc; > - ret = lookup_metapath(ip, &mp); > + goto out; > + > + ret = lookup_metapath(ip, mp); > if (ret < 0) > goto out; > - if (mp.mp_aheight != ip->i_height) > - goto do_alloc; > - ptr = metapointer(ip->i_height - 1, &mp); > - if (*ptr == 0) > - goto do_alloc; > - map_bh(bh_map, inode->i_sb, be64_to_cpu(*ptr)); > - bh = mp.mp_bh[ip->i_height - 1]; > - len = gfs2_extent_length(bh->b_data, bh->b_size, ptr, maxlen, &eob); > - bh_map->b_size = (len << inode->i_blkbits); > - if (eob) > - set_buffer_boundary(bh_map); > + > ret = 0; > + if (mp->mp_aheight != ip->i_height) { > + iomap->length = hole_size(inode, lblock, mp); > + goto out; > + } > + > + ptr = metapointer(ip->i_height - 1, mp); > + if (*ptr) { > + iomap->type = IOMAP_MAPPED; > + iomap->blkno = be64_to_cpu(*ptr); > + } > + > + bh = mp->mp_bh[ip->i_height - 1]; > + len = gfs2_extent_length(bh->b_data, bh->b_size, ptr, > + length >> inode->i_blkbits, &eob); > + iomap->length = len << sdp->sd_sb.sb_bsize_shift; > + /* If we go past eof, round up to the nearest block */ > + if (iomap->offset + iomap->length >= isize) > + iomap->length = round_up((isize - iomap->offset), bsize); > + > out: > - release_metapath(&mp); > - trace_gfs2_bmap(ip, bh_map, lblock, create, ret); > - bmap_unlock(ip, create); > + if (ret || release_mpath) > + release_metapath(mp); > + bmap_unlock(ip, 0); > return ret; > +} > + > +int gfs2_get_iomap(struct inode *inode, loff_t pos, ssize_t length, > + struct iomap *iomap) > +{ > + struct metapath mp; > + > + return _gfs2_get_iomap(inode, pos, length, iomap, &mp, true); > +} > + > +/** > + * gfs2_block_map - Map a block from an inode to a disk block > + * @inode: The inode > + * @lblock: The logical block number > + * @bh_map: The bh to be mapped > + * @create: True if its ok to alloc blocks to satify the request > + * > + * Sets buffer_mapped() if successful, sets buffer_boundary() if a > + * read of metadata will be required before the next block can be > + * mapped. Sets buffer_new() if new blocks were allocated. > + * > + * Returns: errno > + */ > > -do_alloc: > - /* All allocations are done here, firstly check create flag */ > - if (!create) { > - BUG_ON(gfs2_is_stuffed(ip)); > - ret = 0; > +int gfs2_block_map(struct inode *inode, sector_t lblock, > + struct buffer_head *bh_map, int create) > +{ > + struct gfs2_inode *ip = GFS2_I(inode); > + struct gfs2_sbd *sdp = GFS2_SB(inode); > + const size_t maxlen = bh_map->b_size >> inode->i_blkbits; > + struct metapath mp; > + struct iomap iomap; > + sector_t dblock; > + unsigned int dblks; > + bool zero_new = false; > + int ret; > + > + BUG_ON(maxlen == 0); > + > + clear_buffer_mapped(bh_map); > + clear_buffer_new(bh_map); > + clear_buffer_boundary(bh_map); > + trace_gfs2_bmap(ip, bh_map, lblock, create, 1); > + > + /* If we're asked to alloc any missing blocks, we need to preserve > + * the metapath buffers and release them ourselves. */ > + ret = _gfs2_get_iomap(inode, lblock << sdp->sd_sb.sb_bsize_shift, > + bh_map->b_size, &iomap, &mp, false); > + > + bh_map->b_size = iomap.length; > + if (maxlen >= iomap.length) > + set_buffer_boundary(bh_map); > + > + if (ret) > goto out; > - } > > - /* At this point ret is the tree depth of already allocated blocks */ > - if (buffer_zeronew(bh_map)) > - zero_new = true; > - ret = gfs2_bmap_alloc(inode, lblock, zero_new, &mp, maxlen, &dblock, > - &dblks); > - if (ret == 0) { > - map_bh(bh_map, inode->i_sb, dblock); > - bh_map->b_size = dblks << inode->i_blkbits; > - set_buffer_new(bh_map); So far, so good I think... > + if (iomap.type == IOMAP_MAPPED) { > + map_bh(bh_map, inode->i_sb, iomap.blkno); > + bh_map->b_size = iomap.length; > + } else if (create) { > + if (buffer_zeronew(bh_map)) > + zero_new = true; > + ret = gfs2_bmap_alloc(inode, lblock, zero_new, &mp, maxlen, > + &dblock, &dblks); however gfs2_bmap_alloc() should take the struct iomap and not the individual parameters. Also why is this call to gfs2_bmap_alloc() done here rather than in gfs2_get_iomap() ? That way there is no need to pass the mp around. The zero_new flag should just become an iomap flag too, and that should clean this bit up. I think that gfs2_block_map() should just be a wrapper for the new gfs2_get_iomap() function and not try to bypass it for the create case, Steve. > + if (ret == 0) { > + map_bh(bh_map, inode->i_sb, dblock); > + bh_map->b_size = dblks << inode->i_blkbits; > + set_buffer_new(bh_map); > + } > } > - goto out; > + release_metapath(&mp); > +out: > + trace_gfs2_bmap(ip, bh_map, lblock, create, ret); > + return ret; > } > > /* > diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h > index 81ded5e..8da2429 100644 > --- a/fs/gfs2/bmap.h > +++ b/fs/gfs2/bmap.h > @@ -10,6 +10,8 @@ > #ifndef __BMAP_DOT_H__ > #define __BMAP_DOT_H__ > > +#include > + > #include "inode.h" > > struct inode; > @@ -47,6 +49,8 @@ static inline void gfs2_write_calc_reserv(const struct gfs2_inode *ip, > extern int gfs2_unstuff_dinode(struct gfs2_inode *ip, struct page *page); > extern int gfs2_block_map(struct inode *inode, sector_t lblock, > struct buffer_head *bh, int create); > +extern int gfs2_get_iomap(struct inode *inode, loff_t pos, > + ssize_t length, struct iomap *iomap); > extern int gfs2_extent_map(struct inode *inode, u64 lblock, int *new, > u64 *dblock, unsigned *extlen); > extern int gfs2_setattr_size(struct inode *inode, u64 size);