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

[Christoph Hellwig <hch@infradead.org>]

On Mon, Aug 02, 2010 at 05:25:45PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> 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@redhat.com>
> ---
>  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.