ihandle: Add a comment on IH_OPEN/IH_REALLYCLOSE
authorAndrew Deason <adeason@sinenomine.net>
Thu, 4 Apr 2013 19:39:41 +0000 (14:39 -0500)
committerDaria Brashear <shadow@your-file-system.com>
Wed, 21 Jan 2015 15:35:44 +0000 (10:35 -0500)
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 <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>

src/vol/ihandle.h

index 4c76467..a37898e 100644 (file)
  * 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