On Tue, 2011-03-22 at 15:55 -0400, Christoph Hellwig wrote:
> Allow reusing any busy extent for metadata allocations, and reusing busy
> userdata extents for userdata allocations. Most of the complexity is
> propagating the userdata information from the XFS_BMAPI_METADATA flag
> to xfs_bunmapi into the low-level extent freeing routines. After that
> we can just track what type of busy extent we have and treat it accordingly.
Why is it OK to reuse user data extents for user data allocations?
I accept it is, I just haven't worked through in my mind why.
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>
> Index: xfs/fs/xfs/xfs_alloc.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_alloc.c 2011-03-21 14:49:14.000000000 +0100
> +++ xfs/fs/xfs/xfs_alloc.c 2011-03-21 14:51:31.746155282 +0100
> @@ -1396,7 +1396,8 @@ xfs_alloc_ag_vextent_small(
> if (error)
> goto error0;
> if (fbno != NULLAGBLOCK) {
> - xfs_alloc_busy_reuse(args->tp, args->agno, fbno, 1);
> + xfs_alloc_busy_reuse(args->tp, args->agno, fbno, 1,
> + args->userdata);
>
> if (args->userdata) {
> xfs_buf_t *bp;
> @@ -2431,7 +2432,8 @@ int /* error */
> xfs_free_extent(
> xfs_trans_t *tp, /* transaction pointer */
> xfs_fsblock_t bno, /* starting block number of extent */
> - xfs_extlen_t len) /* length of extent */
> + xfs_extlen_t len,
> + bool userdata)/* length of extent */
xfs_extlen_t len, /* length of extent */
bool userdata)
> {
> xfs_alloc_arg_t args;
> int error;
. . .
> @@ -2717,7 +2723,7 @@ restart: (in xfs_alloc_busy_reuse())
>
> overlap = xfs_alloc_busy_try_reuse(pag, busyp,
> fbno, fbno + flen);
> - if (overlap) {
> + if (overlap == -1 || (overlap && userdata)) {
xfs_alloc_busy_try_reuse() (still) never returns non-zero,
so this could just be:
if (overlap == -1 || userdata) {
I understand why we can skip forcing the log if
we're not doing a userdata allocation. But why
don't you also check busyp->flags here when it's
a userdata allocation, to see if it represents a
busy userdata section and therefore would allow
avoiding the log force (like is done below in
xfs_alloc_busy_trim())? You would have to grab
the flag value in busyp before the call.
> spin_unlock(&pag->pagb_lock);
> xfs_log_force(tp->t_mountp, XFS_LOG_SYNC);
> goto restart;
> @@ -2754,6 +2760,7 @@ xfs_alloc_busy_trim(
>
> ASSERT(flen > 0);
>
> +restart:
> spin_lock(&args->pag->pagb_lock);
> rbp = args->pag->pagb_tree.rb_node;
> while (rbp && flen >= args->minlen) {
> @@ -2771,6 +2778,31 @@ xfs_alloc_busy_trim(
> continue;
> }
>
> + if (!args->userdata ||
> + (busyp->flags & XFS_ALLOC_BUSY_USERDATA)) {
> + int overlap;
> +
> + overlap = xfs_alloc_busy_try_reuse(args->pag, busyp,
> + fbno, fbno + flen);
> + if (unlikely(overlap == -1)) {
> + spin_unlock(&args->pag->pagb_lock);
> + xfs_log_force(args->mp, XFS_LOG_SYNC);
> + goto restart;
> + }
> +
> + /*
> + * No more busy extents to search.
> + */
> + if (bbno <= fbno && bend >= fend)
> + goto out;
> +
> + if (fbno < bbno)
> + rbp = rbp->rb_left;
> + else
> + rbp = rbp->rb_right;
> + continue;
> + }
> +
> if (bbno <= fbno) {
> /* start overlap */
>
. . .
> Index: xfs/fs/xfs/xfs_ag.h
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_ag.h 2011-03-21 14:48:04.000000000 +0100
> +++ xfs/fs/xfs/xfs_ag.h 2011-03-21 14:49:21.941981228 +0100
. . .
> @@ -3750,6 +3744,7 @@ xfs_bmap_add_free(
> new = kmem_zone_alloc(xfs_bmap_free_item_zone, KM_SLEEP);
> new->xbfi_startblock = bno;
> new->xbfi_blockcount = (xfs_extlen_t)len;
> + new->xbfi_flags = XFS_BFI_USERDATA;
Couldn't you arrange for the the xfs_bmbt_free_block() path
to *not* set this? (As it stands, it will always be set.)
> for (prev = NULL, cur = flist->xbf_first;
> cur != NULL;
> prev = cur, cur = cur->xbfi_next) {
. . .
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
