| Message ID | 20180418095631.9977-1-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 18, 2018 at 05:56:31PM +0800, Anand Jain wrote: > @@ -155,29 +155,26 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, > * > * uuid_mutex (global lock) > * ------------------------ > - * protects the fs_uuids list that tracks all per-fs fs_devices, resulting from > + * Protects the fs_uuids list that tracks all per-fs fs_devices, resulting from > * the SCAN_DEV ioctl registration or from mount either implicitly (the first > - * device) or requested by the device= mount option > - * > - * the mutex can be very coarse and can cover long-running operations > - * > - * protects: updates to fs_devices counters like missing devices, rw devices, > - * seeding, structure cloning, openning/closing devices at mount/umount time > + * device) or requested by the device= mount option. > * > * global::fs_devs - add, remove, updates to the global list ^^^^^^^ My typo, this should be fs_uuids. > * fs_devices::device_list_mutex (per-fs, with RCU) > * ------------------------------------------------ > - * protects updates to fs_devices::devices, ie. adding and deleting > + * Protects updates to fs_devices::devices, ie. adding and deleting, and its > + * counters like missing devices, rw devices, seeding, structure cloning, > + * openning/closing devices at mount/umount time. > * > - * simple list traversal with read-only actions can be done with RCU protection > + * 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) > + * May be used to exclude some operations from running concurrently without any > + * modifications to the list (see write_all_supers). The uuid_mutex usage is a bit muddy, so far I think that most uses are not necessary so this is in line with this patchset. In some cases we might need to add the device_list_mutex once uuid mutex is gone. The clear usage of the uuid_mutex is when a new fs_devices is added to the global fs_uuids, to prevent concurrent access by device scan and mount. Another one is the seed fs manipulation, that on the higher level works on the linked fs_devices. And the last one is the device renames that happen after a device appears under a different name. So far I haven't noticed any problems during tests of for-next with this patchset, so I guess we'd have to try harder to trigger the potential races. Thre's no device add/delete/replace/scan stress tests. The seeding is not very well covered by tests, so I'll keep the branch in for-next, but more tests are welcome. -- 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
On 04/24/2018 11:48 PM, David Sterba wrote: > On Wed, Apr 18, 2018 at 05:56:31PM +0800, Anand Jain wrote: >> @@ -155,29 +155,26 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, >> * >> * uuid_mutex (global lock) >> * ------------------------ >> - * protects the fs_uuids list that tracks all per-fs fs_devices, resulting from >> + * Protects the fs_uuids list that tracks all per-fs fs_devices, resulting from >> * the SCAN_DEV ioctl registration or from mount either implicitly (the first >> - * device) or requested by the device= mount option >> - * >> - * the mutex can be very coarse and can cover long-running operations >> - * >> - * protects: updates to fs_devices counters like missing devices, rw devices, >> - * seeding, structure cloning, openning/closing devices at mount/umount time >> + * device) or requested by the device= mount option. >> * >> * global::fs_devs - add, remove, updates to the global list > ^^^^^^^ > > My typo, this should be fs_uuids. right. Corrected in v2. > >> * fs_devices::device_list_mutex (per-fs, with RCU) >> * ------------------------------------------------ >> - * protects updates to fs_devices::devices, ie. adding and deleting >> + * Protects updates to fs_devices::devices, ie. adding and deleting, and its >> + * counters like missing devices, rw devices, seeding, structure cloning, >> + * openning/closing devices at mount/umount time. >> * >> - * simple list traversal with read-only actions can be done with RCU protection >> + * 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) >> + * May be used to exclude some operations from running concurrently without any >> + * modifications to the list (see write_all_supers). > > The uuid_mutex usage is a bit muddy, so far I think that most uses are > not necessary so this is in line with this patchset. In some cases we > might need to add the device_list_mutex once uuid mutex is gone. > The clear usage of the uuid_mutex is when a new fs_devices is added to > the global fs_uuids, to prevent concurrent access by device scan and > mount. Yes. I have part#2 of uuid_mutex which will cleanup this part. I got diverged into something else before I could send. Will send soon. Sorry for the delay. > Another one is the seed fs manipulation, that on the higher level > works on the linked fs_devices. And the last one is the device renames > that happen after a device appears under a different name. Sprout doesn't need uuid_mutex, it would need device_list_mutex of the respective seed fs_devices. I am planning this in part#3. As of now its ok to continue to use uuid_mutex. > So far I haven't noticed any problems during tests of for-next with this > patchset, so I guess we'd have to try harder to trigger the potential > races. > Thre's no device add/delete/replace/scan stress tests. stress tests - to exerciser concurrency - right. > The > seeding is not very well covered by tests, so I'll keep the branch in > for-next, but more tests are welcome. Let me find time to add in part#3 as above. 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 > -- 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
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 6d7f10729713..12617a8a7083 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -155,29 +155,26 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info, * * uuid_mutex (global lock) * ------------------------ - * protects the fs_uuids list that tracks all per-fs fs_devices, resulting from + * Protects the fs_uuids list that tracks all per-fs fs_devices, resulting from * the SCAN_DEV ioctl registration or from mount either implicitly (the first - * device) or requested by the device= mount option - * - * the mutex can be very coarse and can cover long-running operations - * - * protects: updates to fs_devices counters like missing devices, rw devices, - * seeding, structure cloning, openning/closing devices at mount/umount time + * device) or requested by the device= mount option. * * global::fs_devs - add, remove, updates to the global list * - * does not protect: manipulation of the fs_devices::devices list! + * Does not protect: manipulation of the fs_devices::devices list! * * btrfs_device::name - renames (write side), read is RCU * * fs_devices::device_list_mutex (per-fs, with RCU) * ------------------------------------------------ - * protects updates to fs_devices::devices, ie. adding and deleting + * Protects updates to fs_devices::devices, ie. adding and deleting, and its + * counters like missing devices, rw devices, seeding, structure cloning, + * openning/closing devices at mount/umount time. * - * simple list traversal with read-only actions can be done with RCU protection + * 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) + * May be used to exclude some operations from running concurrently without any + * modifications to the list (see write_all_supers). * * balance_mutex * -------------
Make the uuid_mutex and device_list_mutex comments inline with the changes. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- Hi David, Can you kindly add this patch to the set: 'Review uuid_mutex usage' Thanks, Anand fs/btrfs/volumes.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-)