[PATCH v3 0/1]

[PATCH v3 0/1]

[Claudio Imbrenda]

This patch adds a few lines to the KVM common code to fire a
KOBJ_CHANGE uevent whenever a KVM VM is created or destroyed. The event
carries five environment variables:

CREATED indicates how many times a new VM has been created. It is
	useful for example to trigger specific actions when the first
	VM is started
COUNT indicates how many VMs are currently active. This can be used for
	logging or monitoring purposes
PID has the pid of the KVM process that has been started or stopped.
	This can be used to perform process-specific tuning.
STATS_PATH contains the path in debugfs to the directory with all the
	runtime statistics for this VM. This is useful for performance
	monitoring and profiling.
EVENT described the type of event, its value can be either "create" or
	"destroy"

Specific udev rules can be then set up in userspace to deal with the
creation or destruction of VMs as needed.

v2 -> v3:
* added EVENT
* shortened the names of the other variables

v1 -> v2:
* added KVM_VM_PID and KVM_VM_STATS_PATH
* some cleanup, the patch should look nicer now
* rebased on 4.12

*** BLURB HERE ***

Claudio Imbrenda (1):
  KVM: trigger uevents when starting or stopping a VM

 virt/kvm/kvm_main.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

-- 
2.7.4

[PATCH v3 1/1] KVM: trigger uevents when starting or stopping a VM

[Claudio Imbrenda]

This patch adds a few lines to the KVM common code to fire a
KOBJ_CHANGE uevent whenever a KVM VM is created or destroyed. The event
carries five environment variables:

CREATED indicates how many times a new VM has been created. It is
	useful for example to trigger specific actions when the first
	VM is started
COUNT indicates how many VMs are currently active. This can be used for
	logging or monitoring purposes
PID has the pid of the KVM process that has been started or stopped.
	This can be used to perform process-specific tuning.
STATS_PATH contains the path in debugfs to the directory with all the
	runtime statistics for this VM. This is useful for performance
	monitoring and profiling.
EVENT described the type of event, its value can be either "create" or
	"destroy"

Specific udev rules can be then set up in userspace to deal with the
creation or destruction of VMs as needed.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
---
 virt/kvm/kvm_main.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f0fe9d0..7c39783 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -130,6 +130,12 @@ EXPORT_SYMBOL_GPL(kvm_rebooting);
 
 static bool largepages_enabled = true;
 
+#define KVM_EVENT_CREATE_VM 0
+#define KVM_EVENT_DESTROY_VM 1
+static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm);
+static unsigned long long kvm_createvm_count;
+static unsigned long long kvm_active_vms;
+
 bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
 {
 	if (pfn_valid(pfn))
@@ -728,6 +734,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	int i;
 	struct mm_struct *mm = kvm->mm;
 
+	kvm_uevent_notify_change(KVM_EVENT_DESTROY_VM, kvm);
 	kvm_destroy_vm_debugfs(kvm);
 	kvm_arch_sync_events(kvm);
 	spin_lock(&kvm_lock);
@@ -3196,6 +3203,7 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 		fput(file);
 		return -ENOMEM;
 	}
+	kvm_uevent_notify_change(KVM_EVENT_CREATE_VM, kvm);
 
 	fd_install(r, file);
 	return r;
@@ -3848,6 +3856,57 @@ static const struct file_operations *stat_fops[] = {
 	[KVM_STAT_VM]   = &vm_stat_fops,
 };
 
+static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
+{
+	char cbuf[32], abuf[32], pidbuf[32], evbuf[16];
+	const char pathvar[11] = "STATS_PATH=";
+	char *ptr[6] = {cbuf, abuf, pidbuf, NULL, NULL, NULL};
+	char *tmp, *pathbuf;
+	unsigned long long created, active;
+	int idx = 3;
+
+	if (!kvm_dev.this_device || !kvm || !kvm->debugfs_dentry)
+		return;
+
+	spin_lock(&kvm_lock);
+	if (type == KVM_EVENT_CREATE_VM) {
+		kvm_createvm_count++;
+		kvm_active_vms++;
+	} else if (type == KVM_EVENT_DESTROY_VM) {
+		kvm_active_vms--;
+	}
+	created = kvm_createvm_count;
+	active = kvm_active_vms;
+	spin_unlock(&kvm_lock);
+
+	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
+	if (pathbuf) {
+		tmp = dentry_path_raw(kvm->debugfs_dentry,
+				      pathbuf + sizeof(pathvar),
+				      PATH_MAX - sizeof(pathvar));
+		if (!IS_ERR(tmp)) {
+			memcpy(tmp - sizeof(pathvar), pathvar, sizeof(pathvar));
+			ptr[idx++] = tmp - sizeof(pathvar);
+		}
+	}
+	snprintf(cbuf, sizeof(cbuf), "CREATED=%llu", created);
+	snprintf(abuf, sizeof(abuf), "COUNT=%llu", active);
+	snprintf(pidbuf, sizeof(pidbuf), "PID=%s",
+		 kvm->debugfs_dentry->d_name.name);
+	tmp = strchr(pidbuf, '-');
+	if (tmp)
+		*tmp = '\0';
+	if (type == KVM_EVENT_CREATE_VM) {
+		snprintf(evbuf, sizeof(evbuf), "EVENT=create");
+		ptr[idx++] = evbuf;
+	} else if (type == KVM_EVENT_DESTROY_VM) {
+		snprintf(evbuf, sizeof(evbuf), "EVENT=destroy");
+		ptr[idx++] = evbuf;
+	}
+	kobject_uevent_env(&kvm_dev.this_device->kobj, KOBJ_CHANGE, ptr);
+	kfree(pathbuf);
+}
+
 static int kvm_init_debug(void)
 {
 	int r = -EEXIST;
-- 
2.7.4

Re: [PATCH v3 1/1] KVM: trigger uevents when starting or stopping a VM

[David Hildenbrand]

On 10.07.2017 11:20, Claudio Imbrenda wrote:

Minor minor nit:

The subject should state "creating or destroying a VM"

> This patch adds a few lines to the KVM common code to fire a
> KOBJ_CHANGE uevent whenever a KVM VM is created or destroyed. The event
> carries five environment variables:
> 
> CREATED indicates how many times a new VM has been created. It is
> 	useful for example to trigger specific actions when the first
> 	VM is started
> COUNT indicates how many VMs are currently active. This can be used for
> 	logging or monitoring purposes
> PID has the pid of the KVM process that has been started or stopped.
> 	This can be used to perform process-specific tuning.
> STATS_PATH contains the path in debugfs to the directory with all the
> 	runtime statistics for this VM. This is useful for performance
> 	monitoring and profiling.
> EVENT described the type of event, its value can be either "create" or
> 	"destroy"
> 
> Specific udev rules can be then set up in userspace to deal with the
> creation or destruction of VMs as needed.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> ---
>  virt/kvm/kvm_main.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f0fe9d0..7c39783 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -130,6 +130,12 @@ EXPORT_SYMBOL_GPL(kvm_rebooting);
>  
>  static bool largepages_enabled = true;
>  
> +#define KVM_EVENT_CREATE_VM 0
> +#define KVM_EVENT_DESTROY_VM 1
> +static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm);
> +static unsigned long long kvm_createvm_count;
> +static unsigned long long kvm_active_vms;
> +
>  bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
>  {
>  	if (pfn_valid(pfn))
> @@ -728,6 +734,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
>  	int i;
>  	struct mm_struct *mm = kvm->mm;
>  
> +	kvm_uevent_notify_change(KVM_EVENT_DESTROY_VM, kvm);
>  	kvm_destroy_vm_debugfs(kvm);
>  	kvm_arch_sync_events(kvm);
>  	spin_lock(&kvm_lock);
> @@ -3196,6 +3203,7 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
>  		fput(file);
>  		return -ENOMEM;
>  	}
> +	kvm_uevent_notify_change(KVM_EVENT_CREATE_VM, kvm);
>  
>  	fd_install(r, file);
>  	return r;
> @@ -3848,6 +3856,57 @@ static const struct file_operations *stat_fops[] = {
>  	[KVM_STAT_VM]   = &vm_stat_fops,
>  };
>  
> +static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
> +{
> +	char cbuf[32], abuf[32], pidbuf[32], evbuf[16];

do we really need that much space for a pid?

> +	const char pathvar[11] = "STATS_PATH=";
> +	char *ptr[6] = {cbuf, abuf, pidbuf, NULL, NULL, NULL};
> +	char *tmp, *pathbuf;
> +	unsigned long long created, active;
> +	int idx = 3;
> +
> +	if (!kvm_dev.this_device || !kvm || !kvm->debugfs_dentry)
> +		return;
> +
> +	spin_lock(&kvm_lock);
> +	if (type == KVM_EVENT_CREATE_VM) {
> +		kvm_createvm_count++;
> +		kvm_active_vms++;
> +	} else if (type == KVM_EVENT_DESTROY_VM) {
> +		kvm_active_vms--;
> +	}
> +	created = kvm_createvm_count;
> +	active = kvm_active_vms;
> +	spin_unlock(&kvm_lock);
> +
> +	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
> +	if (pathbuf) {
> +		tmp = dentry_path_raw(kvm->debugfs_dentry,
> +				      pathbuf + sizeof(pathvar),
> +				      PATH_MAX - sizeof(pathvar));
> +		if (!IS_ERR(tmp)) {
> +			memcpy(tmp - sizeof(pathvar), pathvar, sizeof(pathvar));
> +			ptr[idx++] = tmp - sizeof(pathvar);
> +		}
> +	}
> +	snprintf(cbuf, sizeof(cbuf), "CREATED=%llu", created);

Did you think about using struct kobj_uevent_env / add_uevent_var()?

But most probably the problem here is special handling for
dentry_path_raw().


-- 

Thanks,

David

Re: [PATCH v3 1/1] KVM: trigger uevents when starting or stopping a VM

[Claudio Imbrenda]

On Mon, 10 Jul 2017 11:40:55 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 10.07.2017 11:20, Claudio Imbrenda wrote:
> 
> Minor minor nit:
> 
> The subject should state "creating or destroying a VM"

I'll fix it

[...]

> > +static void kvm_uevent_notify_change(unsigned int type, struct kvm
> > *kvm) +{
> > +	char cbuf[32], abuf[32], pidbuf[32], evbuf[16];  
> 
> do we really need that much space for a pid?

unfortunately yes. we don't have access to the pid when destroying the
VM, so I take it from the debugfs entry, and that has the format
"%d-%d", with pid and file descriptor, both can be 10 bytes long.

> > +	const char pathvar[11] = "STATS_PATH=";
> > +	char *ptr[6] = {cbuf, abuf, pidbuf, NULL, NULL, NULL};
> > +	char *tmp, *pathbuf;
> > +	unsigned long long created, active;
> > +	int idx = 3;
> > +
> > +	if (!kvm_dev.this_device || !kvm || !kvm->debugfs_dentry)
> > +		return;
> > +
> > +	spin_lock(&kvm_lock);
> > +	if (type == KVM_EVENT_CREATE_VM) {
> > +		kvm_createvm_count++;
> > +		kvm_active_vms++;
> > +	} else if (type == KVM_EVENT_DESTROY_VM) {
> > +		kvm_active_vms--;
> > +	}
> > +	created = kvm_createvm_count;
> > +	active = kvm_active_vms;
> > +	spin_unlock(&kvm_lock);
> > +
> > +	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
> > +	if (pathbuf) {
> > +		tmp = dentry_path_raw(kvm->debugfs_dentry,
> > +				      pathbuf + sizeof(pathvar),
> > +				      PATH_MAX - sizeof(pathvar));
> > +		if (!IS_ERR(tmp)) {
> > +			memcpy(tmp - sizeof(pathvar), pathvar,
> > sizeof(pathvar));
> > +			ptr[idx++] = tmp - sizeof(pathvar);
> > +		}
> > +	}
> > +	snprintf(cbuf, sizeof(cbuf), "CREATED=%llu", created);  
> 
> Did you think about using struct kobj_uevent_env / add_uevent_var()?

for what I could see, that only works with a more invasive patch. I'll
see what I can do.
 
> But most probably the problem here is special handling for
> dentry_path_raw().
 

Re: [PATCH v3 1/1] KVM: trigger uevents when starting or stopping a VM

[Paolo Bonzini]

On 10/07/2017 13:53, Claudio Imbrenda wrote:
> On Mon, 10 Jul 2017 11:40:55 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 10.07.2017 11:20, Claudio Imbrenda wrote:
>>
>> Minor minor nit:
>>
>> The subject should state "creating or destroying a VM"
> 
> I'll fix it

I can fix it when applying, no problem.

Paolo

> [...]
> 
>>> +static void kvm_uevent_notify_change(unsigned int type, struct kvm
>>> *kvm) +{
>>> +	char cbuf[32], abuf[32], pidbuf[32], evbuf[16];  
>>
>> do we really need that much space for a pid?
> 
> unfortunately yes. we don't have access to the pid when destroying the
> VM, so I take it from the debugfs entry, and that has the format
> "%d-%d", with pid and file descriptor, both can be 10 bytes long.
> 
>>> +	const char pathvar[11] = "STATS_PATH=";
>>> +	char *ptr[6] = {cbuf, abuf, pidbuf, NULL, NULL, NULL};
>>> +	char *tmp, *pathbuf;
>>> +	unsigned long long created, active;
>>> +	int idx = 3;
>>> +
>>> +	if (!kvm_dev.this_device || !kvm || !kvm->debugfs_dentry)
>>> +		return;
>>> +
>>> +	spin_lock(&kvm_lock);
>>> +	if (type == KVM_EVENT_CREATE_VM) {
>>> +		kvm_createvm_count++;
>>> +		kvm_active_vms++;
>>> +	} else if (type == KVM_EVENT_DESTROY_VM) {
>>> +		kvm_active_vms--;
>>> +	}
>>> +	created = kvm_createvm_count;
>>> +	active = kvm_active_vms;
>>> +	spin_unlock(&kvm_lock);
>>> +
>>> +	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
>>> +	if (pathbuf) {
>>> +		tmp = dentry_path_raw(kvm->debugfs_dentry,
>>> +				      pathbuf + sizeof(pathvar),
>>> +				      PATH_MAX - sizeof(pathvar));
>>> +		if (!IS_ERR(tmp)) {
>>> +			memcpy(tmp - sizeof(pathvar), pathvar,
>>> sizeof(pathvar));
>>> +			ptr[idx++] = tmp - sizeof(pathvar);
>>> +		}
>>> +	}
>>> +	snprintf(cbuf, sizeof(cbuf), "CREATED=%llu", created);  
>>
>> Did you think about using struct kobj_uevent_env / add_uevent_var()?
> 
> for what I could see, that only works with a more invasive patch. I'll
> see what I can do.
>  
>> But most probably the problem here is special handling for
>> dentry_path_raw().
>  
> 

Re: [[PATCH v2] 1/1] kprobes: retry pending optprobe after freeing blocker

[Masami Hiramatsu]

Hi Hongao,

Thanks for updating. After a detailed review, we don't need a new boolean
flag for it. Since the "queued unused probe" (which is handled by
do_free_cleaned_kprobes() eventually) is always disarmed. Thus, we only
need to check all probes to reoptimize sibling probes in the
do_free_cleaned_kprobes().

On Wed, 10 Dec 2025 11:33:21 +0800
hongao <hongao@uniontech.com> wrote:

> The freeing_list cleanup now retries optimizing any sibling probe that was
> deferred while this aggregator was being torn down.  Track the pending
> address in struct optimized_kprobe so __disarm_kprobe() can defer the
> retry until kprobe_optimizer() finishes disarming.
> 
> Signed-off-by: hongao <hongao@uniontech.com>
> ---
> Changes since v1:
> - Replace `kprobe_opcode_t *pending_reopt_addr` with `bool reopt_unblocked_probes`
>   in `struct optimized_kprobe` to avoid storing an address and simplify logic.
> - Use `op->kp.addr` when looking up the sibling optimized probe instead of
>   keeping a separate stored address.
> - Defer re-optimization by setting/clearing `op->reopt_unblocked_probes` in
>   `__disarm_kprobe()` / consuming it in `do_free_cleaned_kprobes()` so the
>   retry runs after the worker finishes disarming.
> - Link to v1: https://lore.kernel.org/all/2B0BC73E9D190B7B+20251027130535.2296913-1-hongao@uniontech.com/
> ---
>  include/linux/kprobes.h |  1 +
>  kernel/kprobes.c        | 28 ++++++++++++++++++++++------
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 8c4f3bb24..4f49925a4 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -338,6 +338,7 @@ DEFINE_INSN_CACHE_OPS(insn);
>  struct optimized_kprobe {
>  	struct kprobe kp;
>  	struct list_head list;	/* list for optimizing queue */
> +	bool reopt_unblocked_probes;
>  	struct arch_optimized_insn optinsn;
>  };

This is not needed. 

>  
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index da59c68df..799542dff 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -514,6 +514,7 @@ static LIST_HEAD(freeing_list);
>  
>  static void kprobe_optimizer(struct work_struct *work);
>  static DECLARE_DELAYED_WORK(optimizing_work, kprobe_optimizer);
> +static void optimize_kprobe(struct kprobe *p);
>  #define OPTIMIZE_DELAY 5
>  
>  /*
> @@ -591,6 +592,21 @@ static void do_free_cleaned_kprobes(void)
>  			 */
>  			continue;
>  		}
> +		if (op->reopt_unblocked_probes) {
> +			struct kprobe *unblocked;
> +
> +			/*
> +			 * The aggregator was holding back another probe while it sat on the
> +			 * unoptimizing/freeing lists.  Now that the aggregator has been fully
> +			 * reverted we can safely retry the optimization of that sibling.
> +			 */
> +
> +			unblocked = get_optimized_kprobe(op->kp.addr);
> +			if (unlikely(unblocked))
> +				optimize_kprobe(unblocked);
> +			op->reopt_unblocked_probes = false;
> +		}

This is what we need. (but do not need to check/update reopt_unblocked_probes.

> +
>  		free_aggr_kprobe(&op->kp);
>  	}
>  }



> @@ -1009,13 +1025,13 @@ static void __disarm_kprobe(struct kprobe *p, bool reopt)
>  		_p = get_optimized_kprobe(p->addr);
>  		if (unlikely(_p) && reopt)
>  			optimize_kprobe(_p);
> +	} else if (reopt && kprobe_aggrprobe(p)) {
> +		struct optimized_kprobe *op =
> +			container_of(p, struct optimized_kprobe, kp);
> +
> +		/* Defer the re-optimization until the worker finishes disarming. */
> +		op->reopt_unblocked_probes = true;

Do not need this.

>  	}
> -	/*
> -	 * TODO: Since unoptimization and real disarming will be done by
> -	 * the worker thread, we can not check whether another probe are
> -	 * unoptimized because of this probe here. It should be re-optimized
> -	 * by the worker thread.
> -	 */

Only remove this comment.

Thank you,

>  }
>  
>  #else /* !CONFIG_OPTPROBES */
> -- 
> 2.47.2
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

[PATCH v3 0/1]

[hongao]

Hi Masami,

Thanks for the review and clarification.

As suggested, I dropped the reopt_unblocked_probes flag and simply retry
optimize_kprobe() from do_free_cleaned_kprobes() after the aggregator is
fully reverted. The stale comment in __disarm_kprobe() is also removed.
No new state is added to struct optimized_kprobe.

Changes since v2:
- reoptimize the sibling directly in do_free_cleaned_kprobes()
- no new fields in struct optimized_kprobe
- remove the obsolete TODO in __disarm_kprobe()
- Link to v2: https://lore.kernel.org/all/C5E6295166D4EDE9+20251210033321.2895692-1-hongao@uniontech.com/

Thanks,
Hongao

---

Hongao (1):
  kprobes: retry blocked optprobe in do_free_cleaned_kprobes

 kernel/kprobes.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

-- 
2.50.1