Re: [PATCH 06/11] btrfs: document device locking

[Anand Jain <anand.jain@oracle.com>]

On 11/03/2017 07:13 PM, Anand Jain wrote:
> 
> 
> Thanks for writing this.
> 
>> + * - fs_devices::device_list_mutex (per-fs, with RCU)
>> + *
>> + *   protects updates to fs_devices::devices, ie. adding and deleting
>> + *
>> + *   simple list traversal with read-only actions can be done with RCU
>> + *   protection
>> + *
>> + *   may be used to exclude some operations from running concurrently 
>> without
>> + *   any modifications to the list (see write_all_supers)
> 
>> + * - volume_mutex
>> + *
>> + *   coarse lock owned by a mounted filesystem; used to exclude some 
>> operations
>> + *   that cannot run in parallel and affect the higher-level 
>> properties of the
>> + *   filesystem like: device add/deleting/resize/replace, or balance
> 
>> + * - chunk_mutex
>> + *
>> + *   protects chunks, adding or removing during allocation, trim or when
>> + *   a new device is added/removed
> 
> ::
> 
>> + * Lock nesting
>> + * ------------
>> + *
>> + * uuid_mutex
>> + *   volume_mutex

I think it should be nested like this, but as of now its not. Ref [1]
  [1]
  btrfs: move volume_mutex into the btrfs_rm_device()

Thanks, Anand

>> + *     device_list_mutex
>> + *       chunk_mutex
>> + *     balance_mutex
> 
> 
> If we have a list of operations that would consume these locks then we 
> can map it accordingly for better clarity.
> 
> To me it looks like we have too many locks.
>   - we don't have to differentiate the mounted and unmounted context
>     for device locks.
>   - Two lock would be sufficient, one for the device list
>     (add/rm,replace,..) and another for device property changes
>     (resize, trim,..).
> 
> Thanks, Anand
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html