On Mon, Jun 11, 2018 at 10:50:54AM +0100, Filipe Manana wrote: > >>> btrfs: replace uuid_mutex by device_list_mutex in > >>> btrfs_open_devices
> >> * > >> * 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 > >> > >> generates some confusion since btrfs_open_devices(), after that > >> commit, no longer takes the uuid_mutex and it > >> updates some fs_devices counters (opened, open_devices, etc). > > > > As uuid_mutex is a global fs_uuids lock for the per fsid operations > > doesn't make any sense. > > > > This problem is reproducible only for-4.18, misc-next if fine. > > I am looking deeper. > > What about the unprotected updates (increments) to fs_devices->opened > and fs_devices->open_devices? > Other functions are accessing/updating them while holding the uuid mutex. The goal is to reduce usage of uuid_mutex only to protect search or update of the fs_uuids list, everything else should be protected by the device_list_mutex. The commit 542c5908abfe84f7 (use device_list_mutex in btrfs_open_devices) implements that but then the access to the ->opened member is not protected consistently. There are patches that convert the use to device_list_mutex but haven't been merged due to refinements or pending review. At this point I think we should revert the one commit 542c5908abfe84f7 as it introduces the locking problems and revisit the whole fs_devices locking scheme again in the dex dev cycle. That will be post rc1 as there might be more to revert. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
