<div class="gmail_quote">On Thu, Mar 29, 2012 at 8:28 AM, Jed Brown <span dir="ltr"><<a href="mailto:jedbrown@mcs.anl.gov">jedbrown@mcs.anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><div class="gmail_quote">On Thu, Mar 29, 2012 at 07:16, Dmitry Karpeev <span dir="ltr"><<a href="mailto:karpeev@mcs.anl.gov" target="_blank">karpeev@mcs.anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>Here the problem isn't in needing to call PCReset() twice per se. The issue is that if the user uses the same PC object in</div><div>two different KSPs and destroys one ksp, that will cause a PCReset() and invalidate the pc for the other ksp -- all despite increfing the pc.</div>
</blockquote></div><br></div><div>Yeah, this is tricky. In this case, is it acceptable to break the dependency explicitly using</div><div><br></div><div>PCSetType(ksp,PCNONE); // will decrement the reference count, but not PCReset()</div>
<div>KSPDestroy(&ksp);</div><div><br></div><div><br></div><div>Skipping the reset means that KSPReset() is basically always incorrect if the user created the PC (for example). I think a top-level KSPReset() should always reset the PC. </div>
</blockquote><div>I am not sure how to solve the problem of the relationship between KSPReset() and PCReset() when the pc is being shared.</div><div>How useful are KSPReset() and PCReset() anyway? All but one uses I found were "trivial" -- called from a PCReset_XXX, where XXX uses a KSP internally. The only nontrivial use I found was in VIRS, where the KSP and PC are reset when the active set changes. Would a simple KSPDestroy() do here instead? Perhaps there are uses of KSPReset()/PCReset() in user code, though.</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>Now maybe ksp->pc should be withheld by KSPDestroy() before calling KSPReset(); if the PC refct hits 0, it will be destroyed in the subsequent PCDestroy().</div>
</blockquote><div><br></div><div>The present problem seems to come from a misuse of KSPReset()/PCDestroy() as a partial destructor inside KSPDestroy():</div><div>the intent here is to destroy the ksp and the underlying pc, and in the case of the pc this should be protected by the refcount.</div>
<div>However, PCDestroy() is being entered "from a side" for the intended meaning -- via KSPReset(), not PCDestroy() and the refcount check isn't being triggered.</div><div><br></div><div>One solution would be not to use KSPReset()/PCReset() as a partial destructor, but that would require duplicating that code directly in KSPDestroy()/PCDestroy(), or we could move this code to KSPReset_Private() that would not cascade to PCReset_Private(). Then we could have:</div>
<div>PetscErrorCode KSPReset(KSP ksp) {</div><div> ... KSPReset_Private(ksp); PCReset_Private(ksp->pc); ...</div><div>}</div><div>PetscErrorCode KSPDestroy(KSP *ksp) {</div><div> ... KSPReset_Private(*ksp); PCDestroy((*ksp)->pc); ...</div>
<div>}</div><div>PetscErrorCode PCDestroy(PC *pc) {</div><div> if (--((PetscObject)(*pc))->refct > 0) {*pc = 0; PetscFunctionReturn(0);} ...</div><div>}</div><div><br></div><div>Dmitry.</div></div><br>