xfs
[Top] [All Lists]

Re: [PATCH 2/2] dio: scale unaligned IO tracking via multiple lists

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/2] dio: scale unaligned IO tracking via multiple lists
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 2 Aug 2010 05:32:59 -0400
Cc: linux-fsdevel@xxxxxxxxxxxxxxx, sandeen@xxxxxxxxxxx, xfs@xxxxxxxxxxx
In-reply-to: <1280733945-16231-3-git-send-email-david@xxxxxxxxxxxxx>
References: <1280733945-16231-1-git-send-email-david@xxxxxxxxxxxxx> <1280733945-16231-3-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-08-17)
On Mon, Aug 02, 2010 at 05:25:45PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> To avoid concerns that a single list and lock tracking the unaligned IOs
> will not scale appropriately, create multiple lists and locks and chose them 
> by
> hashing the unaligned block being zeroed.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
>  fs/direct-io.c |   53 ++++++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 40 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 611524e..95dcba4 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -152,8 +152,29 @@ struct dio_zero_block {
>       atomic_t        ref;            /* reference count */
>  };
>  
> -DEFINE_SPINLOCK(dio_zero_block_lock);
> -LIST_HEAD(dio_zero_block_list);
> +#define DIO_ZERO_BLOCK_NR    37LL
> +struct dio_zero_block_head {
> +     struct list_head        list;
> +     spinlock_t              lock;
> +};
> +
> +struct dio_zero_block_head dio_zero_blocks[DIO_ZERO_BLOCK_NR];

Again, should be static.

> +#define to_dio_zero_list(zb) (&dio_zero_blocks[zb % DIO_ZERO_BLOCK_NR].list)
> +#define to_dio_zero_lock(zb) (&dio_zero_blocks[zb % DIO_ZERO_BLOCK_NR].lock)

> +     struct list_head *list  = to_dio_zero_list(zero_block);
> +     spinlock_t *lock = to_dio_zero_lock(zero_block);

What about just finding the dio_zero_block_head and stuffing it into
a local variable?  Probably doesn't matter in the end anyway.

Also looks good.

If people really care about making this scale better we could also make
it per-sb. 

One really big problem is that no one tells the users that unaligned
direct I/O is a performance problem.  We should add a variant of the
XFS DIOINFO ioctl to the core VFS, including minimally required and
optimal alignment values.

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