Hey,
On Fri, May 22, 2015 at 09:58:10AM -0400, Frediano Ziglio wrote:
}
int qxl_io_update_area(struct qxl_device *qdev, struct qxl_bo *surf,
const struct qxl_rect *area)
const struct qxl_rect *area, bool intr){ int surface_id; uint32_t surface_width, surface_height; @@ -350,7 +347,7 @@ int qxl_io_update_area(struct qxl_device *qdev, struct qxl_bo *surf, mutex_lock(&qdev->update_area_mutex); qdev->ram_header->update_area = *area; qdev->ram_header->update_surface = surface_id;
- ret = wait_for_io_cmd_user(qdev, 0, QXL_IO_UPDATE_AREA_ASYNC, true);
- ret = wait_for_io_cmd_user(qdev, 0, QXL_IO_UPDATE_AREA_ASYNC, intr); mutex_unlock(&qdev->update_area_mutex); return ret;
} @@ -588,10 +585,7 @@ int qxl_update_surface(struct qxl_device *qdev, struct qxl_bo *surf) rect.right = surf->surf.width; rect.top = 0; rect.bottom = surf->surf.height; -retry:
- ret = qxl_io_update_area(qdev, surf, &rect);
- if (ret == -ERESTARTSYS)
goto retry;
- ret = qxl_io_update_area(qdev, surf, &rect, false);
My understanding is that the fix is this hunk? If so, this could be made more obvious with an intermediate commit adding the 'bool intr' arg to qxl_io_update_area and only calling it with 'true' in the appropriate places. This code path is only triggered from qxl_surface_evict() which I assume is not necessarily easily interruptible, so this change makes sense to me. However it would be much better to get a review from Dave Airlie ;)
Christophe
Are you asking if just removing the loop would fix the issue? So you are proposing a first patch that add the argument always passing true and another that change some calls to false? It make sense but still the loop should be removed.
Sorry, I was not clear, removing the loop is fine, I was not suggesting to keep it.
Christophe