Re: [Patch 2/7] tabled: fix the endless recusion when reading long objects

2010-04-06 Thread Jeff Garzik

On 04/01/2010 09:51 PM, Pete Zaitcev wrote:

At certain network and disk speeds, tabled can blow its stack by
filling it with (essentially) endless recursion:

#2  0x0040c077 in cli_write_free (cli=value optimized out, tmp=
 0x7bb910, done=value optimized out) at server.c:397
#3  0x0040ca55 in cli_writable (cli=0x686e90) at server.c:525
#4  0x0040da65 in cli_write_start (cli=0x686e90) at server.c:561
#5  0x00408ad5 in object_get_poke (cli=0x686e90) at object.c:1039
#6  0x0040c077 in cli_write_free (cli=value optimized out, tmp=
 0x7bb8d0, done=value optimized out) at server.c:397
#7  0x0040ca55 in cli_writable (cli=0x686e90) at server.c:525
#8  0x0040da65 in cli_write_start (cli=0x686e90) at server.c:561
#9  0x00408ad5 in object_get_poke (cli=0x686e90) at object.c:1039
#10 0x0040c077 in cli_write_free (cli=value optimized out, tmp=
 0x7bb890, done=value optimized out) at server.c:397

The fix is to deliver callbacks only from the top level.

Callbacks must be delivered every time a send is completed,
which amounts to every call to is_writeable(). Since there
is a large number of callers to it, we found it advantageous
to run callbacks from every source of events. In other words,
every function that is passed to event_set must invoke
cli_write_run_compl. Mind that storage.c contains calls to
event_set.

Signed-off-by: Pete Zaitcevzait...@redhat.com

---
  server/object.c |4 +++
  server/server.c |   52 +++---
  server/tabled.h |6 +
  3 files changed, 50 insertions(+), 12 deletions(-)


applied 2-7


--
To unsubscribe from this list: send the line unsubscribe hail-devel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Patch 2/7] tabled: fix the endless recusion when reading long objects

2010-04-01 Thread Pete Zaitcev
At certain network and disk speeds, tabled can blow its stack by
filling it with (essentially) endless recursion:

#2  0x0040c077 in cli_write_free (cli=value optimized out, tmp=
0x7bb910, done=value optimized out) at server.c:397
#3  0x0040ca55 in cli_writable (cli=0x686e90) at server.c:525
#4  0x0040da65 in cli_write_start (cli=0x686e90) at server.c:561
#5  0x00408ad5 in object_get_poke (cli=0x686e90) at object.c:1039
#6  0x0040c077 in cli_write_free (cli=value optimized out, tmp=
0x7bb8d0, done=value optimized out) at server.c:397
#7  0x0040ca55 in cli_writable (cli=0x686e90) at server.c:525
#8  0x0040da65 in cli_write_start (cli=0x686e90) at server.c:561
#9  0x00408ad5 in object_get_poke (cli=0x686e90) at object.c:1039
#10 0x0040c077 in cli_write_free (cli=value optimized out, tmp=
0x7bb890, done=value optimized out) at server.c:397

The fix is to deliver callbacks only from the top level.

Callbacks must be delivered every time a send is completed,
which amounts to every call to is_writeable(). Since there
is a large number of callers to it, we found it advantageous
to run callbacks from every source of events. In other words,
every function that is passed to event_set must invoke
cli_write_run_compl. Mind that storage.c contains calls to
event_set.

Signed-off-by: Pete Zaitcev zait...@redhat.com

---
 server/object.c |4 +++
 server/server.c |   52 +++---
 server/tabled.h |6 +
 3 files changed, 50 insertions(+), 12 deletions(-)

commit 88959fb8c4cc8232f92805618c5bcaeee9099390
Author: Pete Zaitcev zait...@yahoo.com
Date:   Thu Apr 1 19:04:13 2010 -0600

Fix the endless recursion when reading long objects.

diff --git a/server/object.c b/server/object.c
index d61a446..6d32316 100644
--- a/server/object.c
+++ b/server/object.c
@@ -994,6 +994,9 @@ static bool object_get_poke(struct client *cli)
char *buf;
ssize_t bytes;
 
+   /* XXX flow control - what treshold? */
+   /*  if (cli_wqueued(cli) = 10) return false; */
+
buf = malloc(CLI_DATA_BUF_SZ);
if (!buf)
return false;
@@ -1052,6 +1055,7 @@ static bool object_get_more(struct client *cli, void 
*cb_data, bool done)
 static void object_get_event(struct open_chunk *ochunk)
 {
object_get_poke(ochunk-cli);
+   cli_write_run_compl(ochunk-cli);
 }
 
 bool object_get_body(struct client *cli, const char *user, const char *bucket,
diff --git a/server/server.c b/server/server.c
index 72db151..600c8de 100644
--- a/server/server.c
+++ b/server/server.c
@@ -380,6 +380,8 @@ static void stats_dump(void)
   tabled_srv.stats.event,
   tabled_srv.stats.tcp_accept,
   tabled_srv.stats.opt_write);
+   applog(LOG_INFO, DEBUG: max_write_buf %lu,
+  tabled_srv.stats.max_write_buf);
stor_stats();
rep_stats();
 }
@@ -405,16 +407,24 @@ bool stat_status(struct client *cli, GList *content)
content = g_list_append(content, str);
if (asprintf(str,
 pStats: 
-poll %lu event %lu tcp_accept %lu opt_write %lu/p\r\n,
+poll %lu event %lu tcp_accept %lu opt_write %lu/p\r\n
+pDebug: max_write_buf %lu/p\r\n,
 tabled_srv.stats.poll,
 tabled_srv.stats.event,
 tabled_srv.stats.tcp_accept,
-tabled_srv.stats.opt_write)  0)
+tabled_srv.stats.opt_write,
+tabled_srv.stats.max_write_buf)  0)
return false;
content = g_list_append(content, str);
return true;
 }
 
+static void cli_write_complete(struct client *cli, struct client_write *tmp)
+{
+   list_del(tmp-node);
+   list_add_tail(tmp-node, cli-write_compl_q);
+}
+
 static bool cli_write_free(struct client *cli, struct client_write *tmp,
   bool done)
 {
@@ -433,11 +443,28 @@ static void cli_write_free_all(struct client *cli)
 {
struct client_write *wr, *tmp;
 
+   list_for_each_entry_safe(wr, tmp, cli-write_compl_q, node) {
+   cli_write_free(cli, wr, true);
+   }
list_for_each_entry_safe(wr, tmp, cli-write_q, node) {
cli_write_free(cli, wr, false);
}
 }
 
+bool cli_write_run_compl(struct client *cli)
+{
+   struct client_write *wr;
+   bool do_loop;
+
+   do_loop = false;
+   while (!list_empty(cli-write_compl_q)) {
+   wr = list_entry(cli-write_compl_q.next, struct client_write,
+   node);
+   do_loop |= cli_write_free(cli, wr, true);
+   }
+   return do_loop;
+}
+
 static void cli_free(struct client *cli)
 {
cli_write_free_all(cli);
@@ -455,6 +482,9 @@ static void cli_free(struct client *cli)
 
req_free(cli-req);
 
+   if