Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751861AbbEZIUi (ORCPT ); Tue, 26 May 2015 04:20:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45042 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751421AbbEZIUf (ORCPT ); Tue, 26 May 2015 04:20:35 -0400 Date: Tue, 26 May 2015 10:20:30 +0200 From: Christophe Fergeau To: Frediano Ziglio Cc: spice-devel@lists.freedesktop.org, David Airlie , dri-devel@lists.freedesktop.org, Dave Airlie , linux-kernel@vger.kernel.org Subject: Re: [Spice-devel] [PATCH] Do not loop on ERESTARTSYS using interruptible waits Message-ID: <20150526082030.GF4920@edamame.cdg.redhat.com> References: <1591625424.1112688.1432028990916.JavaMail.zimbra@redhat.com> <163000764.1114319.1432029294390.JavaMail.zimbra@redhat.com> <20150522115805.GR20750@edamame.cdg.redhat.com> <1702166382.3247503.1432303090162.JavaMail.zimbra@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Y1L3PTX8QE8cb2T+" Content-Disposition: inline In-Reply-To: <1702166382.3247503.1432303090162.JavaMail.zimbra@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3362 Lines: 90 --Y1L3PTX8QE8cb2T+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hey, On Fri, May 22, 2015 at 09:58:10AM -0400, Frediano Ziglio wrote: > > > } > > > =20 > > > 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, s= truct > > > qxl_bo *surf, > > > mutex_lock(&qdev->update_area_mutex); > > > qdev->ram_header->update_area =3D *area; > > > qdev->ram_header->update_surface =3D surface_id; > > > - ret =3D wait_for_io_cmd_user(qdev, 0, QXL_IO_UPDATE_AREA_ASYNC, tru= e); > > > + ret =3D wait_for_io_cmd_user(qdev, 0, QXL_IO_UPDATE_AREA_ASYNC, int= r); > > > 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 =3D surf->surf.width; > > > rect.top =3D 0; > > > rect.bottom =3D surf->surf.height; > > > -retry: > > > - ret =3D qxl_io_update_area(qdev, surf, &rect); > > > - if (ret =3D=3D -ERESTARTSYS) > > > - goto retry; > > > + ret =3D qxl_io_update_area(qdev, surf, &rect, false); > >=20 > > 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 ;) > >=20 > > Christophe > >=20 >=20 > 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 --Y1L3PTX8QE8cb2T+ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCAAGBQJVZCzOAAoJEKnYwhQprGyCNDkP/AuN4uJrfN/eodySHjtDlnnf 1ZYTeDjGLYTOLjdeYUGBg0fq21JfoZYArAzD9q+iA3e4xbbLyAsyfUL9JWiUHlv1 7BF4ymLXwPXQ8kpchRaDDCUCpMR1oc3ak1cnZfoD0wvzm7lRuiuR/iN7q/12YiXt XICTZV5WYZZt/Fx7dWPJaAqazqySsiptWyl3GCEBkWL+rr0IqSSmavFw6txs/gaS Cwo2PoSslYonlH9qcOJNJ0yo+KfgoZHfwhROsjPIFBf2+LdR9moZkqF3SY9vE3LZ HU732DWuGHiYOWdcB2maZQEWwUUffF6Vjq1aZaZvrGzCLBcMTPZ5qlBZVuRTgT62 tYt9Eeh3yuJvw0kZGK+MskfAC6ueiDcznYxJv6Xjg+Hk/6rreClLb9Ofwh29lOAX oZOlrSGVQekgLUApA8BlzhnoszI4TdLOhtMknJny+VFhmdtogwEyVwQXactalDWC F2eF3XPkBJQblFuPhzWQVKZrpntFhJm8PTy4laToKdw7Qhy4CI/HuOdTRk+BQJP3 XXqqgX1+hUhodMwjjXYehglv3VQT3eMGi1sZNlbXzDT15X+5qvkhp4GegUAhhnGz d899ta1nEqOkxIOvUcqaYBrNOkuOmzXvskYbFa2EVu1+mtL5W5Trvi64xxsr9zll NfES4qinmBbTSuaFypAm =C9Ot -----END PGP SIGNATURE----- --Y1L3PTX8QE8cb2T+-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/