[OpenAFS-devel] CopyOnWrite failure: ihandle.c Patch: Code Cleanup
Srikanth Vishwanathan
vsrikanth@in.ibm.com
Mon, 1 Apr 2002 19:37:14 -0500
Hi -
This is a cleanup of the original ihandle patch. The diff
is based on ihandle.c 1.6. I don't know if this might help
with the CopyOnWrite problem, but I guess it is worth a try.
I have not compiled this on OpenAFS.
--- ihandle.c.original Mon Apr 1 18:35:46 2002
+++ ihandle.c Mon Apr 1 19:04:30 2002
@@ -259,10 +259,6 @@
IH_LOCK
- if (!ih_Inited) {
- ih_Initialize();
- }
-
/* Do we already have an open file handle for this Inode? */
for (fdP = ihP->ih_fdtail ; fdP != NULL ; fdP = fdP->fd_ihprev) {
if (fdP->fd_status != FD_HANDLE_INUSE) {
@@ -298,7 +294,7 @@
assert(fdP->fd_status == FD_HANDLE_OPEN);
DLL_DELETE(fdP, fdLruHead, fdLruTail, fd_next, fd_prev);
DLL_DELETE(fdP, fdP->fd_ih->ih_fdhead, fdP->fd_ih->ih_fdtail,
- fd_ihnext, fd_ihprev);
+ fd_ihnext, fd_ihprev);
closeFd = fdP->fd_fd;
} else {
if (fdAvailHead == NULL) {
@@ -313,6 +309,9 @@
fdP->fd_status = FD_HANDLE_INUSE;
fdP->fd_fd = fd;
fdP->fd_ih = ihP;
+
+ ihP->ih_refcnt++;
+
/* Add this handle to the Inode's list of open descriptors */
DLL_INSERT_TAIL(fdP, ihP->ih_fdhead, ihP->ih_fdtail, fd_ihnext,
fd_ihprev);
@@ -323,7 +322,6 @@
fdInUseCount -= 1;
}
- ihP->ih_refcnt++;
IH_UNLOCK
return fdP;
}
@@ -340,41 +338,21 @@
return 0;
IH_LOCK
+
assert(ih_Inited);
assert(fdInUseCount > 0);
assert(fdP->fd_status == FD_HANDLE_INUSE);
ihP = fdP->fd_ih;
- /* If a previous attempt to close ( ih_reallyclose() )
- * all fd handles failed, then the IH_REALLY_CLOSED flag is set in
- * the Inode handle so we call fd_reallyclose
+ /* Call fd_reallyclose to really close the unused file handles if
+ * the previous attempt to close (ih_reallyclose()) all file handles
+ * failed (this is determined by checking the ihandle for the flag
+ * IH_REALLY_CLOSED) or we have too many open files.
*/
-
- if ( ihP->ih_flags & IH_REALLY_CLOSED ) {
- IH_UNLOCK
- return (fd_reallyclose(fdP));
- }
-
- /* If we have too many open files then close the descriptor. If we
- * hold the last reference to the Inode handle then wait and let
- * ih_release do the work. */
- if (fdInUseCount > fdCacheSize && ihP->ih_refcnt > 1) {
- assert(fdInUseCount > 0);
- closeFd = fdP->fd_fd;
- DLL_DELETE(fdP, fdP->fd_ih->ih_fdhead, fdP->fd_ih->ih_fdtail,
- fd_ihnext, fd_ihprev);
- DLL_INSERT_TAIL(fdP, fdAvailHead, fdAvailTail, fd_next, fd_prev);
- fdP->fd_status = FD_HANDLE_AVAIL;
- fdP->fd_ih = NULL;
- fdP->fd_fd = INVALID_FD;
- ihP->ih_refcnt--;
+ if (ihP->ih_flags & IH_REALLY_CLOSED || fdInUseCount > fdCacheSize) {
IH_UNLOCK
- OS_CLOSE(closeFd);
- IH_LOCK
- fdInUseCount -= 1;
- IH_UNLOCK
- return 0;
+ return fd_reallyclose(fdP);
}
/* Put this descriptor back into the cache */
@@ -382,7 +360,8 @@
DLL_INSERT_TAIL(fdP, fdLruHead, fdLruTail, fd_next, fd_prev);
/* If this is not the only reference to the Inode then we can
decrement
- * the reference count, otherwise we need to call ih_release. */
+ * the reference count, otherwise we need to call ih_release.
+ */
if (ihP->ih_refcnt > 1) {
ihP->ih_refcnt--;
IH_UNLOCK
@@ -395,7 +374,8 @@
}
/*
- * Return a file descriptor handle to the cache
+ * Actually close the file descriptor handle and return it to
+ * the free list.
*/
int fd_reallyclose(FdHandle_t *fdP)
{
@@ -406,6 +386,7 @@
return 0;
IH_LOCK
+
assert(ih_Inited);
assert(fdInUseCount > 0);
assert(fdP->fd_status == FD_HANDLE_INUSE);
@@ -413,15 +394,25 @@
ihP = fdP->fd_ih;
closeFd = fdP->fd_fd;
- DLL_DELETE(fdP, fdP->fd_ih->ih_fdhead, fdP->fd_ih->ih_fdtail,
- fd_ihnext, fd_ihprev);
+ DLL_DELETE(fdP, ihP->ih_fdhead, ihP->ih_fdtail, fd_ihnext, fd_ihprev);
DLL_INSERT_TAIL(fdP, fdAvailHead, fdAvailTail, fd_next, fd_prev);
+
fdP->fd_status = FD_HANDLE_AVAIL;
fdP->fd_ih = NULL;
fdP->fd_fd = INVALID_FD;
+
+ /* All the file descriptor handles have been closed; reset
+ * the IH_REALLY_CLOSED flag indicating that ih_reallyclose
+ * has completed its job.
+ */
+ if (!ihP->ih_fdhead) {
+ ihP->ih_flags &= ~IH_REALLY_CLOSED;
+ }
+
IH_UNLOCK
OS_CLOSE(closeFd);
IH_LOCK
+
fdInUseCount -= 1;
/* If this is not the only reference to the Inode then we can
decrement
@@ -433,6 +424,7 @@
IH_UNLOCK
ih_release(ihP);
}
+
return 0;
}
@@ -658,163 +650,136 @@
return retval;
}
-/* Close all cached file descriptors for this inode. */
-int ih_reallyclose(IHandle_t *ihP)
+/* Close all unused file descriptors associated with the inode
+ * handle. Called with IH_LOCK held. May drop and reacquire
+ * IH_LOCK. Sets the IH_REALLY_CLOSED flag in the inode handle
+ * if it fails to close all file handles.
+ */
+static int ih_fdclose(IHandle_t *ihP)
{
- int closeCount;
- FdHandle_t *fdP;
- FdHandle_t *head, *tail;
-
- if (!ihP)
- return 0;
-
- IH_LOCK
+ int closeCount, closedAll;
+ FdHandle_t *fdP, *head, *tail, *next;
assert(ihP->ih_refcnt > 0);
+ closedAll = 1;
+ DLL_INIT_LIST(head, tail);
+ ihP->ih_flags &= ~IH_REALLY_CLOSED;
+
/*
* Remove the file descriptors for this Inode from the LRU queue
- * and put them on a temporary queue so we drop the lock before
- * we close the files.
+ * and the IHandle queue and put them on a temporary queue so we
+ * can drop the lock before we close the files.
*/
- DLL_INIT_LIST(head, tail);
- for (fdP = ihP->ih_fdhead ; fdP != NULL ; fdP = fdP->fd_ihnext) {
- if (fdP->fd_status == FD_HANDLE_OPEN) {
- assert(fdP->fd_ih == ihP);
- DLL_DELETE(fdP, fdLruHead, fdLruTail, fd_next, fd_prev);
- DLL_INSERT_TAIL(fdP, head, tail, fd_next, fd_prev);
- } else {
- ihP->ih_flags |= IH_REALLY_CLOSED;
- }
+ for (fdP = ihP->ih_fdhead; fdP != NULL; fdP = next) {
+ next = fdP->fd_ihnext;
+ assert(fdP->fd_ih == ihP);
+ assert(fdP->fd_status == FD_HANDLE_OPEN ||
+ fdP->fd_status == FD_HANDLE_INUSE);
+ if (fdP->fd_status == FD_HANDLE_OPEN) {
+ DLL_DELETE(fdP, ihP->ih_fdhead, ihP->ih_fdtail,
+ fd_ihnext, fd_ihprev);
+ DLL_DELETE(fdP, fdLruHead, fdLruTail, fd_next, fd_prev);
+ DLL_INSERT_TAIL(fdP, head, tail, fd_next, fd_prev);
+ } else {
+ closedAll = 0;
+ ihP->ih_flags |= IH_REALLY_CLOSED;
+ }
}
-
- /*
- * If we found any file descriptors in use, then we dont zero out
- * fdhead and fdtail, since ih_reallyclose() will be called again on
this
- * Inode handle
- */
- if ( ! (ihP->ih_flags & IH_REALLY_CLOSED) )
- DLL_INIT_LIST(ihP->ih_fdhead, ihP->ih_fdtail);
+ /* If the ihandle reference count is 1, we should have
+ * closed all file descriptors.
+ */
+ if (ihP->ih_refcnt == 1 || closedAll) {
+ assert(closedAll);
+ assert(!ihP->ih_fdhead);
+ assert(!ihP->ih_fdtail);
+ }
if (head == NULL) {
- IH_UNLOCK
- return 0;
+ return 0; /* No file descriptors closed */
}
+ IH_UNLOCK
+
/*
* Close the file descriptors
*/
closeCount = 0;
- for (fdP = head ; fdP != NULL ; fdP = fdP->fd_ihnext) {
- IH_UNLOCK
- OS_CLOSE(fdP->fd_fd);
- IH_LOCK
- assert(fdInUseCount > 0);
- fdInUseCount -= 1;
- fdP->fd_status = FD_HANDLE_AVAIL;
- fdP->fd_fd = INVALID_FD;
- fdP->fd_ih = NULL;
- closeCount++;
+ for (fdP = head; fdP != NULL; fdP = fdP->fd_next) {
+ OS_CLOSE(fdP->fd_fd);
+ fdP->fd_status = FD_HANDLE_AVAIL;
+ fdP->fd_fd = INVALID_FD;
+ fdP->fd_ih = NULL;
+ closeCount++;
}
+ IH_LOCK
+
+ assert(fdInUseCount >= closeCount);
+ fdInUseCount -= closeCount;
+
/*
* Append the temporary queue to the list of available descriptors
*/
if (fdAvailHead == NULL) {
- fdAvailHead = head;
- fdAvailTail = tail;
+ fdAvailHead = head;
+ fdAvailTail = tail;
} else {
- fdAvailTail->fd_next = head;
- head->fd_prev = fdAvailTail;
- fdAvailTail = tail;
+ fdAvailTail->fd_next = head;
+ head->fd_prev = fdAvailTail;
+ fdAvailTail = tail;
}
+
+ return 0;
+}
+
+/* Close all cached file descriptors for this inode. */
+int ih_reallyclose(IHandle_t *ihP)
+{
+ if (!ihP)
+ return 0;
+
+ IH_LOCK
+
+ assert(ihP->ih_refcnt > 0);
+ ih_fdclose(ihP);
+
IH_UNLOCK
return 0;
}
/* Release an Inode handle. All cached file descriptors for this
- * inode are closed when the last reference to this handle is released */
+ * inode are closed when the last reference to this handle is released
+ */
int ih_release(IHandle_t *ihP)
{
- int closeCount;
- FdHandle_t *fdP;
- FdHandle_t *head, *tail;
int ihash;
if (!ihP)
- return 0;
+ return 0;
IH_LOCK
- /**
- * If the IH_REALLY_CLOSED flag is set then clear it here before
adding
- * the Inode handle to the available queue
- */
- if ( ihP->ih_flags & IH_REALLY_CLOSED )
- ihP->ih_flags &= ~IH_REALLY_CLOSED;
+ assert(ihP->ih_refcnt > 0);
- ihP->ih_refcnt--;
- if (ihP->ih_refcnt > 0) {
- IH_UNLOCK
- return 0;
+ if (ihP->ih_refcnt > 1) {
+ ihP->ih_refcnt--;
+ IH_UNLOCK
+ return 0;
}
- assert(ihP->ih_refcnt == 0);
-
ihash = IH_HASH(ihP->ih_dev, ihP->ih_vid, ihP->ih_ino);
DLL_DELETE(ihP, ihashTable[ihash].ihash_head,
- ihashTable[ihash].ihash_tail, ih_next, ih_prev);
-
- /*
- * Remove the file descriptors for this Inode from the LRU queue
- * and put them on a temporary queue so we drop the lock before
- * we close the files.
- */
- DLL_INIT_LIST(head, tail);
- for (fdP = ihP->ih_fdhead ; fdP != NULL ; fdP = fdP->fd_ihnext) {
- assert(fdP->fd_status == FD_HANDLE_OPEN);
- assert(fdP->fd_ih == ihP);
- DLL_DELETE(fdP, fdLruHead, fdLruTail, fd_next, fd_prev);
- DLL_INSERT_TAIL(fdP, head, tail, fd_next, fd_prev);
- }
- DLL_INIT_LIST(ihP->ih_fdhead, ihP->ih_fdtail);
+ ihashTable[ihash].ihash_tail, ih_next, ih_prev);
- if (head == NULL) {
- DLL_INSERT_TAIL(ihP, ihAvailHead, ihAvailTail, ih_next, ih_prev);
- IH_UNLOCK
- return 0;
- }
+ ih_fdclose(ihP);
- /*
- * Close the file descriptors
- */
- closeCount = 0;
- for (fdP = head ; fdP != NULL ; fdP = fdP->fd_ihnext) {
- IH_UNLOCK
- OS_CLOSE(fdP->fd_fd);
- IH_LOCK
- assert(fdInUseCount > 0);
- fdInUseCount -= 1;
- fdP->fd_status = FD_HANDLE_AVAIL;
- fdP->fd_fd = INVALID_FD;
- fdP->fd_ih = NULL;
- closeCount++;
- }
+ ihP->ih_refcnt--;
- /*
- * Append the temporary queue to the list of available descriptors
- */
- if (fdAvailHead == NULL) {
- fdAvailHead = head;
- fdAvailTail = tail;
- } else {
- fdAvailTail->fd_next = head;
- head->fd_prev = fdAvailTail;
- fdAvailTail = tail;
- }
DLL_INSERT_TAIL(ihP, ihAvailHead, ihAvailTail, ih_next, ih_prev);
+
IH_UNLOCK
return 0;
Thanks,
Srikanth.