Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1233804
| From | Ben Hutchings <ben@decadent.org.uk> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 01/10] nbd: Fix timeout detection |
| Date | 2015-09-28 02:30 +0200 |
| Message-ID | <qduum-6iZ-1@gated-at.bofh.it> (permalink) |
| References | <pYm5I-7AK-3@gated-at.bofh.it> <pYm5I-7AK-5@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
[Multipart message — attachments visible in raw view] - view raw
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. > > 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. > > 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. > > 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. > > 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(). 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). [...] > +> > task = READ_ONCE(nbd->task_send); > +> > if (task) > +> > > force_sig(SIGKILL, nbd->task_send); [...] And this is just... what? What is the point of using READ_ONCE() if you're going to look up nbd->task_send again? Ben. -- Ben Hutchings All extremists should be taken out and shot.
Back to linux.kernel | Previous | Next — Next in thread | Find similar | Unroll thread
Re: [PATCH 01/10] nbd: Fix timeout detection Ben Hutchings <ben@decadent.org.uk> - 2015-09-28 02:30 +0200 Re: [PATCH 01/10] nbd: Fix timeout detection Markus Pargmann <mpa@pengutronix.de> - 2015-10-01 08:10 +0200
csiph-web