Re: [PATCH linux-2.6 04/04] brsem: convert cpucontrol to brsem

[Tejun Heo <htejun@gmail.com>]

Nick Piggin wrote:
> Tejun Heo wrote:
> 
>> +/*
>> + * cpucontrol is a brsem used to synchronize cpu hotplug events.
>> + * Invoking lock_cpu_hotplug() read-locks cpucontrol and no
>> + * hotplugging events will occur until it's released.
>> + *
>> + * Unfortunately, brsem itself makes use of lock_cpu_hotplug() and
>> + * performing brsem write-lock operations on cpucontrol deadlocks.
>> + * This is avoided by...
>> + *
>> + * a. guaranteeing that cpu hotplug events won't occur during the
>> + *    write-lock operations, and
>> + *
>> + * b. skipping lock_cpu_hotplug() inside brsem.
>> + *
>> + * #a is achieved by acquiring and releasing cpucontrol_mutex outside
>> + * cpucontrol write-lock.  #b is achieved by skipping
>> + * lock_cpu_hotplug() inside brsem if the current task is
>> + * cpucontrol_mutex holder (is_cpu_hotplug_holder() test).
>> + *
>> + * Also, note that cpucontrol is first initialized with
>> + * BRSEM_BYPASS_INITIALIZER and then initialized again with
>> + * __create_brsem() instead of simply using create_brsem().  This is
>> + * necessary as cpucontrol brsem gets used way before brsem subsystem
>> + * becomes up and running.
>> + *
>> + * Until brsem is properly initialized, all brsem ops succeed
>> + * unconditionally.  cpucontrol becomes operational only after
>> + * cpucontrol_init() is finished, which should be called after
>> + * brsem_init_early().
>> + */
> 
> 
> Mmm, this is just insane IMO.
> 
> Note that I happen to also think the idea (brsems) have merit, and
> that cpucontrol may be one of the places where a sane implementation
> would actually be useful... but at least when you're introducing
> this kind of complexity anywhere, you *really* need to be able to
> back it up with numbers.
> 
> As far as the VFS race fix goes, I guess Al or someone else will
> comment on its correctness. But I think it might be nicer to first
> fix it with a regular rwsem and then show some numbers to justify
> its conversion to a brsem.
> 
> If you need interruptible rwsems, I almost got an implementation
> working a while back, and David Howells recently said he was
> interested in doing them... so that's not an impossibility.
> 
> Nick
> 

  Hello, Nick.

  I do agree that it's absolutely ugly.  I thought about ripping the 
3-tage init'ing and cpu hotplug stuff as currently cpu hotplug locking 
isn't used frequently, but was just giving a shot.  I'll strip this 
thing out.

  Thanks.

-- 
tejun