[Devel] [PATCH vz7 v2] ploop: properly restore old delta kobject on replace error

[Alexander Atanasov]

On 9.06.23 9:11, Pavel Tikhomirov wrote:
> 
> 
> On 09.06.2023 00:19, Alexander Atanasov wrote:
>> Current code removes the old_delta kobject before the new delta
>> is ready to be used in case there is an error while reading BAT,
>> the "too short BAT" error we have seen.
> 
> The above sentence seems incomplete, as in an opposite case where there 
> was no error, old_delta kobject would have been removed before new delta 
> is ready anyway =)

Ok, i will try to make it clear.

.... [snip] ...

>> +/*
>> + * Have to implement own version of kobject_rename since it is GPL 
>> only symbol
>> + */
>> +static int ploop_rename_delta(struct ploop_delta *delta, int level, 
>> char *pref)
>> +{
>> +    kobject_del(&delta->kobj);
>> +    return KOBJECT_ADD(&delta->kobj, &delta->plo->kobj,
>> +              "%s%d", pref ? : "", level);
>> +}
>> +
> 
> It would probably be cleaner to put this function just before the 
> function using it (rename_deltas).

Ok.

... [snip] ...

>> -
>> -out_kobj_del:
>> -    kobject_del(&delta->kobj);
>>       kobject_put(&plo->kobj);
>> +    return 0;
>> +out_kobj_restore:
>> +    /* we haven't dropped our plo->kobj ref just add back */
>> +    err = KOBJECT_ADD(&old_delta->kobj, &plo->kobj, "%d", 
>> old_delta->level);
>> +    if (err < 0) {
>> +        PL_ERR(plo, "Failed to restore old delta kobject\n");
>> +        /*
>> +         * Get an extra ref to parent object as kobject_add does, so it
>> +         * later kobject_del doesn't destory it
>> +         */
>> +        kobject_get(&plo->kobj);
> 
> Later kobject_del would not see parent object as first kobject_del did 
> "kobj->parent = NULL;", so in this aspect you should not be getting 
> reference here.
> 
> note: Problem I was talking in my previous message was about kobj->kset 
> put.

Yes, the problem comes from doing double kobject_put which results in 
double kobj_kset_leave call which results in double kset_put.
I've missed the clearing of kobject->parent so came the idea about the
extra ref. Now i wonder would it help to take an extra ref for the 
kobj->kset since there is no other way to handle the error. What do you 
think about that ?


-- 
Regards,
Alexander Atanasov