On Tue, Mar 30, 2021 at 10:03 AM Jason Gunthorpe <j...@nvidia.com> wrote: > > On Tue, Mar 30, 2021 at 09:05:29AM -0700, Dan Williams wrote: > > > > If you can't clearly point to the *data* under RCU protection it is > > > being used wrong. > > > > Agree. > > > > The data being protected is the value of > > dev->kobj.state_in_sysfs. The > > So where is that read under: > > + idx = srcu_read_lock(&cxl_memdev_srcu); > + rc = __cxl_memdev_ioctl(cxlmd, cmd, arg); > + srcu_read_unlock(&cxl_memdev_srcu, idx); > > ?
device_is_registered() inside __cxl_memdev_ioctl(). > It can't read the RCU protected data outside the RCU critical region, > and it can't read/write RCU protected data without using the helper > macros which insert the required barriers. The required barriers are there. srcu_read_lock() + device_is_registered() is paired with cdev_device_del() + synchronize_rcu(). > IMHO this can't use 'dev->kobj.state_in_sysfs' as the RCU protected data. This usage of srcu is functionally equivalent to replacing srcu_read_lock() with down_read() and the shutdown path with: cdev_device_del(...); down_write(...): up_write(...); ...to flush readers that read ->state_in_sysfs == 1.