[Top] [All Lists]

Re: [PATCH 3/4] xfs: add a reference count to the CIL context

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 3/4] xfs: add a reference count to the CIL context
From: Alex Elder <aelder@xxxxxxx>
Date: Thu, 19 May 2011 16:54:54 -0500
Cc: <xfs@xxxxxxxxxxx>
In-reply-to: <20110504190011.508745503@xxxxxxxxxxxxxxxxxxxxxx>
References: <20110504185513.136746538@xxxxxxxxxxxxxxxxxxxxxx> <20110504190011.508745503@xxxxxxxxxxxxxxxxxxxxxx>
Reply-to: <aelder@xxxxxxx>
On Wed, 2011-05-04 at 14:55 -0400, Christoph Hellwig wrote:
> plain text document attachment (xfs-cil-ctx-refcounting)
> For the upcoming asynchronoyus discard support we need to be able to delay
> freeing the CIL context until the last discard request that reference it
> has completed.  Add a reference count to the CIL context, and only clear
> the busy extents and free the CIL context structure when it reaches zero,
> and a work item to allow freeing it from irq context.
> Note that this means delaying the clearing of the busy extents for a little
> bit even on non-discard mounts, but with the new busy extent trim/reuse
> code there is no real life impact of this change.

It will affect allocation patterns but it's really hard to say
whether it will make them worse or better...

> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

This looks good.  Encapsulating the context initialization
was a good step even without the new work item field.  One
little thing below.

Reviewed-by: Alex Elder <aelder@xxxxxxx>

PS  I've run out of time and will have to review the
    fourth patch in this series tomorrow.

> Index: xfs/fs/xfs/xfs_log_cil.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_log_cil.c     2011-05-03 12:00:49.000000000 +0200
> +++ xfs/fs/xfs/xfs_log_cil.c  2011-05-03 12:17:19.399350953 +0200
> @@ -20,7 +20,7 @@
>  #include "xfs_types.h"
>  #include "xfs_bit.h"
>  #include "xfs_log.h"
> -#include "xfs_inum.h"
> +#include "xfs_inum.h" 
>  #include "xfs_trans.h"
>  #include "xfs_trans_priv.h"
>  #include "xfs_log_priv.h"

Kill this hunk.  (You added a space at end-of-line.)

. . .

<Prev in Thread] Current Thread [Next in Thread>