From mboxrd@z Thu Jan 1 00:00:00 1970 From: nils.carlson@ericsson.com (Nils Carlson) Date: Wed, 23 Feb 2011 16:54:52 +0100 Subject: [ltt-dev] [UST PATCH] libust: Fix multiple fd close during fork In-Reply-To: <4D651DD0.8070805@gmail.com> References: <1298464816-19832-1-git-send-email-nils.carlson@ericsson.com> <4D651DD0.8070805@gmail.com> Message-ID: Yep. fixed. merged. /Nils On Wed, 23 Feb 2011, Yannick Brosseau wrote: > On 2011-02-23 07:43, Nils Carlson wrote: >> >> This sort of fixes things in a wider sense as with the original patch >> we might leave trailing fds open and a non "auto" trace would have >> been neither destroyed nor have its fds closed... >> >> Hope its ok with you yannick. Keep up the good work! > > that's fine with me. But shouldn't we also do this kind of loop for all > the "auto" in the ust_fork function? > >> >> /Nils >> >> On Wed, 23 Feb 2011, Nils Carlson wrote: >> >>> Remove superfluous fd closes during fork and also destroy all >>> traces instead of just "auto". >>> >>> Reported-by: Yannick Brosseau >>> Signed-off-by: Nils Carlson >>> --- >>> libust/tracectl.c | 17 ++++++++--------- >>> 1 files changed, 8 insertions(+), 9 deletions(-) >>> >>> diff --git a/libust/tracectl.c b/libust/tracectl.c >>> index 1bd7229..1fc0118 100644 >>> --- a/libust/tracectl.c >>> +++ b/libust/tracectl.c >>> @@ -1569,6 +1569,7 @@ static void ust_fork(void) >>> { >>> struct ust_buffer *buf, *buf_tmp; >>> struct ustcomm_sock *sock, *sock_tmp; >>> + struct ust_trace *trace, *trace_tmp; >>> int result; >>> >>> /* FIXME: technically, the locks could have been taken before the >>> fork */ >>> @@ -1589,18 +1590,16 @@ static void ust_fork(void) >>> /* Delete all blocked consumers */ >>> cds_list_for_each_entry_safe(buf, buf_tmp, &open_buffers_list, >>> open_buffers_list) { >>> - result = close(buf->data_ready_fd_read); >>> - if (result == -1) { >>> - PERROR("close"); >>> - } >>> - result = close(buf->data_ready_fd_write); >>> - if (result == -1) { >>> - PERROR("close"); >>> - } >>> cds_list_del(&buf->open_buffers_list); >>> } >>> >>> - ltt_trace_destroy("auto", 1); >>> + /* >>> + * FIXME: This could be prettier, we loop over the list twice and >>> + * following good locking practice should lock around the loop >>> + */ >>> + cds_list_for_each_entry_safe(trace, trace_tmp, <t_traces.head, >>> list) { >>> + ltt_trace_destroy(trace->trace_name, 1); >>> + } >>> >>> /* Clean up the listener socket and epoll, keeping the scoket >>> file */ >>> ustcomm_del_named_sock(listen_sock, 1); >>> -- >>> 1.7.1 >>> >>> > >