From: Marc Dionne Date: Wed, 22 Apr 2015 18:06:12 +0000 (-0300) Subject: Linux: mmap: Apply recursion check only to recursion cases X-Git-Tag: upstream/1.6.13^2~10 X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=2c95c104fd526f99db6237fc49534ad7dd6e92c0;p=packages%2Fo%2Fopenafs.git Linux: mmap: Apply recursion check only to recursion cases The CPageWrite flag was originally added to prevent a scenario where a thread doing "writepage" would realize that the cache was too full and that some of its contents need to be written back to the server. Before writing back it would ask the OS to flush any dirty VM associated with the vcache entries that are to be written, to make sure the data is not stale. This flush could itself trigger writeback, leading to deadly recursion. One such scenario is a process doing mmap writes to a file larger than the cache. With some kernel versions and some callers of writepage, this can cause the mapping to be marked as being in an error state, leading to EIO errors passed back to user space. Make the recursion check more specific to only bail when the calling thread is one that was originally seen writing. A list of current writers is maintained instead of a single state flag. This lets other threads (like the flusher thread) go on with writeback to the same file, and limits the WRITEPAGE_ACTIVATE return case to call sites that can deal with it. In testing this helps avoid EIO errors when writing large chunks of data through mmap. Thanks to Yadav Yadavendra for extensive analysis and testing. Reviewed-on: http://gerrit.openafs.org/11124 Reviewed-by: Daria Brashear Tested-by: BuildBot (cherry picked from commit 95b857399d71cb1f6619e625bff256f8c4c72c6a) Change-Id: I08ced97c4f58f95375fda2ed9c707cdf7657e493 Reviewed-on: http://gerrit.openafs.org/11877 Reviewed-by: Benjamin Kaduk Tested-by: BuildBot Reviewed-by: Stephan Wiesand --- diff --git a/src/afs/LINUX/osi_vcache.c b/src/afs/LINUX/osi_vcache.c index 391e7d455..e64265f76 100644 --- a/src/afs/LINUX/osi_vcache.c +++ b/src/afs/LINUX/osi_vcache.c @@ -125,6 +125,9 @@ osi_NewVnode(void) tvc->v = ip; #endif + INIT_LIST_HEAD(&tvc->pagewriters); + spin_lock_init(&tvc->pagewriter_lock); + return tvc; } diff --git a/src/afs/LINUX/osi_vm.c b/src/afs/LINUX/osi_vm.c index 8bc792e45..2ca3ad3ea 100644 --- a/src/afs/LINUX/osi_vm.c +++ b/src/afs/LINUX/osi_vm.c @@ -91,7 +91,7 @@ osi_VM_StoreAllSegments(struct vcache *avc) { struct inode *ip = AFSTOV(avc); - if (avc->f.states & CPageWrite) + if (!list_empty(&avc->pagewriters)) return; /* someone already writing */ /* filemap_fdatasync() only exported in 2.4.5 and above */ diff --git a/src/afs/LINUX/osi_vnodeops.c b/src/afs/LINUX/osi_vnodeops.c index 0aaad2d9c..91c09ed17 100644 --- a/src/afs/LINUX/osi_vnodeops.c +++ b/src/afs/LINUX/osi_vnodeops.c @@ -2571,10 +2571,27 @@ out: * locked */ static inline int afs_linux_prepare_writeback(struct vcache *avc) { - if (avc->f.states & CPageWrite) { - return AOP_WRITEPAGE_ACTIVATE; + pid_t pid; + struct pagewriter *pw; + + pid = MyPidxx2Pid(MyPidxx); + /* Prevent recursion into the writeback code */ + spin_lock(&avc->pagewriter_lock); + list_for_each_entry(pw, &avc->pagewriters, link) { + if (pw->writer == pid) { + spin_unlock(&avc->pagewriter_lock); + return AOP_WRITEPAGE_ACTIVATE; + } } - avc->f.states |= CPageWrite; + spin_unlock(&avc->pagewriter_lock); + + /* Add ourselves to writer list */ + pw = osi_Alloc(sizeof(struct pagewriter)); + pw->writer = pid; + spin_lock(&avc->pagewriter_lock); + list_add_tail(&pw->link, &avc->pagewriters); + spin_unlock(&avc->pagewriter_lock); + return 0; } @@ -2593,7 +2610,26 @@ afs_linux_dopartialwrite(struct vcache *avc, cred_t *credp) { static inline void afs_linux_complete_writeback(struct vcache *avc) { - avc->f.states &= ~CPageWrite; + struct pagewriter *pw, *store; + pid_t pid; + struct list_head tofree; + + INIT_LIST_HEAD(&tofree); + pid = MyPidxx2Pid(MyPidxx); + /* Remove ourselves from writer list */ + spin_lock(&avc->pagewriter_lock); + list_for_each_entry_safe(pw, store, &avc->pagewriters, link) { + if (pw->writer == pid) { + list_del(&pw->link); + /* osi_Free may sleep so we need to defer it */ + list_add_tail(&pw->link, &tofree); + } + } + spin_unlock(&avc->pagewriter_lock); + list_for_each_entry_safe(pw, store, &tofree, link) { + list_del(&pw->link); + osi_Free(pw, sizeof(struct pagewriter)); + } } /* Writeback a given page syncronously. Called with no AFS locks held */ diff --git a/src/afs/afs.h b/src/afs/afs.h index cc855271b..0dbc11bf8 100644 --- a/src/afs/afs.h +++ b/src/afs/afs.h @@ -612,9 +612,7 @@ struct SimpleLocks { #define CBulkStat 0x00020000 /* loaded by a bulk stat, and not ref'd since */ #define CUnlinkedDel 0x00040000 #define CVFlushed 0x00080000 -#ifdef AFS_LINUX22_ENV -#define CPageWrite 0x00200000 /* to detect vm deadlock - linux */ -#elif defined(AFS_SGI_ENV) +#if defined(AFS_SGI_ENV) #define CWritingUFS 0x00200000 /* to detect vm deadlock - used by sgi */ #elif defined(AFS_DARWIN80_ENV) #define CEvent 0x00200000 /* to preclude deadlock when sending events */ @@ -900,8 +898,19 @@ struct vcache { struct afs_q multiPage; /* list of multiPage_range structs */ #endif afs_uint32 lastBRLWarnTime; /* last time we warned about byte-range locks */ +#ifdef AFS_LINUX26_ENV + spinlock_t pagewriter_lock; + struct list_head pagewriters; /* threads that are writing vm pages */ +#endif }; +#ifdef AFS_LINUX26_ENV +struct pagewriter { + struct list_head link; + pid_t writer; +}; +#endif + #define DONT_CHECK_MODE_BITS 0 #define CHECK_MODE_BITS 1 #define CMB_ALLOW_EXEC_AS_READ 2 /* For the NFS xlator */ diff --git a/src/afs/afs_vcache.c b/src/afs/afs_vcache.c index 179c57247..f3b78803b 100644 --- a/src/afs/afs_vcache.c +++ b/src/afs/afs_vcache.c @@ -180,6 +180,26 @@ afs_FlushVCache(struct vcache *avc, int *slept) /* remove entry from the volume hash table */ QRemove(&avc->vhashq); +#if defined(AFS_LINUX26_ENV) + { + struct pagewriter *pw, *store; + struct list_head tofree; + + INIT_LIST_HEAD(&tofree); + spin_lock(&avc->pagewriter_lock); + list_for_each_entry_safe(pw, store, &avc->pagewriters, link) { + list_del(&pw->link); + /* afs_osi_Free may sleep so we need to defer it */ + list_add_tail(&pw->link, &tofree); + } + spin_unlock(&avc->pagewriter_lock); + list_for_each_entry_safe(pw, store, &tofree, link) { + list_del(&pw->link); + afs_osi_Free(pw, sizeof(struct pagewriter)); + } + } +#endif + if (avc->mvid) osi_FreeSmallSpace(avc->mvid); avc->mvid = (struct VenusFid *)0;