Hi Pete, Phil,
something like the attached patch? does not incude the ib and gm changes..
Murali
On Mon, 21 Aug 2006, Pete Wyckoff wrote:
> [EMAIL PROTECTED] wrote on Mon, 21 Aug 2006 14:32 -0500:
> > Sam and I raised this question for the following reason (motivated during
> > the discussion of
> > BMI-MX's design with Scott)
> > Suppose a BMI method wants to keep around a pre-allocated pool of
> > unexpected buffers, it seems wasteful to copy them into a user buffer and
> > then free it again.
> > So we were kind of wondering if a test_unexpected() returns one of those
> > preallocated buffers (and mark them internally as being used) and then
> > the caller can then call BMI_memfree() which will return it to the pool of
> > unexpected buffers (by marking them as free again).
> >
> > But if this does not appeal to you guys, I suspect the unexpected_free()
> > callback function would also be better than free()..
>
> I think doing what Phil suggested with BMI_unexpected_free() is the
> best way to go. This is the same thing you're describing except it
> makes explicit the separate functions of: 1) freeing
> network-registered memory (BMI_memfree), and 2) acknowledging an
> unexpected message has been serviced (BMI_unexpected_free).
>
> You'll just have to go through the code and audit all users of
> testunexpected() to make sure they call your new function instead of
> normal free(); it'll have to be mandatory from now on. And add
> stubs in bmi_* that implement this by calling free().
>
> Don't expect to see huge performance gains by doing this, though, as
> unexpected messages tend to be pretty small and everything will sit
> in L2 anyawy. But saving the malloc/memcpy/free is probably worth
> the expansion of the API, I'll agree. It'll also clear up any
> confusion about the lifetime of returned buffers, and bring
> responsibility for flow control of unexpected messages up to the
> application where it belongs. Once you toss in these changes, I'll
> switch bmi_ib over to be clever with unexpected buffers too, as you
> describe.
>
> -- Pete
>
>
Index: src/io/bmi/bmi-method-support.h
===================================================================
RCS file: /projects/cvsroot/pvfs2-1/src/io/bmi/bmi-method-support.h,v
retrieving revision 1.24
diff -u -r1.24 bmi-method-support.h
--- src/io/bmi/bmi-method-support.h 24 May 2006 13:24:49 -0000 1.24
+++ src/io/bmi/bmi-method-support.h 22 Aug 2006 13:11:03 -0000
@@ -79,6 +79,9 @@
int (*BMI_meth_memfree) (void *,
bmi_size_t,
enum bmi_op_type);
+
+ int (*BMI_meth_unexpected_free) (void *);
+
int (*BMI_meth_post_send) (bmi_op_id_t *,
method_addr_p,
const void *,
Index: src/io/bmi/bmi.c
===================================================================
RCS file: /projects/cvsroot/pvfs2-1/src/io/bmi/bmi.c,v
retrieving revision 1.72
diff -u -r1.72 bmi.c
--- src/io/bmi/bmi.c 11 Aug 2006 19:18:05 -0000 1.72
+++ src/io/bmi/bmi.c 22 Aug 2006 13:11:03 -0000
@@ -1092,6 +1092,38 @@
return (ret);
}
+/** Acknowledge that an unexpected message has been
+ * serviced that was returned from BMI_test_unexpected().
+ *
+ * \return 0 on success, -errno on failure.
+ */
+int BMI_unexpected_free(PVFS_BMI_addr_t addr,
+ void *buffer)
+{
+ ref_st_p tmp_ref = NULL;
+ int ret = -1;
+
+ /* find a reference that matches this address */
+ gen_mutex_lock(&ref_mutex);
+ tmp_ref = ref_list_search_addr(cur_ref_list, addr);
+ if (!tmp_ref)
+ {
+ gen_mutex_unlock(&ref_mutex);
+ return (bmi_errno_to_pvfs(-EINVAL));
+ }
+ gen_mutex_unlock(&ref_mutex);
+
+ if (!tmp_ref->interface->BMI_meth_unexpected_free)
+ {
+ gossip_err("unimplemented BMI_meth_unexpected_free callback\n");
+ return bmi_errno_to_pvfs(-EOPNOTSUPP);
+ }
+ /* free the memory */
+ ret = tmp_ref->interface->BMI_meth_unexpected_free(buffer);
+
+ return (ret);
+}
+
/** Pass in optional parameters.
*
* \return 0 on success, -errno on failure.
Index: src/io/bmi/bmi.h
===================================================================
RCS file: /projects/cvsroot/pvfs2-1/src/io/bmi/bmi.h,v
retrieving revision 1.27
diff -u -r1.27 bmi.h
--- src/io/bmi/bmi.h 2 Aug 2005 17:56:12 -0000 1.27
+++ src/io/bmi/bmi.h 22 Aug 2006 13:11:03 -0000
@@ -111,6 +111,9 @@
bmi_size_t size,
enum bmi_op_type send_recv);
+int BMI_unexpected_free(PVFS_BMI_addr_t addr,
+ void *buffer);
+
int BMI_set_info(PVFS_BMI_addr_t addr,
int option,
void *inout_parameter);
Index: src/io/bmi/bmi_gm/bmi-gm.c
===================================================================
RCS file: /projects/cvsroot/pvfs2-1/src/io/bmi/bmi_gm/bmi-gm.c,v
retrieving revision 1.80
diff -u -r1.80 bmi-gm.c
--- src/io/bmi/bmi_gm/bmi-gm.c 30 May 2006 15:39:01 -0000 1.80
+++ src/io/bmi/bmi_gm/bmi-gm.c 22 Aug 2006 13:11:03 -0000
@@ -148,28 +148,28 @@
/* exported method interface */
struct bmi_method_ops bmi_gm_ops = {
- BMI_gm_method_name,
- BMI_gm_initialize,
- BMI_gm_finalize,
- BMI_gm_set_info,
- BMI_gm_get_info,
- BMI_gm_memalloc,
- BMI_gm_memfree,
- BMI_gm_post_send,
- BMI_gm_post_sendunexpected,
- BMI_gm_post_recv,
- BMI_gm_test,
- BMI_gm_testsome,
- BMI_gm_testcontext,
- BMI_gm_testunexpected,
- BMI_gm_method_addr_lookup,
- BMI_gm_post_send_list,
- BMI_gm_post_recv_list,
- BMI_gm_post_sendunexpected_list,
- BMI_gm_open_context,
- BMI_gm_close_context,
- BMI_gm_cancel,
- NULL
+ .method_name = BMI_gm_method_name,
+ .BMI_meth_initialize = BMI_gm_initialize,
+ .BMI_meth_finalize = BMI_gm_finalize,
+ .BMI_meth_set_info = BMI_gm_set_info,
+ .BMI_meth_get_info = BMI_gm_get_info,
+ .BMI_meth_memalloc = BMI_gm_memalloc,
+ .BMI_meth_memfree = BMI_gm_memfree,
+ .BMI_meth_post_send = BMI_gm_post_send,
+ .BMI_meth_post_sendunexpected_list = BMI_gm_post_sendunexpected,
+ .BMI_meth_post_recv = BMI_gm_post_recv,
+ .BMI_meth_test = BMI_gm_test,
+ .BMI_meth_testsome = BMI_gm_testsome,
+ .BMI_meth_testcontext = BMI_gm_testcontext,
+ .BMI_meth_testunexpected = BMI_gm_testunexpected,
+ .BMI_meth_method_addr_lookup = BMI_gm_method_addr_lookup,
+ .BMI_meth_post_send_list = BMI_gm_post_send_list,
+ .BMI_meth_post_recv_list = BMI_gm_post_recv_list,
+ .BMI_meth_post_sendunexpected_list = BMI_gm_post_sendunexpected_list,
+ .BMI_meth_open_context = BMI_gm_open_context,
+ .BMI_meth_close_context = BMI_gm_close_context,
+ .BMI_meth_cancel = BMI_gm_cancel,
+ .BMI_meth_rev_lookup_unexpected = NULL
};
/* module parameters */
Index: src/io/bmi/bmi_tcp/bmi-tcp.c
===================================================================
RCS file: /projects/cvsroot/pvfs2-1/src/io/bmi/bmi_tcp/bmi-tcp.c,v
retrieving revision 1.105
diff -u -r1.105 bmi-tcp.c
--- src/io/bmi/bmi_tcp/bmi-tcp.c 11 Aug 2006 19:18:05 -0000 1.105
+++ src/io/bmi/bmi_tcp/bmi-tcp.c 22 Aug 2006 13:11:04 -0000
@@ -67,6 +67,7 @@
int BMI_tcp_memfree(void *buffer,
bmi_size_t size,
enum bmi_op_type send_recv);
+int BMI_tcp_unexpected_free(void *buffer);
int BMI_tcp_post_send(bmi_op_id_t * id,
method_addr_p dest,
const void *buffer,
@@ -285,28 +286,29 @@
/* exported method interface */
struct bmi_method_ops bmi_tcp_ops = {
- BMI_tcp_method_name,
- BMI_tcp_initialize,
- BMI_tcp_finalize,
- BMI_tcp_set_info,
- BMI_tcp_get_info,
- BMI_tcp_memalloc,
- BMI_tcp_memfree,
- BMI_tcp_post_send,
- BMI_tcp_post_sendunexpected,
- BMI_tcp_post_recv,
- BMI_tcp_test,
- BMI_tcp_testsome,
- BMI_tcp_testcontext,
- BMI_tcp_testunexpected,
- BMI_tcp_method_addr_lookup,
- BMI_tcp_post_send_list,
- BMI_tcp_post_recv_list,
- BMI_tcp_post_sendunexpected_list,
- BMI_tcp_open_context,
- BMI_tcp_close_context,
- BMI_tcp_cancel,
- BMI_tcp_addr_rev_lookup_unexpected
+ .method_name = BMI_tcp_method_name,
+ .BMI_meth_initialize = BMI_tcp_initialize,
+ .BMI_meth_finalize = BMI_tcp_finalize,
+ .BMI_meth_set_info = BMI_tcp_set_info,
+ .BMI_meth_get_info = BMI_tcp_get_info,
+ .BMI_meth_memalloc = BMI_tcp_memalloc,
+ .BMI_meth_memfree = BMI_tcp_memfree,
+ .BMI_meth_unexpected_free = BMI_tcp_unexpected_free,
+ .BMI_meth_post_send = BMI_tcp_post_send,
+ .BMI_meth_post_sendunexpected = BMI_tcp_post_sendunexpected,
+ .BMI_meth_post_recv = BMI_tcp_post_recv,
+ .BMI_meth_test = BMI_tcp_test,
+ .BMI_meth_testsome = BMI_tcp_testsome,
+ .BMI_meth_testcontext = BMI_tcp_testcontext,
+ .BMI_meth_testunexpected = BMI_tcp_testunexpected,
+ .BMI_meth_method_addr_lookup = BMI_tcp_method_addr_lookup,
+ .BMI_meth_post_send_list = BMI_tcp_post_send_list,
+ .BMI_meth_post_recv_list = BMI_tcp_post_recv_list,
+ .BMI_meth_post_sendunexpected_list = BMI_tcp_post_sendunexpected_list,
+ .BMI_meth_open_context = BMI_tcp_open_context,
+ .BMI_meth_close_context = BMI_tcp_close_context,
+ .BMI_meth_cancel = BMI_tcp_cancel,
+ .BMI_meth_rev_lookup_unexpected = BMI_tcp_addr_rev_lookup_unexpected
};
/* module parameters */
@@ -638,6 +640,21 @@
buffer = NULL;
}
+ return (0);
+}
+
+/* BMI_tcp_unexpected_free()
+ *
+ * Frees memory that was returned from BMI_tcp_test_unexpected()
+ *
+ * returns 0 on success, -errno on failure
+ */
+int BMI_tcp_unexpected_free(void *buffer)
+{
+ if (buffer)
+ {
+ free(buffer);
+ }
return (0);
}
Index: src/server/pvfs2-server.c
===================================================================
RCS file: /projects/cvsroot/pvfs2-1/src/server/pvfs2-server.c,v
retrieving revision 1.222
diff -u -r1.222 pvfs2-server.c
--- src/server/pvfs2-server.c 17 Aug 2006 08:11:51 -0000 1.222
+++ src/server/pvfs2-server.c 22 Aug 2006 13:11:04 -0000
@@ -630,7 +630,6 @@
if (ret < 0)
{
PVFS_perror_gossip("Error: server_state_machine_start",
ret);
- free(s_op->unexp_bmi_buff.buffer);
/* TODO: tell BMI to drop this address? */
/* set return code to zero to allow server to continue
* processing
@@ -1781,6 +1780,14 @@
s_op->unexp_bmi_buff.addr,
s_op->unexp_bmi_buff.size);
+ /* acknowledge that the unexpected buffer has been used up.
+ * If *someone* decides to do in-place decoding, then we will have to move
+ * this back to state_machine_complete().
+ */
+ BMI_unexpected_free(s_op->unexp_bmi_buff.addr,
+ s_op->unexp_bmi_buff.buffer);
+ s_op->unexp_bmi_buff.buffer = NULL;
+
s_op->req = (struct PVFS_server_req *)s_op->decoded.buffer;
if (ret == -PVFS_EPROTONOSUPPORT)
{
@@ -1931,12 +1938,6 @@
if (ENCODING_IS_VALID(s_op->decoded.enc_type))
{
PINT_decode_release(&(s_op->decoded),PINT_DECODE_REQ);
- }
-
- /* free the buffer that the unexpected request came in on */
- if (s_op->unexp_bmi_buff.buffer)
- {
- free(s_op->unexp_bmi_buff.buffer);
}
/* Remove s_op from the inprogress_sop_list */
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
