On Fri, Dec 09, 2016 at 09:47:56PM +0000, Chris Wilson wrote:
> On Fri, Dec 09, 2016 at 03:19:42PM +0100, Daniel Vetter wrote:
> > Looping when we keep track of this is silly. Only thing we have to
> > be careful is with sampling the connector count. To avoid inconsisten
> > results due to gcc re-computing this, use READ_ONCE.
>
> Later on in the function we take the mutex that should prevent the
> values from changing.
>
> If each block was like
>
> mutex_lock(&mode_config->lockc);
Well that mutex_lock is fiction too. It's not needed for any of the lists,
and it definitely doesn't protect connector_list. Step-by-step I'd say ...
-Daniel
>
> count = dev->mode_config.num_crtc;
> if (card_res->count_crtcs >= count) {
> u32 __user id = u64_to_user_ptr(card_res->crtc_id_ptr);
> unsigned int copied = 0;
>
> drm_for_each_crtc(crtc, dev) {
> if (copied >= count)
> break;
>
> if (put_user(crtc->base.id, id + copied)) {
> ret = -EFAULT;
> goto out;
> }
>
> copied++;
> }
>
> count = copied;
> }
> card_res->count_crtcs = count;
>
> ...
>
> mutex_unlock(&mode_config->lockc);
>
> it would look a bit neater.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
