From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756103AbdERNGF (ORCPT ); Thu, 18 May 2017 09:06:05 -0400 Received: from mx2.suse.de ([195.135.220.15]:37384 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756086AbdERNGD (ORCPT ); Thu, 18 May 2017 09:06:03 -0400 Date: Thu, 18 May 2017 15:05:58 +0200 From: Libor Pechacek To: Miroslav Benes Cc: jpoimboe@redhat.com, jeyu@redhat.com, jikos@kernel.org, pmladek@suse.com, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] livepatch: Add force sysfs attribute Message-ID: <20170518130558.GE9599@fm.suse> References: <20170518120043.7205-1-mbenes@suse.cz> <20170518120043.7205-2-mbenes@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170518120043.7205-2-mbenes@suse.cz> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu 18-05-17 14:00:41, Miroslav Benes wrote: > Add write-only force attribute to livepatch sysfs infrastructure. We can > use it later to force couple of events during a live patching process. > Be it a sending of a fake signal or forcing of the tasks' successful > conversion. > > It does not make sense to use the force facility when there is no > transaction running (although there is no harm doing that). Therefore we > limit only to situations when klp_transition_patch variable is set. > Normally, klp_mutex lock should be grabbed, because the variable is > shared. However that would hold the action back unnecessarily because of > waiting for the lock, so we omit the lock here. The resulting race > window is harmless (using force when there is no transaction running). > > Signed-off-by: Miroslav Benes > --- > Documentation/ABI/testing/sysfs-kernel-livepatch | 9 +++++ > kernel/livepatch/core.c | 45 ++++++++++++++++++++++++ > 2 files changed, 54 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-kernel-livepatch b/Documentation/ABI/testing/sysfs-kernel-livepatch > index d5d39748382f..26e9f58cea9e 100644 > --- a/Documentation/ABI/testing/sysfs-kernel-livepatch > +++ b/Documentation/ABI/testing/sysfs-kernel-livepatch > @@ -8,6 +8,15 @@ Contact: live-patching@vger.kernel.org > The /sys/kernel/livepatch directory contains subdirectories for > each loaded live patch module. > > +What: /sys/kernel/livepatch/force > +Date: May 2017 > +KernelVersion: 4.13.0 > +Contact: live-patching@vger.kernel.org > +Description: > + A write-only attribute that allows administrator to affect the > + course of an existing transition. A fake signal can be send or ^^^^ "sent" > + tasks TIF can be cleared. > + > What: /sys/kernel/livepatch/ > Date: Nov 2014 > KernelVersion: 3.19.0 > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index b9628e43c78f..84f8944704ad 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -437,6 +437,7 @@ EXPORT_SYMBOL_GPL(klp_enable_patch); > * Sysfs Interface > * > * /sys/kernel/livepatch > + * /sys/kernel/livepatch/force > * /sys/kernel/livepatch/ > * /sys/kernel/livepatch//enabled > * /sys/kernel/livepatch//transition > @@ -444,6 +445,43 @@ EXPORT_SYMBOL_GPL(klp_enable_patch); > * /sys/kernel/livepatch/// > */ > > +static ssize_t force_store(struct kobject *kobj, struct kobj_attribute *attr, > + const char *buf, size_t count) > +{ > + unsigned long val; > + int ret; > + > + ret = kstrtoul(buf, 10, &val); > + if (ret) > + return ret; > + > + /* > + * klp_mutex lock is not grabbed here intentionally. It is not really > + * needed. The race window is harmless and grabbing the lock would only > + * hold the action back. > + */ > + if (!klp_transition_patch) { > + pr_info("no patching in progress. Force not allowed\n"); proposing smoother wording and information sharing pr_info("no patching in progress, forced action (%d) ineffective", val); > + return -EINVAL; > + } > + > + switch (val) { I felt strong confusion for a while looking at a function what does nothing. A comment that this is intentionally an empty shell, at this stage, would be welcome. > + default: > + return -EINVAL; > + } > + > + return count; > +} > + > +static struct kobj_attribute force_kobj_attr = __ATTR_WO(force); > +static struct attribute *klp_attrs[] = { > + &force_kobj_attr.attr, > + NULL > +}; > +static struct attribute_group klp_sysfs_group = { > + .attrs = klp_attrs, > +}; > + > static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr, > const char *buf, size_t count) > { > @@ -954,6 +992,13 @@ static int __init klp_init(void) > if (!klp_root_kobj) > return -ENOMEM; > > + ret = sysfs_create_group(klp_root_kobj, &klp_sysfs_group); > + if (ret) { > + pr_err("cannot create livepatch attributes in sysfs\n"); > + kobject_put(klp_root_kobj); > + return ret; > + } > + > return 0; > } > Libor > -- > 2.12.2 > > -- > To unsubscribe from this list: send the line "unsubscribe live-patching" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Libor Pechacek SUSE Labs