On 6 April 2018 at 16:06, Bob Peterson <rpeterso@xxxxxxxxxx> wrote:
> 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)
That's certainly possible as well.
Thanks,
Andreas
