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