Re: question about xfs_alloc_fix_freelist()

[Michael Nishimoto <miken@xxxxxxxxx>]

David Chinner wrote:
> On Fri, May 02, 2008 at 02:01:47PM -0700, Michael Nishimoto wrote:
>  >
>  > The following code can be found near the end of xfs_alloc_fix_freelist:
>  >
>  >                 if (targs.agbno == NULLAGBLOCK) {
>  >                         if (flags & XFS_ALLOC_FLAG_FREEING)
>  >                                 break;
>  >                         xfs_trans_brelse(tp, agflbp);
>  >                         args->agbp = NULL;
>  >                         return 0;
>  >                 }
>  >
>  > Don't we need to release agbp too by calling xfs_trans_brelse(tp, agbp)?
>
> I don't think so. AFAICT, The agbp (agf block) is linked into the
> transaction and by this point we may have modified the AGF (think
> multiple iterations of the loop to fill the free list). Given that
> it may be modified, we shouldn't release it here but instead allow
> the transaction commit/abort to do that for us at the appropriate
> time.
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> Principal Engineer
> SGI Australian Software Group

I understood that the transaction abort code will free resources/locks
when passing back an error.  However, by returning args->agbp = NULL,
aren't we telling the calling code that we have not satisfied the request
in the current AG?

I do see the part of about the AGF possibly getting modified.

It would help to have a clear comment description of possible inputs vs.
expected outputs for this function. :-)

   thanks,

       Michael