All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Tobin C. Harding" <me@tobin.cc>
To: Petr Mladek <pmladek@suse.com>
Cc: "Tobin C. Harding" <tobin@kernel.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Jiri Kosina <jikos@kernel.org>, Miroslav Benes <mbenes@suse.cz>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] livepatch: Use correct kobject cleanup function
Date: Wed, 1 May 2019 07:37:30 +1000	[thread overview]
Message-ID: <20190430213730.GC9454@eros.localdomain> (raw)
In-Reply-To: <20190430150811.4hzhtz4w46o6numh@pathway.suse.cz>

On Tue, Apr 30, 2019 at 05:08:11PM +0200, Petr Mladek wrote:
> On Tue 2019-04-30 10:15:34, Tobin C. Harding wrote:
> > The correct cleanup function after a call to kobject_init_and_add() has
> > succeeded is kobject_del() _not_ kobject_put().  kobject_del() calls
> > kobject_put().
> 
> Really? I see only kobject_put(kobj->parent) in kobject_del.
> It decreases a reference of the _parent_ object and not
> the given one.

Thanks Petr, you are right.  I misread kobject_del().  The story
thickens, so we need to call kobject_del() AND kobject_put().

> Also the section "Kobject removal" in Documentation/kobject.txt
> says that kobject_del() is for two-stage removal. kobject_put()
> still needs to get called at a later time.

Is this call sequence above what is meant by 'two-stage removal', I
didn't really understand that bit of the docs (and I almost always just
assume docs are stale and take them as a hint only :)

> IMHO, this patch causes that kobject_put() would never get called.

I'll do a v2 of this one and re-check all the patches on this I've
already sent (including the docs ones).

> That said, we could probably make the removal a bit cleaner
> by using kobject_del() in klp_free_patch_start() and
> kobject_put() in klp_free_patch_finish(). But I have
> to think more about it.

Noted, thanks for your review.

	Tobin
	

      reply	other threads:[~2019-04-30 21:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-30  0:15 [PATCH 0/2] livepatch: Fix usage of kobject_init_and_add() Tobin C. Harding
2019-04-30  0:15 ` [PATCH 1/2] livepatch: Fix kobject memleak Tobin C. Harding
2019-04-30  8:42   ` Greg Kroah-Hartman
2019-04-30 10:44     ` Miroslav Benes
2019-04-30 10:46       ` Greg Kroah-Hartman
2019-04-30 22:39       ` Tobin C. Harding
2019-04-30 14:56   ` Petr Mladek
2019-04-30 15:10     ` Greg Kroah-Hartman
2019-04-30  0:15 ` [PATCH 2/2] livepatch: Use correct kobject cleanup function Tobin C. Harding
2019-04-30  8:43   ` Greg Kroah-Hartman
2019-04-30 11:00   ` Miroslav Benes
2019-04-30 21:38     ` Tobin C. Harding
2019-04-30 15:08   ` Petr Mladek
2019-04-30 21:37     ` Tobin C. Harding [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=20190430213730.GC9454@eros.localdomain \
    --to=me@tobin.cc \
    --cc=gregkh@linuxfoundation.org \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=pmladek@suse.com \
    --cc=tobin@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.