On 6 April 2018 at 16:06, Bob Peterson <rpete...@redhat.com> 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

Reply via email to