| Message ID | 1485715421-17182-2-git-send-email-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Sun, Jan 29, 2017 at 07:43:38PM +0100, Christoph Hellwig wrote: > We don't just need the structure to track busy extents which can be > avoided with a synchronous transaction, but also to keep track of > pending discard. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- Looks fine, though I wonder if we should create a kmem_cache similar to all of the other log item structures and whatnot... Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/xfs_extent_busy.c | 13 +------------ > 1 file changed, 1 insertion(+), 12 deletions(-) > > diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c > index 162dc18..29c2f99 100644 > --- a/fs/xfs/xfs_extent_busy.c > +++ b/fs/xfs/xfs_extent_busy.c > @@ -45,18 +45,7 @@ xfs_extent_busy_insert( > struct rb_node **rbp; > struct rb_node *parent = NULL; > > - new = kmem_zalloc(sizeof(struct xfs_extent_busy), KM_MAYFAIL); > - if (!new) { > - /* > - * No Memory! Since it is now not possible to track the free > - * block, make this a synchronous transaction to insure that > - * the block is not reused before this transaction commits. > - */ > - trace_xfs_extent_busy_enomem(tp->t_mountp, agno, bno, len); > - xfs_trans_set_sync(tp); > - return; > - } > - > + new = kmem_zalloc(sizeof(struct xfs_extent_busy), KM_SLEEP); > new->agno = agno; > new->bno = bno; > new->length = len; > -- > 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 -- 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
On Fri, Feb 03, 2017 at 10:20:52AM -0500, Brian Foster wrote: > On Sun, Jan 29, 2017 at 07:43:38PM +0100, Christoph Hellwig wrote: > > We don't just need the structure to track busy extents which can be > > avoided with a synchronous transaction, but also to keep track of > > pending discard. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > --- > > Looks fine, though I wonder if we should create a kmem_cache similar to > all of the other log item structures and whatnot... Using the isolation of a slab cache for such a short lived structure seems counter productive. -- 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
On Sat, Feb 04, 2017 at 10:50:40AM +0100, Christoph Hellwig wrote: > On Fri, Feb 03, 2017 at 10:20:52AM -0500, Brian Foster wrote: > > On Sun, Jan 29, 2017 at 07:43:38PM +0100, Christoph Hellwig wrote: > > > We don't just need the structure to track busy extents which can be > > > avoided with a synchronous transaction, but also to keep track of > > > pending discard. > > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > --- > > > > Looks fine, though I wonder if we should create a kmem_cache similar to > > all of the other log item structures and whatnot... > > Using the isolation of a slab cache for such a short lived structure > seems counter productive. I was thinking more about the repeated allocation/free of said structures than lifetime, particularly since we've converted an opportunistic allocation to a required/sleeping one. Just a thought though.. looking again, should we have KM_NOFS here as well? Brian > -- > 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 -- 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
On Mon, Feb 06, 2017 at 11:43:18AM -0500, Brian Foster wrote: > I was thinking more about the repeated allocation/free of said > structures than lifetime, particularly since we've converted an > opportunistic allocation to a required/sleeping one. The allocator is one and the same - the different is that kmalloc does a trivial lookup of the cache to use first, while kmem_cache_alloc specifies an exact cache. > Just a thought though.. looking again, should we have KM_NOFS here as > well? xfs_extent_busy_insert is always called inside transaction context, so we get implicit NOFS semantics. The existing code already relies on that. -- 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
diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c index 162dc18..29c2f99 100644 --- a/fs/xfs/xfs_extent_busy.c +++ b/fs/xfs/xfs_extent_busy.c @@ -45,18 +45,7 @@ xfs_extent_busy_insert( struct rb_node **rbp; struct rb_node *parent = NULL; - new = kmem_zalloc(sizeof(struct xfs_extent_busy), KM_MAYFAIL); - if (!new) { - /* - * No Memory! Since it is now not possible to track the free - * block, make this a synchronous transaction to insure that - * the block is not reused before this transaction commits. - */ - trace_xfs_extent_busy_enomem(tp->t_mountp, agno, bno, len); - xfs_trans_set_sync(tp); - return; - } - + new = kmem_zalloc(sizeof(struct xfs_extent_busy), KM_SLEEP); new->agno = agno; new->bno = bno; new->length = len;
We don't just need the structure to track busy extents which can be avoided with a synchronous transaction, but also to keep track of pending discard. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_extent_busy.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-)