Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 23 Aug 2022 08:57:58 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     SeongJae Park <sj@kernel.org>
Cc:     akpm@linux-foundation.org, badari.pulavarty@intel.com,
        damon@lists.linux.dev, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH v3] mm/damon/dbgfs: avoid duplicate context directory
 creation
Message-ID: <YwR6dgz1LEWGQagq@kroah.com>
References: <YwMhLHNfc7Fcdrd5@kroah.com>
 <20220822165236.87421-1-sj@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220822165236.87421-1-sj@kernel.org>
Precedence: bulk

On Mon, Aug 22, 2022 at 04:52:36PM +0000, SeongJae Park wrote:
> > >  	new_dir = debugfs_create_dir(name, root);
> > > +	/* Below check is required for a potential duplicated name case */
> > > +	if (IS_ERR(new_dir))
> > > +		return PTR_ERR(new_dir);
> > 
> > Did you just leak the memory allocated above this check?  It's hard to
> > determine as you are setting global variables.
> 
> We re-alloc the metadata arrays for context above for this new context, and we
> do not re-alloc those in this failure case.  So yes, the arrays will have one
> more item that not really needed and also not really will be used.
> 
> However, the variable for the array, 'dbgfs_nr_ctxs' is not increased here.
> Therefore, the arrays will be re-allocated to the proper size when this
> function or other function that re-alloc the arrays based on 'dbgfs_nr_ctxs'
> (For example, 'dbgfs_rm_context()') are called.
> 
> So, though the arrays could have not-really-needed one entry that is only waste
> of memory, as it's only a small and fixed amount of memory (just one more
> pointer), and as the unneeded memory will eventually be returned (by a next
> 'dbgfs_{mk,rm}_context()' call), I think that's no problem.  This is what I
> intended for keeping the logic simple.
> 
> If I'm missing something, please let me know, though.

Ah, that makes more sense, thanks.  The code was not obvious in that
error paths normally clean up allocations that were done earlier.

All is good.

thanks,

greg k-h