On Sat, Dec 14, 2019 at 08:26:24AM +0800, Anand Jain wrote: > > > On 14/12/19 1:02 AM, David Sterba wrote: > > On Fri, Dec 13, 2019 at 05:43:32PM +0100, David Sterba wrote: > >>> Looked into this further, actually we don't need any lock here > >>> the device delete thread which calls kobject_put() makes sure > >>> sysfs read is closed. So an existing sysfs read thread will have > >>> to complete before device free. > >>> > >>> > >>> CPU1 CPU2 > >>> > >>> btrfs_rm_device > >>> open file > >>> btrfs_sysfs_rm_device_link > >>> call read, access freed device > >>> sysfs waits for the open file > >>> to close. > >> > >> How exactly does sysfs wait for the device? Is it eg wait_event checking > >> number of references? If the file stays open by an evil process is it > >> going to block the device removal indefinitelly? > > > > Yeah, sysfs waits until the file is closed. Eg. umount can be stalled > > that way too. > > And similar to umount, I don't think we should return EBUSY > for btrfs_rm_device if the device sysfs attribute is opened, > as sysfs show attributes are non blocking and would be completed > in the timely manner. While I don't think the blocking behaviour is totally OK, returning EBUSY could be confusing without any other explanation. Also leaving sysfs files open but not read is kind of strange on itself.
