From 71072c2bb373a6ae5edec91884985c3cfc478147 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Thu, 4 Apr 2013 14:39:41 -0500 Subject: [PATCH] ihandle: Add a comment on IH_OPEN/IH_REALLYCLOSE Currently, it's not really 'safe' in ihandle to issue an IH_OPEN against an IHandle_t when an IH_REALLYCLOSE is running at the same time. The reasons for this are explained a bit in ticket 131530 and related commits, but briefly: Say IH_OPEN runs, and drops IH_LOCK to open a new fd on disk. Then IH_REALLYCLOSE runs and closes all fds, or marks them as needing close. The running IH_OPEN then acquires IH_LOCK again and puts the newly-opened fd onto the per-IH list of fds. We now have an fd that effectively "survives" across the IH_REALLYCLOSE; effectively IH_REALLYCLOSE did not close all fds for the ih. This is possibly fixable by maintaining some extra information in IHandle_t's, but this is only a problem if we allow IH_OPEN calls to happen simultaneously with IH_REALLYCLOSE calls. Ever since ih_sync_thread was removed (or changed to not call IH_OPEN), there should be no cases where this is possible. All instances of IH_REALLYCLOSE happen during error recovery for a newly-created file, or happen under a per-vnode write lock, or for volume metadata files only happens when the ref count for a volume drops to zero when we're offlining the volume. So, do not bother trying to fix this, since doing so is currently a waste of time and the resulting complexity could introduce bugs. But in case someone ever tries to do something resulting in IH_OPEN calls executing outside the normal threads of execution, add a comment around the IH_REALLYCLOSE explanations to try and briefly explain that this cannot currently be done. Change-Id: I989806635f3b048b0c084480a4b02dc1902ba031 Reviewed-on: http://gerrit.openafs.org/9709 Tested-by: BuildBot Reviewed-by: Daria Brashear --- src/vol/ihandle.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/vol/ihandle.h b/src/vol/ihandle.h index 4c76467d2c..a37898e4e8 100644 --- a/src/vol/ihandle.h +++ b/src/vol/ihandle.h @@ -19,6 +19,16 @@ * cached by IO_CLOSE. To make sure a file descriptor is really closed call * IH_REALLYCLOSE. * + * Note that IH_REALLYCLOSE does not guarantee that all file descriptors are + * closed (or are marked as needing close) by the time it returns, in the case + * of an IH_OPEN being executed in parallel. If a separate thread is running + * an IH_OPEN on the same IHandle_t at around the same time, it is possible for + * it to open the file, then IH_REALLYCLOSE runs, and then IH_OPEN associates + * the file handle with the IHandle_t. For this reason, it is probably an + * error if it is ever possible to have an IH_OPEN call run at the same time as + * an IH_REALLYCLOSE call on the same IHandle_t. If such functionality is + * required, ihandle must be changed to accomodate this. + * * The IHandle_t also provides a place to do other optimizations. In the * NT user space file system, we keep a separate special file for the * link counts and using the IHandle_t allows keeping the details of