From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id DEEC72034B9C0 for ; Wed, 13 Dec 2017 13:34:00 -0800 (PST) From: "Verma, Vishal L" Subject: Re: [patch] btt: fix uninitialized err_lock Date: Wed, 13 Dec 2017 21:38:39 +0000 Message-ID: <1513201117.3301.127.camel@intel.com> References: In-Reply-To: Content-Language: en-US Content-ID: MIME-Version: 1.0 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" To: "Williams, Dan J" , "jmoyer@redhat.com" Cc: "linux-nvdimm@lists.01.org" List-ID: 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 Ah, good find - the fix looks good to me. Reviewed-by: Vishal Verma > > 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