[Cluster-devel] [PATCH v2 2/6] gfs2: Clean up gfs2_adjust_reservation

[Andreas Gruenbacher <agruenba@redhat.com>]

On Mon, 3 Dec 2018 at 15:32, Bob Peterson <rpeterso@redhat.com> wrote:
>
> ----- Original Message -----
> > Unconditionally call gfs2_adjust_reservation in gfs2_alloc_blocks.  Move
> > the code for updating rd_free and rd_free_clone from gfs2_alloc_blocks
> > into gfs2_adjust_reservation.
> >
> > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> > ---
> >  fs/gfs2/rgrp.c | 26 ++++++++++++++++----------
> >  1 file changed, 16 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> > index bbcf8b8b2597f..733e21cd4cf25 100644
> > --- a/fs/gfs2/rgrp.c
> > +++ b/fs/gfs2/rgrp.c
> (snip)
> > -     if (gfs2_rs_active(&ip->i_res))
> > -             gfs2_adjust_reservation(ip, &rbm, *nblocks);
> > +     error = gfs2_adjust_reservation(ip, &rbm, *nblocks);
>
> Wait. At what point do we require a reservation for all block allocations?
> In today's code, a reservation is not required, so the check for
> "if (gfs2_rs_active(&ip->i_res))" is necessary because you can't adjust a
> reservation that's not active (in the tree)...being within the tree
> determines whether the reservation is active or not.
>
> Yes, I'm looking forward to the day when we require a reservation for all
> block allocations as per my original version (this needs to happen to allow
> rgrp sharing between processes.) Perhaps a future patch will add the
> requirement, but at this point, there's no such requirement, which means
> this patch will probably break during a git bisect, no?

gfs2_adjust_reservation already checks if there is an active
reservation, so the duplicate check here is unnecessary.

> We only get into this situation when all bitmaps are nearly full or fragmented
> and therefore no reservation will fit the minimum requirements, but we still
> need to allow small one-shot block allocations. So we need to do lots of
> full-bitmap testing. Or else we need to add this patch only after the
> patch that makes reservations mandatory.
>
> Regards,
> Bob Peterson

Andreas