Re: [RFC PATCH 0/2] livepatch: Avoid possible race when releasing the patch

From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Petr Mladek <pmladek@suse.com>
Cc: mbenes@suse.cz, jeyu@redhat.com, jikos@kernel.org,
	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  0/2] livepatch: Avoid possible race when releasing the patch
Date: Mon, 23 May 2016 11:30:52 -0500	[thread overview]
Message-ID: <20160523163052.hxulffafuqg2vw23@treble> (raw)
In-Reply-To: <1464018848-4303-1-git-send-email-pmladek@suse.com>

On Mon, May 23, 2016 at 05:54:06PM +0200, Petr Mladek wrote:
> There was a long discussion about a possible race with sysfs, kobjects
> when removing an unused livepatch, see
> https://lkml.kernel.org/g/%3C1462190242-24731-1-git-send-email-mbenes@suse.cz%3E
> 
> This patch set tries to implement what looked the most preferred solution
> from the discussion. I did my best to keep the patch definition simple.
> But I am not super happy with the result.

Hi Petr,

Thanks a lot for looking at this.  I'm also not crazy about this patch,
but it does help with understanding the proposal.

> I send the current state before I spent even more time on different
> approaches.
> 
> I personally think that we might get better result if we declare
> some limited structures, define them statically and then copy all
> data into the final structures in a single call. I did not implement
> this because it was weird on the first look but I am not sure now.

I agree that would be better than this patch.  In fact, IIRC, that's
what Seth first implemented in the original livepatch patches, but then
the rest of us complained about it ;-)

It would be a clean separation of struct lifetimes, would allow us to
get rid of the "external" and "internal" comments in livepatch.h, and
would reduce the possibility of the user accidentally messing with our
internal state.

But it would be a little unusual to have two sets of structs (private
and public), and it would require some additional boilerplate code to do
the copying.

But even so, I think I would slightly prefer it over anything else.

> But even more I would prefer the solution with the completion.
> It is already used by the module framework. It does not look
> that hacky to me after all.

The completion would be the easiest solution.  In my mind it would be a
hack, but reasonable minds can disagree about that :-)  But anyway, I
could live with it.  I don't have a strong preference either way.

-- 
Josh