On Fri, Dec 09, 2016 at 09:57:45PM +0000, Chris Wilson wrote:
On Fri, Dec 09, 2016 at 03:19:43PM +0100, Daniel Vetter wrote:
We can be more clever than that and merge the 2 list walking loops. It's all under the one mutex critical section anyway, so nothing funny can happen.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_mode_config.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 64c2971478db..a6205a92dfee 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -84,7 +84,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_mode_card_res *card_res = data;
- struct list_head *lh; struct drm_framebuffer *fb; struct drm_connector *connector; struct drm_crtc *crtc;
@@ -105,25 +104,20 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
mutex_lock(&file_priv->fbs_lock);
- /*
* For the non-control nodes we need to limit the list of resources* by IDs in the group list for this node*/- list_for_each(lh, &file_priv->fbs)
fb_count++;- /* handle this in 4 parts */ /* FBs */
- if (card_res->count_fbs >= fb_count) {
copied = 0;fb_id = (uint32_t __user *)(unsigned long)card_res->fb_id_ptr;list_for_each_entry(fb, &file_priv->fbs, filp_head) {if (put_user(fb->base.id, fb_id + copied)) {mutex_unlock(&file_priv->fbs_lock);return -EFAULT;}copied++;
- copied = 0;
- fb_id = (uint32_t __user *)(unsigned long)card_res->fb_id_ptr;
- list_for_each_entry(fb, &file_priv->fbs, filp_head) {
fb_count++;if (fb_count > card_res->count_fbs)continue;if (put_user(fb->base.id, fb_id + copied)) {mutex_unlock(&file_priv->fbs_lock);return -EFAULT;copied++;if (fb_count < card_res->count_fbs && put_user(fb->base.id fb_id + fb_count) { } fb_count++; // no need for copied
I'd just like for one style :) Otoh, this seems reasonable to apply to crtc/encoders/connectors as well, and on the other we already have a counter for the others. Hmm, setting count = copied in the previous patch is thereby wrong (change in uABI).
It's only a change if the connector count changed between when we read it and when we walked the list. But I guess you've convinced me that simplifying this loops more is a good idea, and this way we can use the same style for all of them. -Daniel