Re: [PATCH 12/25] sysfs: Introduce sysfs_rename_mutex

From: Eric W. Biederman
Date: Wed Aug 08 2007 - 04:31:39 EST


Tejun Heo <htejun@xxxxxxxxx> writes:

> Hello, Eric.
>
> Eric W. Biederman wrote:
>> Looking carefully at the rename code we have a subtle dependency
>> that the structure of sysfs not change while we are performing
>> a rename. If the parent directory of the object we are renaming
>> changes while the rename is being performed nasty things could
>> happen when we go to release our locks.
>>
>> So introduce a sysfs_rename_mutex to prevent this highly
>> unlikely theoretical issue.
>
> Yeah, it's a theoretical issue. Rename/move implementation has always
> depended on the parent structure not changing beneath it, but it's nice
> to tighten up loose ends.
>
>> +DEFINE_MUTEX(sysfs_rename_mutex);
>
> Probably doesn't really matter but wouldn't a rwsem fit better?

Maybe. I didn't feel any loss in when I was writing the code.
Very few code paths actually seem to care.

>> @@ -774,7 +775,7 @@ static struct dentry *__sysfs_get_dentry(struct
> super_block *sb, struct sysfs_di
>> * down from there looking up dentry for each step.
>> *
>> * LOCKING:
>> - * Kernel thread context (may sleep)
>> + * mutex_lock(sysfs_rename_mutex)

Well this is weird in that it should be on sysfs_get_dentry
more then __sysfs_get_dentry but otherwise it's ok.

> LOCKING describes what locks should be held when entering the function,
> so proper description would be something like...
>
> Kernel thread context, grabs sysfs_rename_mutex

For rename_dir and move_dir yes. I was updating the rules
for sysfs_get_dentry. Which really wants it's parents to
hold that lock.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/