Re: [PATCH 4/4] btrfs: sysfs, add devid/dev_state kobject and attribute

David Sterba
2020-01-06T16:00:18Z

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.