linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Kosina <jikos@kernel.org>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Miroslav Benes <mbenes@suse.cz>,
	jeyu@redhat.com, pmladek@suse.com, jslaby@suse.cz,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
	huawei.libin@huawei.com, minfei.huang@yahoo.com
Subject: Re: [RFC PATCH] livepatch: allow removal of a disabled patch
Date: Thu, 5 May 2016 23:08:53 +0200 (CEST)	[thread overview]
Message-ID: <alpine.LNX.2.00.1605052254010.31937@cbobk.fhfr.pm> (raw)
In-Reply-To: <20160505150403.h76mtzyqikitov6f@treble>

On Thu, 5 May 2016, Josh Poimboeuf wrote:

> I would disagree with the statement that the dynamic kobject doesn't
> scale.  We would just need a helper function to get from a kobject to
> its klp_patch.
>
> In fact, to me it seems like the right way to do it.  It doesn't make
> sense for the code which creates the kobject to be different from the
> code which initializes it.  It's slightly out of context, but
> kobject.txt does say:
> 
>   "Code which creates a kobject must, of course, initialize that object."
> 
> I view the completion as a hack to compensate for the fact that we're
> abusing the kobject interface.  And so it makes sense to me that
> CONFIG_DEBUG_KOBJECT_RELEASE would cause problems, because we're using
> kobjects in the wrong way.
> 
> So in my view, the two options are:
> 
> 1. Convert the kobject to dynamic as I described.
> 
> 2. Change the klp_register() interface so that klp_patch gets allocated
>    in livepatch code.
> 
> I'd be curious to hear what others think.

My understanding is that the concern here is that walking through the 
complete linked list every time sysfs node is accessed, just to figure out 
whether we're able to find a klp_patch entry that points back to the 
particular kobject that's being passed to the sysfs callback, isn't really 
super-efficient.

I personally wouldn't worry *that* much about that particular aspect 
(sysfs operations are hardly considered time critical anyway), but I'd 
have to think a bit more whether this is really safe wrt. deadlocks 
between kernfs locks and klp_mutex; but so far it seems to me that 
klp_mutex always nests below kernfs, so it should be OK.

Unfortunately, this still feels like a non-negligible (mis|ab)use of 
kobjects that could bite us later wrt. maintainability and general clarity 
of the code, and therefore tying klp_patch lifetime to (de)allocations 
from within livepatch code itself seems like much better idea to me.

Thanks,

-- 
Jiri Kosina
SUSE Labs

  reply	other threads:[~2016-05-05 21:08 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-02 11:57 [RFC PATCH] livepatch: allow removal of a disabled patch Miroslav Benes
2016-05-02 15:08 ` Josh Poimboeuf
2016-05-03  8:16   ` Miroslav Benes
2016-05-31 23:13     ` Jiri Kosina
2016-05-03 21:37 ` Josh Poimboeuf
2016-05-03 22:31   ` Jiri Kosina
2016-05-04  2:39     ` Josh Poimboeuf
2016-05-04  3:36       ` Josh Poimboeuf
2016-05-04 11:58         ` Miroslav Benes
2016-05-04 13:14           ` Josh Poimboeuf
2016-05-04 14:35             ` Miroslav Benes
2016-05-04 16:14               ` Josh Poimboeuf
2016-05-05  8:28                 ` Miroslav Benes
2016-05-05 13:27                   ` Josh Poimboeuf
2016-05-05 14:25                     ` Miroslav Benes
2016-05-05 15:04                       ` Josh Poimboeuf
2016-05-05 21:08                         ` Jiri Kosina [this message]
2016-05-06  0:42                         ` Jessica Yu
2016-05-06  7:51                           ` Miroslav Benes

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=alpine.LNX.2.00.1605052254010.31937@cbobk.fhfr.pm \
    --to=jikos@kernel.org \
    --cc=huawei.libin@huawei.com \
    --cc=jeyu@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=minfei.huang@yahoo.com \
    --cc=pmladek@suse.com \
    /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;
as well as URLs for NNTP newsgroup(s).