From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756383AbbJAGEo (ORCPT ); Thu, 1 Oct 2015 02:04:44 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:55228 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753243AbbJAGEj (ORCPT ); Thu, 1 Oct 2015 02:04:39 -0400 Date: Thu, 1 Oct 2015 08:04:36 +0200 From: Markus Pargmann To: Ben Hutchings Cc: Jens Axboe , linux-kernel@vger.kernel.org, nbd-general@lists.sourceforge.net, kernel@pengutronix.de, Michal Belczyk , Hermann Lauer Subject: Re: [PATCH 01/10] nbd: Fix timeout detection Message-ID: <20151001060436.GN19121@pengutronix.de> References: <1439792409-28543-1-git-send-email-mpa@pengutronix.de> <1439792409-28543-2-git-send-email-mpa@pengutronix.de> <1443400064.2517.16.camel@decadent.org.uk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="prC3/KjdfqNV7evK" Content-Disposition: inline In-Reply-To: <1443400064.2517.16.camel@decadent.org.uk> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 08:02:26 up 11 days, 19:23, 97 users, load average: 4.13, 5.30, 4.45 User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::7 X-SA-Exim-Mail-From: mpa@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --prC3/KjdfqNV7evK Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Mon, Sep 28, 2015 at 01:27:44AM +0100, Ben Hutchings wrote: > On Mon, 2015-08-17 at 08:20 +0200, Markus Pargmann wrote: > > At the moment the nbd timeout just detects hanging tcp operations. This > > is not enough to detect a hanging or bad connection as expected of a > > timeout. > >=20 > > This patch redesigns the timeout detection to include some more cases. > > The timeout is now in relation to replies from the server. If the server > > does not send replies within the timeout the connection will be shut > > down. > >=20 > > The patch adds a continous timer 'timeout_timer' that is setup in one of > > two cases: > > - The request list is empty and we are sending the first request out to > > the server. We want to have a reply within the given timeout, > > otherwise we consider the connection to be dead. > > - A server response was received. This means the server is still > > communicating with us. The timer is reset to the timeout value. > >=20 > > The timer is not stopped if the list becomes empty. It will just trigger > > a timeout which will directly leave the handling routine again as the > > request list is empty. > >=20 > > The whole patch does not use any additional explicit locking. The > > list_empty() calls are safe to be used concurrently. The timer is locked > > internally as we just use mod_timer and del_timer_sync(). >=20 > This is crazy. The timer is locked internally but the tasks are not. > So it is possible for the timeout handler to kill a task after it > exited from nbd_do_it()/nbd_thread_recv(), or after it exited entirely > (use-after-free). Indeed, thanks. I am working on a patch to fix this issue. Best Regards, Markus >=20 > [...] > > +> > task =3D READ_ONCE(nbd->task_send); > > +> > if (task) > > +> > > force_sig(SIGKILL, nbd->task_send); > [...] >=20 > And this is just... what? What is the point of using READ_ONCE() if > you're going to look up nbd->task_send again? >=20 > Ben. >=20 > --=20 > Ben Hutchings > All extremists should be taken out and shot. --=20 Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | --prC3/KjdfqNV7evK Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWDMz0AAoJEEpcgKtcEGQQy5kP/2koo/Rg7ntcvbFFTZeexTkJ hVw7TyzqSs4yIuH5HZaAm6djocqhAwMQ3u0MlmGnqRLS3F7RTFMvzYHp2QgwiGbn 4cEwih+rIHMO02rVvzjzYrwinYU8XX5bBtrRYsvrCyEcWL+SQiiKp9g0JKU8dBPE PQqk9zFI5rBCCh7uNhxFQmgajgxvhEluQ7lGyEqbCx338ANoQIyLLUb/aWeGvlBY F34pZPn9hm84f0xXkdFyIM4FyPS555CnCt/hkvMtpgWZzLXQiP4+x3SDUWCpDnMm p+hZt5jHOyhQW6XyISwuH675hx8ec8rgCSS5IKr0uncmvRma45MQJCt1jlF4qgX+ VtY0kD/SnDLPKmXTQ9emwX5lkPfZT8ezW4ZK9ElgmW9oek1w23R7fYEZnOf031tG yu1w1z0p+bbDaom+kSm+UVmxvps5o62bkcDp1mwYUYK1eUxRV4akJjTySTNybjAj /UadWD60FKeX315E+q47DkjNSZnIqD7ghSWRqrjy9sPo9f7Iu068RLz/J2mrIxuz Vr1KFs8Y7CA0I5gaTp6Fc+YQSeqeQyt1TaJoLuczSK5wXYsLlErGb4F4k0ce/GR3 XibCz7lZdVYfwtNR88eneUc7NhmLBN0+tdX16E7IkoZzTasdR6YkVijMbGrZ6KG4 eYXfeSjti7Xn5z3D6ZOd =HDk0 -----END PGP SIGNATURE----- --prC3/KjdfqNV7evK--