From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751267AbdJBMZO (ORCPT <rfc822;w@1wt.eu>);
        Mon, 2 Oct 2017 08:25:14 -0400
Received: from mx2.mpynet.fi ([82.197.21.85]:23859 "EHLO mx2.mpynet.fi"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1750957AbdJBMZN (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 2 Oct 2017 08:25:13 -0400
Date: Mon, 2 Oct 2017 15:25:11 +0300
From: Rakesh Pandit <rakesh@tuxera.com>
To: Javier =?iso-8859-1?Q?Gonz=E1lez?= <jg@lightnvm.io>
CC: Matias =?iso-8859-1?Q?Bj=F8rling?= <mb@lightnvm.io>,
        <linux-block@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 5/6] lightnvm: pblk: free up mempool allocation for
 erases correctly
Message-ID: <20171002122510.GA3946@hercules.tuxera.com>
References: <20171001132555.GA5763@hercules.tuxera.com>
 <5E8B439C-5971-49DF-BDC4-3B53268F8FF4@lightnvm.io>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <5E8B439C-5971-49DF-BDC4-3B53268F8FF4@lightnvm.io>
User-Agent: Mutt/1.8.0 (2017-02-23)
X-ClientProxiedBy: tuxera-exch.ad.tuxera.com (10.20.48.11) To
 tuxera-exch.ad.tuxera.com (10.20.48.11)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Oct 02, 2017 at 02:09:35PM +0200, Javier González wrote:
> > On 1 Oct 2017, at 15.25, Rakesh Pandit <rakesh@tuxera.com> wrote:
> > 
> > While separating read and erase mempools in 22da65a1b pblk_g_rq_cache
> > was used two times to set aside memory both for erase and read
> > requests.  Because same kmem cache is used repeatedly a single call to
> > kmem_cache_destroy wouldn't deallocate everything.  Repeatedly doing
> > loading and unloading of pblk modules would eventually result in some
> > leak.
> > 
> > The fix is to really use separate kmem cache and track it
> > appropriately.
> > 
> > Fixes: 22da65a1b ("lightnvm: pblk: decouple read/erase mempools")
> > Signed-off-by: Rakesh Pandit <rakesh@tuxera.com>
> > 
> 
> I'm not sure I follow this logic. I assume that you're thinking of the
> refcount on kmem_cache. During cache creation, all is good; if a
> different cache creation fails, destruction is guaranteed, since the
> refcount is 0. On tear down (pblk_core_free), we destroy the mempools
> associated to the caches. In this case, the refcount goes to 0 too, as
> we destroy the 2 mempools. So I don't see where the leak can happen. Am
> I missing something?
> 
> In any case, Jens reported some bugs on the mempools, where we did not
> guarantee forward progress. Here you can find the original discussion
> and the mempool audit [1]. Would be good if you reviewed these.
> 
> [1] https://www.spinics.net/lists/kernel/msg2602274.html
> 

Thanks, yes makes sense to follow up in patch thread.  I will respond
to above questions there later today.