linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
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
Date: Tue, 23 Aug 2022 08:57:58 +0200	[thread overview]
Message-ID: <YwR6dgz1LEWGQagq@kroah.com> (raw)
In-Reply-To: <20220822165236.87421-1-sj@kernel.org>

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


      reply	other threads:[~2022-08-23  6:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-21 18:08 SeongJae Park
2022-08-22  6:24 ` Greg KH
2022-08-22 16:52   ` SeongJae Park
2022-08-23  6:57     ` Greg KH [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YwR6dgz1LEWGQagq@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=badari.pulavarty@intel.com \
    --cc=damon@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=sj@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox