Hi,

----- Original Message -----
(snip)
> +static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
> +                         unsigned flags, struct iomap *iomap)
> +{
> +     struct gfs2_inode *ip = GFS2_I(inode);
> +     struct metapath mp = { .mp_aheight = 1, };
> +     int ret;
> +
> +     trace_gfs2_iomap_start(ip, pos, length, flags);
> +     if (flags & IOMAP_WRITE) {
> +             ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
> +             if (!ret && iomap->type == IOMAP_HOLE)
> +                     ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
> +             release_metapath(&mp);
> +     } else {
> +             ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
> +             release_metapath(&mp);
> +     }

Why the redundancy? Why not just move gfs2_iomap_get before the if, and
release_metapath after the if? Something like:
+       ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
+
+       if (!ret && flags & IOMAP_WRITE && iomap->type == IOMAP_HOLE))
+               ret = gfs2_iomap_alloc(inode, iomap, flags, &mp);
+
+       release_metapath(&mp);
(snip)

Bob Peterson

Reply via email to