sys-io-error-handling.patch:
----------------------------
This patch fixes a variety of error handling issues in sys-io.sm. The
error handling paths in here still seem like they may be a little
fragile, but this patch addresses a couple of specific areas where we
could report an error faster or more accurately.
- The status returned in the final write ack was being ignored.
- If the final write ack comes back before the flow completes, then we
should go ahead and cancel the flow and honor the ack status. Otherwise
we may end up waiting a long time for the flow to finish and will
probably replace the valid error code from the server with the timeout
error code from the flow.
- The return code of decode function for final write ack was being ignored.
- Added an extra check to make sure that retries are skipped if an I/O
operation is explicitly cancelled.
flow-cancel-complete.patch:
--------------------------
I don't know that this is ever triggered, but this patch addresses a
possible bug in the flow protocol. After the cancellation function is
called, the flow protocol should immediately check to see if the flow
has been marked as complete. It is possible that the flow will change
state during that function.
test-setgid-arg-parsing.patch
---------------------------
This fixes a minor bug in the argument parsing for the test-setgid
program. It needs to reset some getopt flags before running getopt in a
second loop to retrieve arguments specific to this test case.
trove-nothread-build.patch
--------------------------
This is a trivial patch that fixes a build error if trove is compiled
with threading disabled.
---------------------
PatchSet 476
Date: 2006/03/06 21:44:36
Author: pcarns
Branch: HEAD
Tag: (none)
Log:
added test for completion in fp_multiqueue_cancel() path
[artf32991]
Members:
src/io/flow/flowproto-bmi-trove/flowproto-multiqueue.c:1.12->1.13
Index: src/io/flow/flowproto-bmi-trove/flowproto-multiqueue.c
diff -u src/io/flow/flowproto-bmi-trove/flowproto-multiqueue.c:1.12 src/io/flow/flowproto-bmi-trove/flowproto-multiqueue.c:1.13
--- src/io/flow/flowproto-bmi-trove/flowproto-multiqueue.c:1.12 Mon Mar 6 14:43:39 2006
+++ src/io/flow/flowproto-bmi-trove/flowproto-multiqueue.c Mon Mar 6 14:44:36 2006
@@ -476,6 +476,12 @@
lld(flow_d->total_transferred));
assert(flow_d->state == FLOW_TRANSMITTING);
handle_io_error(-PVFS_ECANCEL, NULL, flow_data);
+ if(flow_data->parent->state == FLOW_COMPLETE)
+ {
+ gen_mutex_unlock(flow_data->parent->flow_mutex);
+ FLOW_CLEANUP(flow_data);
+ return(0);
+ }
}
else
{
---------------------
PatchSet 479
Date: 2006/03/08 19:52:46
Author: pcarns
Branch: HEAD
Tag: (none)
Log:
fix multiple sys-io.sm bugs:
- don't ignore status in final write ack
- if final write ack comes early, go ahead and cancel flow
- don't ignore return code of decode function for final write ack
- make sure to skip retries if op_cancelled is set
[artf32976]
Members:
src/client/sysint/sys-io.sm:1.20->1.21
Index: src/client/sysint/sys-io.sm
diff -u src/client/sysint/sys-io.sm:1.20 src/client/sysint/sys-io.sm:1.21
--- src/client/sysint/sys-io.sm:1.20 Fri Feb 17 08:09:43 2006
+++ src/client/sysint/sys-io.sm Wed Mar 8 12:52:46 2006
@@ -1084,6 +1084,22 @@
cur_ctx->msg_send_has_been_posted = 0;
cur_ctx->msg_recv_has_been_posted = 0;
}
+ else
+ {
+ /* if we successfully received an ack. If the flow has _not_
+ * finished, then we should go ahead and cancel it (the ack is
+ * reporting an error, no point in waiting for flow timeout).
+ */
+ if(cur_ctx->flow_in_progress != 0)
+ {
+ job_flow_cancel(cur_ctx->flow_job_id,
+ pint_client_sm_context);
+ /* bump up the retry count to prevent the state machine from
+ * restarting after this error propigates
+ */
+ sm_p->u.io.retry_count = sm_p->msgarray_params.retry_limit;
+ }
+ }
}
check_next_step:
@@ -1120,7 +1136,7 @@
if (!cur_ctx->msg_send_has_been_posted)
break;
}
- if (i < sm_p->u.io.datafile_count) {
+ if (i < sm_p->u.io.datafile_count && !sm_p->op_cancelled) {
gossip_debug(GOSSIP_IO_DEBUG,
"*** %s: some msgpairs to repost\n", __func__);
js_p->error_code = IO_RETRY;
@@ -1192,8 +1208,8 @@
}
else
{
- PVFS_perror_gossip(
- "io_check_context_status found error", ret);
+ gossip_debug(GOSSIP_IO_DEBUG,
+ "io_check_context_status found error: %d", ret);
}
break;
}
@@ -1260,11 +1276,16 @@
goto analyze_results_exit;
}
+ /* all other errors we should just propigate immediately */
+ if(ret != 0)
+ {
+ js_p->error_code = ret;
+ goto analyze_results_exit;
+ }
+
gossip_debug(GOSSIP_IO_DEBUG, "total bytes transferred is %lld\n",
lld(sm_p->u.io.total_size));
- ret = 0;
-
if(sm_p->u.io.io_type == PVFS_IO_WRITE)
{
js_p->error_code = 0;
@@ -1968,30 +1989,11 @@
cur_ctx->msg.recv_status.error_code, cur_ctx);
ret = cur_ctx->msg.recv_status.error_code;
}
- else if (cur_ctx->flow_status.error_code)
- {
- gossip_debug(GOSSIP_IO_DEBUG,
- " error (%d) in flow for context %p\n",
- cur_ctx->flow_status.error_code, cur_ctx);
- PINT_flow_reset(&cur_ctx->flow_desc);
- ret = cur_ctx->flow_status.error_code;
- }
- else if (io_type == PVFS_IO_READ)
- {
- gossip_debug(
- GOSSIP_IO_DEBUG, " %lld bytes read from context %p\n",
- lld(cur_ctx->flow_desc.total_transferred), cur_ctx);
-
- /* size for reads are reported in the flow */
- *total_size += cur_ctx->flow_desc.total_transferred;
-
- /*
- we can't reset the flow here in case we have to do a zero
- fill adjustment that we haven't detected yet
- */
- }
else if (io_type == PVFS_IO_WRITE)
{
+ /* we check the write ack status before the flow status so that the
+ * error code that the server reported in the ack takes precedence
+ */
if (cur_ctx->write_ack.recv_status.error_code)
{
gossip_debug(
@@ -2023,6 +2025,9 @@
cur_ctx);
*total_size += resp->u.write_completion.total_completed;
+
+ /* pass along the error code from the server as well */
+ ret = resp->status;
PINT_decode_release(&decoded_resp, PINT_DECODE_RESP);
}
@@ -2036,7 +2041,38 @@
cur_ctx->write_ack.encoded_resp_p,
cur_ctx->write_ack.max_resp_sz, BMI_RECV);
}
+ else if (cur_ctx->flow_status.error_code)
+ {
+ gossip_debug(GOSSIP_IO_DEBUG,
+ " error (%d) in flow for context %p\n",
+ cur_ctx->flow_status.error_code, cur_ctx);
+ PINT_flow_reset(&cur_ctx->flow_desc);
+ ret = cur_ctx->flow_status.error_code;
+ }
+ }
+ else if (cur_ctx->flow_status.error_code)
+ {
+ gossip_debug(GOSSIP_IO_DEBUG,
+ " error (%d) in flow for context %p\n",
+ cur_ctx->flow_status.error_code, cur_ctx);
+ PINT_flow_reset(&cur_ctx->flow_desc);
+ ret = cur_ctx->flow_status.error_code;
}
+ else if (io_type == PVFS_IO_READ)
+ {
+ gossip_debug(
+ GOSSIP_IO_DEBUG, " %lld bytes read from context %p\n",
+ lld(cur_ctx->flow_desc.total_transferred), cur_ctx);
+
+ /* size for reads are reported in the flow */
+ *total_size += cur_ctx->flow_desc.total_transferred;
+
+ /*
+ we can't reset the flow here in case we have to do a zero
+ fill adjustment that we haven't detected yet
+ */
+ }
+
return ret;
}
---------------------
PatchSet 499
Date: 2006/03/20 21:55:57
Author: pcarns
Branch: HEAD
Tag: (none)
Log:
fixed a bug either introduced or exposed by previous sys-io.sm error handling
cleanups- default to 0 error code when processing results of small io write
unless we find that something has gone wrong.
[artf33105]
Members:
src/client/sysint/sys-io.sm:1.21->1.22
Index: src/client/sysint/sys-io.sm
diff -u src/client/sysint/sys-io.sm:1.21 src/client/sysint/sys-io.sm:1.22
--- src/client/sysint/sys-io.sm:1.21 Wed Mar 8 12:52:46 2006
+++ src/client/sysint/sys-io.sm Mon Mar 20 14:55:57 2006
@@ -1188,6 +1188,7 @@
* small I/O state machine sets the total_size directly
*/
+ ret = 0;
if(!sm_p->u.io.small_io)
{
for(i = 0; i < sm_p->u.io.context_count; i++)
diff -Naur pvfs2/test/client/vfs/test-setgid.c pvfs2-new/test/client/vfs/test-setgid.c
--- pvfs2/test/client/vfs/test-setgid.c 2006-03-09 21:42:05.000000000 +0100
+++ pvfs2-new/test/client/vfs/test-setgid.c 2006-03-26 18:22:55.000000000 +0200
@@ -470,7 +470,7 @@
}
memset(opts, 0, sizeof(struct setgid_options));
-
+ optind = 0;
while((ret = getopt_long_only(argc, argv, "", long_opts, &option_index)) != -1)
{
switch (ret)
---------------------
PatchSet 497
Date: 2006/03/17 23:00:34
Author: pcarns
Branch: HEAD
Tag: (none)
Log:
minor fix in trove to allow it to be built without thread support again
Members:
src/io/trove/trove-dbpf/dbpf-dspace.c:1.9->1.10
Index: src/io/trove/trove-dbpf/dbpf-dspace.c
diff -u src/io/trove/trove-dbpf/dbpf-dspace.c:1.9 src/io/trove/trove-dbpf/dbpf-dspace.c:1.10
--- src/io/trove/trove-dbpf/dbpf-dspace.c:1.9 Wed Jan 4 08:51:01 2006
+++ src/io/trove/trove-dbpf/dbpf-dspace.c Fri Mar 17 16:00:34 2006
@@ -34,7 +34,6 @@
#include "dbpf-thread.h"
#include "pvfs2-internal.h"
-extern gen_mutex_t dbpf_attr_cache_mutex;
extern pthread_cond_t dbpf_op_completed_cond;
extern dbpf_op_queue_p dbpf_completion_queue_array[TROVE_MAX_CONTEXTS];
extern gen_mutex_t *dbpf_completion_queue_array_mutex[TROVE_MAX_CONTEXTS];
@@ -42,6 +41,7 @@
extern struct qlist_head dbpf_op_queue;
extern gen_mutex_t dbpf_op_queue_mutex;
#endif
+extern gen_mutex_t dbpf_attr_cache_mutex;
extern struct PINT_perf_counter* PINT_server_pc;
int64_t s_dbpf_metadata_writes = 0, s_dbpf_metadata_reads = 0;
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
