That may be. The thing that is a little tricky is that technically the
mgmt code can be run in a handful of different modes (thread pool,
thread per op, polling etc.) and we weren't entirely sure if one of the
other modes would need the entry to survive longer than the
complete_op() function or not. This patch specifically fixes the leak
for the threaded-queue mode that trove-directio uses and ignores the
other cases since we have no convenient way to test them.
I kind of doubt anyone will ever use the other modes, though. Maybe it
is more practical in the long term to do the cleanup inside of
complete_op, but then also get rid of the code for the other N modes to
simplify things and make it so we don't have to worry about the other paths?
-Phil
On 07/07/2011 06:12 PM, Becky Ligon wrote:
Yes. I agree. That's exactly what I had found, too, but I think that
code belongs inside of the PINT_manager_complete_op function. Thoughts?
Becky
On Thu, Jul 7, 2011 at 5:06 PM, Benjamin Severs
<[email protected] <mailto:[email protected]>> wrote:
Thanks to some direction by Phil Carns, we've managed to correct
the memory
leak that was affecting directio operations. Below is the patch
that addresses
the issue. The problem was that once all the *fast* calls were
switched to
*safe* calls, the proper cleanup routines were never being called.
We simply
placed the cleanup logic that was present in the
PINT_context_test_* methods
after the PINT_manager_complete_op call.
diff --git a/src/common/mgmt/pint-worker-threaded-queues.c
b/src/common/mgmt/pint-worker-threaded-queues.c
index 1359768..a088e3f 100644
--- src/common/mgmt/pint-worker-threaded-queues.c
+++ src/common/mgmt/pint-worker-threaded-queues.c
@@ -423,6 +423,7 @@ static void
*PINT_worker_queues_thread_function(void *
ptr)
{
for(i = 0; i < op_count; ++i)
{
+ struct PINT_op_entry *op_entry;
op = PINT_op_from_qentry(qentries[i]);
/* service the operation */
ret = PINT_manager_service_op(
@@ -440,6 +441,14 @@ static void
*PINT_worker_queues_thread_function(void *
ptr)
/* fatal if we can't complete an op */
goto free_ops;
}
+
+ op_entry = id_gen_safe_lookup(op->id);
+ if(op_entry)
+ {
+ id_gen_safe_unregister(op_entry->op.id
<http://op.id>);
+ free(op_entry);
+ op_entry = NULL;
+ }
}
}
}
--
Benjamin Severs
_______________________________________________
Pvfs2-developers mailing list
[email protected]
<mailto:[email protected]>
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
--
Becky Ligon
OrangeFS Support and Development
Omnibond Systems
Anderson, South Carolina
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
