Re: [PATCH] kobject: record and send PREV_SEQNUM with uevents

[Peter Rajnoha]

On 10/16/2017 03:55 PM, Greg KH wrote:
> On Mon, Oct 16, 2017 at 02:35:44PM +0200, Peter Rajnoha wrote:
>> Record last uevent seqnum that was used with kobject's uevent and send
>> it with next uevent as PREV_SEQNUM=<num> variable.
>>
>> This enhances the way we are able to track and handle uevents in userspace
>> if we are trying to catch up with all the uevents that were generated
>> while we were not listening or the uevents which were not delivered.
>> This may happen if the userspace listener is not running yet when the
>> uevent is triggered or there's a period of time when it's not listening
>> (e.g. because the userspace listener is being restarted while a new
>> uevent fires).
>>
>> A userspace listener can compare the last uevent seqnum it knows about
>> with the last uevent seqnum the kernel reports through uevents delivered
>> to userspace, both genuine and synthetic ones (the ones generated by
>> writing uevent action name to /sys/.../uevent file). Then, if needed,
>> userspace may execute appropriate actions based on that. We don't need
>> to reevaluate and recheck all items, instead, we can concentrate only
>> on items for which we really and actually need to reflect changed state
>> in kernel while the userspace listener was not listening for uevents.
>>
>> Signed-off-by: Peter Rajnoha <prajnoha@xxxxxxxxxx>
>> ---
>>  include/linux/kobject.h |  1 +
>>  lib/kobject.c           |  1 +
>>  lib/kobject_uevent.c    | 15 +++++++++++++--
>>  3 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/kobject.h b/include/linux/kobject.h
>> index e0a6205caa71..ee6db28b6186 100644
>> --- a/include/linux/kobject.h
>> +++ b/include/linux/kobject.h
>> @@ -73,6 +73,7 @@ struct kobject {
>>  #ifdef CONFIG_DEBUG_KOBJECT_RELEASE
>>  	struct delayed_work	release;
>>  #endif
>> +	u64			last_uevent_seqnum;
>>  	unsigned int state_initialized:1;
>>  	unsigned int state_in_sysfs:1;
>>  	unsigned int state_add_uevent_sent:1;
>> diff --git a/lib/kobject.c b/lib/kobject.c
>> index 763d70a18941..2cc809f10ae7 100644
>> --- a/lib/kobject.c
>> +++ b/lib/kobject.c
>> @@ -190,6 +190,7 @@ static void kobject_init_internal(struct kobject *kobj)
>>  		return;
>>  	kref_init(&kobj->kref);
>>  	INIT_LIST_HEAD(&kobj->entry);
>> +	kobj->last_uevent_seqnum = 0;
>>  	kobj->state_in_sysfs = 0;
>>  	kobj->state_add_uevent_sent = 0;
>>  	kobj->state_remove_uevent_sent = 0;
>> diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
>> index f237a09a5862..f1cbed06a395 100644
>> --- a/lib/kobject_uevent.c
>> +++ b/lib/kobject_uevent.c
>> @@ -454,8 +454,19 @@ int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
>>  	}
>>  
>>  	mutex_lock(&uevent_sock_mutex);
>> -	/* we will send an event, so request a new sequence number */
>> -	retval = add_uevent_var(env, "SEQNUM=%llu", (unsigned long long)++uevent_seqnum);
>> +	/*
>> +	 * We will send an event, so request a new sequence number.
>> +	 * Also, record the number for next uevent's PREV_SEQNUM.
>> +	 */
>> +	kobj->last_uevent_seqnum = ++uevent_seqnum;
>> +	retval = add_uevent_var(env, "SEQNUM=%llu",
>> +				(unsigned long long)uevent_seqnum);
>> +	if (retval) {
>> +		mutext_unlock(&uevent_sock_mutex);
>> +		goto exit;
>> +	}
>> +	retval = add_uevent_var(env, "PREV_SEQNUM=%llu",
>> +				(unsigned long long)kobj->last_uevent_seqnum);
> 
> I'm confused, why are we storing "last_uevent_seqnum" here at all, if it
> is only just the current one-1?  What else do you do with that value?
I'm sorry, I've sent incorrect patch by mistake. This is the correct one: