Re: [patch] btt: fix uninitialized err_lock

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Wed, 2017-12-13 at 16:33 -0500, Jeff Moyer wrote:
> Hi,
> 
> When a sector mode namespace is initially created, the arena's
> err_lock
> is not initialized.  If, on the other hand, the namespace already
> exists, the mutex is initialized.  To fix the issue, I moved the
> mutex
> initialization into the arena_alloc, which is called by both
> discover_arenas and create_arenas.
> 
> This was discovered on an older kernel where mutex_trylock checks the
> count to determine whether the lock is held.  Because the data
> structure
> is kzalloc-d, that count was 0 (held), and I/O to the device would
> hang
> forever waiting for the lock to be released (see btt_write_pg, for
> example).  Current kernels have a different mutex implementation that
> checks for a non-null owner, and so this doesn't show up as a
> problem.
> If that lock were ever contended, it might cause issues, but you'd
> have
> to be really unlucky, I think.
> 
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>

Ah, good find - the fix looks good to me.
Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>

> 
> diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
> index e949e33..5860f99 100644
> --- a/drivers/nvdimm/btt.c
> +++ b/drivers/nvdimm/btt.c
> @@ -630,6 +630,7 @@ static struct arena_info *alloc_arena(struct btt
> *btt, size_t size,
>  		return NULL;
>  	arena->nd_btt = btt->nd_btt;
>  	arena->sector_size = btt->sector_size;
> +	mutex_init(&arena->err_lock);
>  
>  	if (!size)
>  		return arena;
> @@ -758,7 +759,6 @@ static int discover_arenas(struct btt *btt)
>  		arena->external_lba_start = cur_nlba;
>  		parse_arena_meta(arena, super, cur_off);
>  
> -		mutex_init(&arena->err_lock);
>  		ret = btt_freelist_init(arena);
>  		if (ret)
>  			goto out;
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm