From 1c4dc26584533afe3c6c7cfd24a0418cb1552a00 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Wed, 19 Dec 2012 17:11:40 -0600 Subject: [PATCH] ihandle: Remove ih_sync_thread ih_sync_thread currently syncs files flagged as needing synchronization in the background every 10 seconds. This practice has caused severe data corruption on more than one occasion over the past few years (124359, 131530). It has also been argued repeatedly that it provides no meaningful additional on-disk consistency, so there is no reason for it to exist even if it were error-free. Syncing files in the background provides no guarantee on the consistency of the file contents, since the files are not synced in any order with respect to each other, or with respect to what filesystem operations may be occurring in the application. Additionally, journalling filesystems common on fileserver backends will typically ensure some consistency after a certain amount of time (by default, 5 seconds on ZFS and ext3+), so doing this sync ourselves is often redundant or even counterproductive. So, to avoid current and future issues with ih_sync_thread interacting with other ihandle users, just get rid of it. Files flagged as needing sync are still synced (not in the background) during IH_REALLYCLOSE. FIXES 131530 Change-Id: I29571c82c5b7454cd834b339fd48baeb9963a87b Reviewed-on: http://gerrit.openafs.org/8797 Tested-by: BuildBot Reviewed-by: Derrick Brashear --- src/vol/ihandle.c | 79 ----------------------------------------------- 1 file changed, 79 deletions(-) diff --git a/src/vol/ihandle.c b/src/vol/ihandle.c index 8b053b37a..8b5d751f0 100644 --- a/src/vol/ihandle.c +++ b/src/vol/ihandle.c @@ -76,7 +76,6 @@ int fdInUseCount = 0; IHashBucket_t ihashTable[I_HANDLE_HASH_SIZE]; static int _ih_release_r(IHandle_t * ihP); -void *ih_sync_thread(void *); /* start-time configurable I/O limits */ ih_init_params vol_io_params; @@ -156,23 +155,6 @@ ih_Initialize(void) } #endif fdCacheSize = min(fdMaxCacheSize, vol_io_params.fd_initial_cachesize); - - { -#ifdef AFS_PTHREAD_ENV - pthread_t syncer; - pthread_attr_t tattr; - - pthread_attr_init(&tattr); - pthread_attr_setdetachstate(&tattr, PTHREAD_CREATE_DETACHED); - - pthread_create(&syncer, &tattr, ih_sync_thread, NULL); -#else /* AFS_PTHREAD_ENV */ - PROCESS syncer; - LWP_CreateProcess(ih_sync_thread, 16*1024, LWP_MAX_PRIORITY - 2, - NULL, "ih_syncer", &syncer); -#endif /* AFS_PTHREAD_ENV */ - } - } /* Make the file descriptor cache as big as possible. Don't this call @@ -975,67 +957,6 @@ ih_condsync(IHandle_t * ihP) return code; } -void -ih_sync_all(void) { - - int ihash; - - IH_LOCK; - for (ihash = 0; ihash < I_HANDLE_HASH_SIZE; ihash++) { - IHandle_t *ihP, *ihPnext; - - ihP = ihashTable[ihash].ihash_head; - if (ihP) - ihP->ih_refcnt++; /* must not disappear over unlock */ - for (; ihP; ihP = ihPnext) { - - if (ihP->ih_synced) { - FdHandle_t *fdP; - - ihP->ih_synced = 0; - IH_UNLOCK; - - fdP = IH_OPEN(ihP); - if (fdP) { - OS_SYNC(fdP->fd_fd); - FDH_CLOSE(fdP); - } - - IH_LOCK; - } - - /* when decrementing the refcount, the ihandle might disappear - and we might not even be able to proceed to the next one. - Hence the gymnastics putting a hold on the next one already */ - ihPnext = ihP->ih_next; - if (ihPnext) ihPnext->ih_refcnt++; - - if (ihP->ih_refcnt > 1) - ihP->ih_refcnt--; - else - _ih_release_r(ihP); - } - } - IH_UNLOCK; -} - -void * -ih_sync_thread(void *dummy) { - afs_pthread_setname_self("ih_syncer"); - while(1) { - -#ifdef AFS_PTHREAD_ENV - sleep(10); -#else /* AFS_PTHREAD_ENV */ - IOMGR_Sleep(60); -#endif /* AFS_PTHREAD_ENV */ - - ih_sync_all(); - } - return NULL; -} - - /************************************************************************* * OS specific support routines. *************************************************************************/ -- 2.39.5