Hi Daniel,
On Monday, 4 September 2017 12:04:38 EEST Daniel Vetter wrote:
On Mon, Sep 04, 2017 at 11:41:05AM +0300, Laurent Pinchart wrote:
On Monday, 4 September 2017 10:26:15 EEST Daniel Vetter wrote:
[snip]
Yup, expect imo this should be done in drm core (as much as possible, i.e. for ioctl at least, standard sysfs), plus then enter/exit exported to drivers for their stuff.
And it really needs to be srcu. For kms drivers the atomic_inc/dec wont matter, for render drivers it will show up (some of our ioctls are 100% lock less in the fastpath, not a single atomic op in there).
Don't forget that we need to check the disconnect flag when entering ioctls to return -ENODEV. I'm open to clever solutions, but I'd first aim for correctness with a lock and then replace the internal implementation.
As Noralf pointed out, we already check for drm_dev_is_unplugged(). Maybe not in all places, but that can be fixed.
Yes, but I believe that both the unplugged check and enter reference get need to be done atomically.
And you can't first make all ioctl slower and then fix it up, at least not spread over multiple patch series. I guess for developing, doing the simpler atomic counter first is ok. But the same patch series needs to move over to srcu at the end.
To SRCU or something similar, yes.
But I'm not sure that's a good idea, since implementing this 100% correctly using your atomic_t idea means you implement half of srcu anyway. Otoh discovering all those races should be an interesting journey :-)