Re: [PATCH 6/6] drm/tinydrm: Support device unplug

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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 :-)

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel