All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pete Zaitcev <zaitcev@redhat.com>
To: Jeff Garzik <jeff@garzik.org>
Cc: Project Hail List <hail-devel@vger.kernel.org>
Subject: [Patch 03/12] CLD: fix commentary
Date: Sat, 17 Apr 2010 22:39:12 -0600	[thread overview]
Message-ID: <20100417223912.28263c88@redhat.com> (raw)

Add and fix some comments regarding the reasons behind the pipe etc.
No code changes.

Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>

---
 lib/cldc.c |   27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

commit e675f2f316bbb24ca84c1bc23e4d1c6d53b029de
Author: Master <zaitcev@lembas.zaitcev.lan>
Date:   Sat Apr 17 19:14:30 2010 -0600

    Fix commentary.

diff --git a/lib/cldc.c b/lib/cldc.c
index c1e8993..6ab2fe4 100644
--- a/lib/cldc.c
+++ b/lib/cldc.c
@@ -1372,7 +1372,7 @@ enum {
 
 /*
  * All the error printouts are likely to be lost for daemons, but it's
- * not a big deal. We abort instead of exist to indicate that something
+ * not a big deal. We abort instead in order to indicate that something
  * went wrong, so system features should report it (usualy as a core).
  * When debugging, strace or -F mode will capture the output.
  */
@@ -1503,13 +1503,29 @@ static void ncld_p_event(void *priv, struct cldc_session *csp,
 		if (!nsess->is_up)
 			return;
 		nsess->is_up = false;
-		/* XXX wake up all I/O waiters here */
+
+		/*
+		 * The cldc layer must deliver the callbacks for all pending
+		 * CLD operations of the failed session. If we force-wake their
+		 * waiters, all sorts of funny things happen with the lifetimes
+		 * of related structures.
+		 */
+		// 
+		// g_cond_broadcast(nsess->cond);
+
 		/*
 		 * This is a trick. As a direct callback from clcd layer,
 		 * we are running under nsess->mutex, so we cannot call back
 		 * into a user of ncld. If we do, it may invoke another
 		 * ncld operation and deadlock. So, bump session callbacks
 		 * into the part of the helper thread that runs unlocked.
+		 *
+		 * Notice that we are already running on the context of the
+		 * thread that will deliver the event, so pipe really is not
+		 * needed: could as well set a flag and test it right after
+		 * the call to cldc_udp_receive_pkt(). But pipe also provides
+		 * a queue of events, just in case. It's not like these events
+		 * are super-performance critical.
 		 */
 		cmd = NCLD_CMD_SESEV;
 		write(nsess->to_thread[1], &cmd, 1);
@@ -1820,7 +1836,12 @@ int ncld_del(struct ncld_sess *nsess, const char *fname)
 		g_mutex_unlock(nsess->mutex);
 		return -rc;
 	}
-	/* XXX A delete operation is not accounted for end-session */
+	/* 
+	 * A delete operation is not accounted for end-session (e.g. nios).
+	 * This means: do not call ncld_del and ncld_close together.
+	 * The ncld_close can be invoked by ncld_sess_close, so don't
+	 * do that either.
+	 */
 	g_mutex_unlock(nsess->mutex);
 
 	rc = ncld_wait_del(&dpb);

                 reply	other threads:[~2010-04-18  4:39 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100417223912.28263c88@redhat.com \
    --to=zaitcev@redhat.com \
    --cc=hail-devel@vger.kernel.org \
    --cc=jeff@garzik.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.