From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:58620 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753520AbcLNSZC (ORCPT ); Wed, 14 Dec 2016 13:25:02 -0500 Date: Wed, 14 Dec 2016 13:24:32 -0500 From: Brian Foster Subject: Re: [PATCH 3/4] xfs: adjust allocation length in xfs_alloc_space_available Message-ID: <20161214182432.GC24645@bfoster.bfoster> References: <1481644767-9098-1-git-send-email-hch@lst.de> <1481644767-9098-4-git-send-email-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1481644767-9098-4-git-send-email-hch@lst.de> Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Christoph Hellwig Cc: linux-xfs@vger.kernel.org, eguan@redhat.com, darrick.wong@oracle.com On Tue, Dec 13, 2016 at 04:59:26PM +0100, Christoph Hellwig wrote: > We must decide in xfs_alloc_fix_freelist if we can perform an > allocation from a given AG is possible or not based on the available > space, and should not fail the allocation past that point on a > healthy file system. > > But currently we have two additional places that second-guess > xfs_alloc_fix_freelist: xfs_alloc_ag_vextent tries to adjust the > maxlen parameter to remove the reservation before doing the > allocation (but ignores the various minium freespace requirements), > and xfs_alloc_fix_minleft tries to fix up the allocated length > after we've found an extent, but ignores the reservations and also > doesn't take the AGFL into account (and thus fails allocations > for not matching minlen in some cases). > > Remove all these later fixups and just correct the maxlen argument > inside xfs_alloc_fix_freelist once we have the AGF buffer locked. > > Signed-off-by: Christoph Hellwig > --- > fs/xfs/libxfs/xfs_alloc.c | 78 +++++++++-------------------------------------- > fs/xfs/libxfs/xfs_alloc.h | 2 +- > 2 files changed, 16 insertions(+), 64 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c > index 91a6c17..87328a8 100644 > --- a/fs/xfs/libxfs/xfs_alloc.c > +++ b/fs/xfs/libxfs/xfs_alloc.c ... > @@ -2073,10 +2015,19 @@ xfs_alloc_space_available( > > /* do we have enough free space remaining for the allocation? */ > available = (int)(pag->pagf_freeblks + pag->pagf_flcount - > - reservation - min_free - args->total); > - if (available < (int)args->minleft || available <= 0) > + reservation - min_free - args->minleft); > + if (available < (int)args->total) > return false; > > + /* > + * Clamp maxlen to the amount of free space available for the actual > + * extent allocation. > + */ > + if (available < (int)args->maxlen && !(flags & XFS_ALLOC_FLAG_CHECK)) { > + args->maxlen = available; > + ASSERT(args->maxlen > 0); Nit: could we assert args->maxlen >= args->minlen here as well? Otherwise looks good to me: Reviewed-by: Brian Foster > + } > + > return true; > } > > @@ -2122,7 +2073,8 @@ xfs_alloc_fix_freelist( > } > > need = xfs_alloc_min_freelist(mp, pag); > - if (!xfs_alloc_space_available(args, need, flags)) > + if (!xfs_alloc_space_available(args, need, flags | > + XFS_ALLOC_FLAG_CHECK)) > goto out_agbp_relse; > > /* > diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h > index 7c404a6..1d0f48a 100644 > --- a/fs/xfs/libxfs/xfs_alloc.h > +++ b/fs/xfs/libxfs/xfs_alloc.h > @@ -56,7 +56,7 @@ typedef unsigned int xfs_alloctype_t; > #define XFS_ALLOC_FLAG_FREEING 0x00000002 /* indicate caller is freeing extents*/ > #define XFS_ALLOC_FLAG_NORMAP 0x00000004 /* don't modify the rmapbt */ > #define XFS_ALLOC_FLAG_NOSHRINK 0x00000008 /* don't shrink the freelist */ > - > +#define XFS_ALLOC_FLAG_CHECK 0x00000010 /* test only, don't modify args */ > > /* > * Argument structure for xfs_alloc routines. > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html