Changeset 77458 in vbox for trunk/src/VBox/Additions/linux

Timestamp:

Feb 25, 2019 2:40:13 PM (6 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

129031

Message:

linux/vboxsf: Fixed sf_writepage issue wrt finding a valid writable host handle to use. bugref:9172

Location:

trunk/src/VBox/Additions/linux/sharedfolders

Files:

• dirops.c (modified) (2 diffs)
• files_vboxsf (modified) (1 diff)
• regops.c (modified) (27 diffs)
• vfsmod.c (modified) (5 diffs)
• vfsmod.h (modified) (5 diffs)

trunk/src/VBox/Additions/linux/sharedfolders/dirops.c

@@ -432 +432 @@
                 sf_new_i->handle = SHFL_HANDLE_NIL;
                 sf_new_i->force_reread = 0;
+                RTListInit(&sf_new_i->HandleList);
+#ifdef VBOX_STRICT
+                sf_new_i->u32Magic = SF_INODE_INFO_MAGIC;
+#endif
 
                 ino = iunique(parent->i_sb, 1);
@@ -519 +523 @@
 
         sf_init_inode(sf_g, inode, info);
+
         sf_new_i->path = path;
-        SET_INODE_INFO(inode, sf_new_i);
+        RTListInit(&sf_new_i->HandleList);
         sf_new_i->force_restat = 1;
         sf_new_i->force_reread = 0;
+#ifdef VBOX_STRICT
+        sf_new_i->u32Magic = SF_INODE_INFO_MAGIC;
+#endif
+        SET_INODE_INFO(inode, sf_new_i);
 
         d_instantiate(dentry, inode);

trunk/src/VBox/Additions/linux/sharedfolders/files_vboxsf

@@ -30 +30 @@
     ${PATH_ROOT}/include/iprt/fs.h=>include/iprt/fs.h \
     ${PATH_ROOT}/include/iprt/latin1.h=>include/iprt/latin1.h \
+    ${PATH_ROOT}/include/iprt/list.h=>include/iprt/list.h \
     ${PATH_ROOT}/include/iprt/log.h=>include/iprt/log.h \
     ${PATH_ROOT}/include/iprt/mangling.h=>include/iprt/mangling.h \

trunk/src/VBox/Additions/linux/sharedfolders/regops.c

@@ -73 +73 @@
 #endif /* < 2.6.0 */
 
+
+/**
+ * Called when an inode is released to unlink all handles that might impossibly
+ * still be associated with it.
+ *
+ * @param   pInodeInfo  The inode which handles to drop.
+ */
+void sf_handle_drop_chain(struct sf_inode_info *pInodeInfo)
+{
+        struct sf_handle *pCur, *pNext;
+        unsigned long     fSavedFlags;
+        SFLOGFLOW(("sf_handle_drop_chain: %p\n", pInodeInfo));
+        spin_lock_irqsave(&g_SfHandleLock, fSavedFlags);
+
+        RTListForEachSafe(&pInodeInfo->HandleList, pCur, pNext, struct sf_handle, Entry) {
+                AssertMsg((pCur->fFlags & (SF_HANDLE_F_MAGIC_MASK | SF_HANDLE_F_ON_LIST)) == (SF_HANDLE_F_MAGIC | SF_HANDLE_F_ON_LIST),
+                          ("%p %#x\n", pCur, pCur->fFlags));
+                pCur->fFlags |= SF_HANDLE_F_ON_LIST;
+                RTListNodeRemove(&pCur->Entry);
+        }
+
+        spin_unlock_irqrestore(&g_SfHandleLock, fSavedFlags);
+}
+
+
+/**
+ * Locates a handle that matches all the flags in @a fFlags.
+ *
+ * @returns Pointer to handle on success (retained), use sf_handle_release() to
+ *          release it.  NULL if no suitable handle was found.
+ * @param   pInodeInfo  The inode info to search.
+ * @param   fFlagsSet   The flags that must be set.
+ * @param   fFlagsClear The flags that must be clear.
+ */
+struct sf_handle *sf_handle_find(struct sf_inode_info *pInodeInfo, uint32_t fFlagsSet, uint32_t fFlagsClear)
+{
+        struct sf_handle *pCur;
+        unsigned long     fSavedFlags;
+        spin_lock_irqsave(&g_SfHandleLock, fSavedFlags);
+
+        RTListForEach(&pInodeInfo->HandleList, pCur, struct sf_handle, Entry) {
+                AssertMsg((pCur->fFlags & (SF_HANDLE_F_MAGIC_MASK | SF_HANDLE_F_ON_LIST)) == (SF_HANDLE_F_MAGIC | SF_HANDLE_F_ON_LIST),
+                          ("%p %#x\n", pCur, pCur->fFlags));
+                if ((pCur->fFlags & (fFlagsSet | fFlagsClear)) == fFlagsSet) {
+                        uint32_t cRefs = ASMAtomicIncU32(&pCur->cRefs);
+                        if (cRefs > 1) {
+                                spin_unlock_irqrestore(&g_SfHandleLock, fSavedFlags);
+                                SFLOGFLOW(("sf_handle_find: returns %p\n", pCur));
+                                return pCur;
+                        }
+                        /* Oops, already being closed (safe as it's only ever increased here). */
+                        ASMAtomicDecU32(&pCur->cRefs);
+                }
+        }
+
+        spin_unlock_irqrestore(&g_SfHandleLock, fSavedFlags);
+        SFLOGFLOW(("sf_handle_find: returns NULL!\n"));
+        return NULL;
+}
+
+
+/**
+ * Slow worker for sf_handle_release() that does the freeing.
+ *
+ * @returns 0 (ref count).
+ * @param   pHandle         The handle to release.
+ * @param   sf_g            The info structure for the shared folder associated
+ *                          with the handle.
+ * @param   pszCaller       The caller name (for logging failures).
+ */
+uint32_t sf_handle_release_slow(struct sf_handle *pHandle, struct sf_glob_info *sf_g, const char *pszCaller)
+{
+        int rc;
+        unsigned long fSavedFlags;
+
+        SFLOGFLOW(("sf_handle_release_slow: %p (%s)\n", pHandle, pszCaller));
+
+        /*
+         * Remove from the list.
+         */
+        spin_lock_irqsave(&g_SfHandleLock, fSavedFlags);
+
+        AssertMsg((pHandle->fFlags & SF_HANDLE_F_MAGIC_MASK) == SF_HANDLE_F_MAGIC, ("%p %#x\n", pHandle, pHandle->fFlags));
+        Assert(pHandle->pInodeInfo);
+        Assert(pHandle->pInodeInfo && pHandle->pInodeInfo->u32Magic == SF_INODE_INFO_MAGIC);
+
+        if (pHandle->fFlags & SF_HANDLE_F_ON_LIST) {
+                pHandle->fFlags &= ~SF_HANDLE_F_ON_LIST;
+                RTListNodeRemove(&pHandle->Entry);
+        }
+
+        spin_unlock_irqrestore(&g_SfHandleLock, fSavedFlags);
+
+        /*
+         * Actually destroy it.
+         */
+        rc = VbglR0SfHostReqCloseSimple(sf_g->map.root, pHandle->hHost);
+        if (RT_FAILURE(rc))
+                LogFunc(("Caller %s: VbglR0SfHostReqCloseSimple %#RX64 failed with rc=%Rrc\n", pszCaller, pHandle->hHost, rc));
+        pHandle->hHost  = SHFL_HANDLE_NIL;
+        pHandle->fFlags = SF_HANDLE_F_MAGIC_DEAD;
+        kfree(pHandle);
+        return 0;
+}
+
+
+/**
+ * Appends a handle to a handle list.
+ *
+ * @param   pInodeInfo          The inode to add it to.
+ * @param   pHandle             The handle to add.
+ */
+void sf_handle_append(struct sf_inode_info *pInodeInfo, struct sf_handle *pHandle)
+{
+#ifdef VBOX_STRICT
+        struct sf_handle *pCur;
+#endif
+        unsigned long fSavedFlags;
+
+        SFLOGFLOW(("sf_handle_append: %p (to %p)\n", pHandle, pInodeInfo));
+        AssertMsg((pHandle->fFlags & (SF_HANDLE_F_MAGIC_MASK | SF_HANDLE_F_ON_LIST)) == SF_HANDLE_F_MAGIC,
+                  ("%p %#x\n", pHandle, pHandle->fFlags));
+        Assert(pInodeInfo->u32Magic == SF_INODE_INFO_MAGIC);
+
+        spin_lock_irqsave(&g_SfHandleLock, fSavedFlags);
+
+        AssertMsg((pHandle->fFlags & (SF_HANDLE_F_MAGIC_MASK | SF_HANDLE_F_ON_LIST)) == SF_HANDLE_F_MAGIC,
+                  ("%p %#x\n", pHandle, pHandle->fFlags));
+#ifdef VBOX_STRICT
+        RTListForEach(&pInodeInfo->HandleList, pCur, struct sf_handle, Entry) {
+                Assert(pCur != pHandle);
+                AssertMsg((pCur->fFlags & (SF_HANDLE_F_MAGIC_MASK | SF_HANDLE_F_ON_LIST)) == (SF_HANDLE_F_MAGIC | SF_HANDLE_F_ON_LIST),
+                          ("%p %#x\n", pCur, pCur->fFlags));
+        }
+        pHandle->pInodeInfo = pInodeInfo;
+#endif
+
+        pHandle->fFlags |= SF_HANDLE_F_ON_LIST;
+        RTListAppend(&pInodeInfo->HandleList, &pHandle->Entry);
+
+        spin_unlock_irqrestore(&g_SfHandleLock, fSavedFlags);
+}
 
 
@@ -131 +273 @@
                            uint32_t * nread, uint64_t pos)
 {
-        int rc = VbglR0SfRead(&client_handle, &sf_g->map, sf_r->handle,
+        int rc = VbglR0SfRead(&client_handle, &sf_g->map, sf_r->Handle.hHost,
                               pos, nread, buf, false /* already locked? */ );
         if (RT_FAILURE(rc)) {
@@ -390 +532 @@
                          * Issue the request and unlock the pages.
                          */
-                        rc = VbglR0SfHostReqReadPgLst(sf_g->map.root, pReq, sf_r->handle, offFile, cbChunk, cPages);
+                        rc = VbglR0SfHostReqReadPgLst(sf_g->map.root, pReq, sf_r->Handle.hHost, offFile, cbChunk, cPages);
 
                         sf_unlock_user_pages(papPages, cPages, true /*fSetDirty*/);
@@ -457 +599 @@
         struct address_space *mapping = inode->i_mapping;
 
-        TRACE();
+        SFLOGFLOW(("sf_reg_read: inode=%p file=%p buf=%p size=%#zx off=%#llx\n", inode, file, buf, size, *off));
+
         if (!S_ISREG(inode->i_mode)) {
                 LogFunc(("read from non regular file %d\n", inode->i_mode));
@@ -491 +634 @@
                     && (PAGE_SIZE - ((uintptr_t)pReq & PAGE_OFFSET_MASK)) >= cbReq) {
                         ssize_t cbRet;
-                        int vrc = VbglR0SfHostReqReadEmbedded(sf_g->map.root, pReq, sf_r->handle, *off, (uint32_t)size);
+                        int vrc = VbglR0SfHostReqReadEmbedded(sf_g->map.root, pReq, sf_r->Handle.hHost, *off, (uint32_t)size);
                         if (RT_SUCCESS(vrc)) {
                                 cbRet = pReq->Parms.cb32Read.u.value32;
@@ -518 +661 @@
                         if (pReq) {
                                 ssize_t cbRet;
-                                int vrc = VbglR0SfHostReqReadContig(sf_g->map.root, pReq, sf_r->handle, *off, (uint32_t)size,
-                                                                    pvBounce, virt_to_phys(pvBounce));
+                                int vrc = VbglR0SfHostReqReadContig(sf_g->map.root, pReq, sf_r->Handle.hHost, *off,
+                                                                    (uint32_t)size, pvBounce, virt_to_phys(pvBounce));
                                 if (RT_SUCCESS(vrc)) {
                                         cbRet = pReq->Parms.cb32Read.u.value32;
@@ -629 +772 @@
                          * Issue the request and unlock the pages.
                          */
-                        rc = VbglR0SfHostReqWritePgLst(sf_g->map.root, pReq, sf_r->handle, offFile, cbChunk, cPages);
+                        rc = VbglR0SfHostReqWritePgLst(sf_g->map.root, pReq, sf_r->Handle.hHost, offFile, cbChunk, cPages);
 
                         sf_unlock_user_pages(papPages, cPages, false /*fSetDirty*/);
@@ -702 +845 @@
         loff_t                pos;
 
-        TRACE();
+        SFLOGFLOW(("sf_reg_write: inode=%p file=%p buf=%p size=%#zx off=%#llx\n", inode, file, buf, size, *off));
         BUG_ON(!sf_i);
         BUG_ON(!sf_g);
@@ -753 +896 @@
                         ssize_t cbRet;
                         if (copy_from_user(pReq->abData, buf, size) == 0) {
-                                int vrc = VbglR0SfHostReqWriteEmbedded(sf_g->map.root, pReq, sf_r->handle, pos, (uint32_t)size);
+                                int vrc = VbglR0SfHostReqWriteEmbedded(sf_g->map.root, pReq, sf_r->Handle.hHost,
+                                                                       pos, (uint32_t)size);
                                 if (RT_SUCCESS(vrc)) {
                                         cbRet = pReq->Parms.cb32Write.u.value32;
@@ -832 +976 @@
         SHFLCREATEPARMS *pCreateParms;  /* temp glue */
 
-        TRACE();
+        SFLOGFLOW(("sf_reg_open: inode=%p file=%p flags=%#x %s\n",
+                   inode, file, file->f_flags, sf_i ? sf_i->path->String.ach : NULL));
         BUG_ON(!sf_g);
         BUG_ON(!sf_i);
-
-        LogFunc(("open %s\n", sf_i->path->String.utf8));
 
         sf_r = kmalloc(sizeof(*sf_r), GFP_KERNEL);
@@ -843 +986 @@
                 return -ENOMEM;
         }
+
+        RTListInit(&sf_r->Handle.Entry);
+        sf_r->Handle.cRefs  = 1;
+        sf_r->Handle.fFlags = SF_HANDLE_F_FILE | SF_HANDLE_F_MAGIC;
+        sf_r->Handle.hHost  = SHFL_HANDLE_NIL;
 
         /* Already open? */
@@ -852 +1000 @@
                  */
                 sf_i->force_restat = 1;
-                sf_r->handle = sf_i->handle;
+                sf_r->Handle.hHost = sf_i->handle;
                 sf_i->handle = SHFL_HANDLE_NIL;
-                sf_i->file = file;
                 file->private_data = sf_r;
+
+                sf_r->Handle.fFlags |= SF_HANDLE_F_READ | SF_HANDLE_F_WRITE; /** @todo check */
+                sf_handle_append(sf_i, &sf_r->Handle);
+                SFLOGFLOW(("sf_reg_open: returns 0 (#1) - sf_i=%p hHost=%#llx\n", sf_i, sf_r->Handle.hHost));
                 return 0;
         }
@@ -898 +1049 @@
         case O_RDONLY:
                 pCreateParms->CreateFlags |= SHFL_CF_ACCESS_READ;
+                sf_r->Handle.fFlags |= SF_HANDLE_F_READ;
                 break;
 
         case O_WRONLY:
                 pCreateParms->CreateFlags |= SHFL_CF_ACCESS_WRITE;
+                sf_r->Handle.fFlags |= SF_HANDLE_F_WRITE;
                 break;
 
         case O_RDWR:
                 pCreateParms->CreateFlags |= SHFL_CF_ACCESS_READWRITE;
+                sf_r->Handle.fFlags |= SF_HANDLE_F_READ | SF_HANDLE_F_WRITE;
                 break;
 
@@ -915 +1069 @@
                 LogFunc(("O_APPEND set\n"));
                 pCreateParms->CreateFlags |= SHFL_CF_ACCESS_APPEND;
+                sf_r->Handle.fFlags |= SF_HANDLE_F_APPEND;
         }
 
@@ -942 +1097 @@
 
         sf_i->force_restat = 1;
-        sf_r->handle = pCreateParms->Handle;
-        sf_i->file = file;
+        sf_r->Handle.hHost = pCreateParms->Handle;
         file->private_data = sf_r;
+        sf_handle_append(sf_i, &sf_r->Handle);
         VbglR0PhysHeapFree(pReq);
+        SFLOGFLOW(("sf_reg_open: returns 0 (#2) - sf_i=%p hHost=%#llx\n", sf_i, sf_r->Handle.hHost));
         return rc_linux;
 }
+
 
 /**
@@ -958 +1115 @@
 static int sf_reg_release(struct inode *inode, struct file *file)
 {
-        int rc;
         struct sf_reg_info *sf_r;
         struct sf_glob_info *sf_g;
         struct sf_inode_info *sf_i = GET_INODE_INFO(inode);
 
-        TRACE();
+        SFLOGFLOW(("sf_reg_release: inode=%p file=%p\n", inode, file));
         sf_g = GET_GLOB_INFO(inode->i_sb);
         sf_r = file->private_data;
@@ -980 +1136 @@
                 filemap_fdatawait(inode->i_mapping);
 #endif
-        rc = VbglR0SfHostReqCloseSimple(sf_g->map.root, sf_r->handle);
-        if (RT_FAILURE(rc))
-                LogFunc(("VbglR0SfHostReqCloseSimple failed rc=%Rrc\n", rc));
-        sf_r->handle = SHFL_HANDLE_NIL;
-
-        kfree(sf_r);
-        sf_i->file = NULL;
+
+        /* Release sf_r, closing the handle if we're the last user. */
+        file->private_data = NULL;
+        sf_handle_release(&sf_r->Handle, sf_g, "sf_reg_release");
+
         sf_i->handle = SHFL_HANDLE_NIL;
-        file->private_data = NULL;
         return 0;
 }
@@ -1003 +1156 @@
 static loff_t sf_reg_llseek(struct file *file, loff_t off, int whence)
 {
+        SFLOGFLOW(("sf_reg_llseek: file=%p off=%lld whence=%d\n", file, off, whence));
+
         switch (whence) {
 #ifdef SEEK_HOLE
@@ -1010 +1165 @@
                 case SEEK_END: {
                         struct sf_reg_info *sf_r = file->private_data;
-                        int rc = sf_inode_revalidate_with_handle(GET_F_DENTRY(file), sf_r->handle, true /*fForce*/);
+                        int rc = sf_inode_revalidate_with_handle(GET_F_DENTRY(file), sf_r->Handle.hHost, true /*fForce*/);
                         if (rc == 0)
                                 break;
@@ -1128 +1283 @@
         int           err;
 
-        TRACE();
+        SFLOGFLOW(("sf_readpage: inode=%p file=%p page=%p off=%#llx\n", inode, file, page, (uint64_t)page->index << PAGE_SHIFT));
+
         if (!is_bad_inode(inode)) {
             VBOXSFREADPGLSTREQ *pReq = (VBOXSFREADPGLSTREQ *)VbglR0PhysHeapAlloc(sizeof(*pReq));
@@ -1141 +1297 @@
                     vrc = VbglR0SfHostReqReadPgLst(sf_g->map.root,
                                                    pReq,
-                                                   sf_r->handle,
+                                                   sf_r->Handle.hHost,
                                                    (uint64_t)page->index << PAGE_SHIFT,
                                                    PAGE_SIZE,
@@ -1173 +1329 @@
 }
 
+
 /**
  * Used to write out the content of a dirty page cache page to the host file.
@@ -1181 +1338 @@
 static int sf_writepage(struct page *page, struct writeback_control *wbc)
 {
-        int err;
-
-        TRACE();
-
-        VBOXSFWRITEPGLSTREQ *pReq = (VBOXSFWRITEPGLSTREQ *)VbglR0PhysHeapAlloc(sizeof(*pReq));
-        if (pReq) {
-                struct address_space *mapping   = page->mapping;
-                struct inode         *inode     = mapping->host;
-                struct sf_glob_info  *sf_g      = GET_GLOB_INFO(inode->i_sb);
-                struct sf_inode_info *sf_i      = GET_INODE_INFO(inode);
-                struct file          *file      = sf_i->file; /** @todo r=bird: This isn't quite sane wrt readonly vs writeable. */
-                struct sf_reg_info   *sf_r      = file->private_data;
-                uint64_t const        cbFile    = i_size_read(inode);
-                uint64_t const        offInFile = (uint64_t)page->index << PAGE_SHIFT;
-                uint32_t const        cbToWrite = page->index != (cbFile >> PAGE_SHIFT) ? PAGE_SIZE
-                                                : (uint32_t)cbFile & (uint32_t)PAGE_OFFSET_MASK;
-                int                   vrc;
-
-                pReq->PgLst.offFirstPage = 0;
-                pReq->PgLst.aPages[0]    = page_to_phys(page);
-                vrc = VbglR0SfHostReqWritePgLst(sf_g->map.root,
-                                                pReq,
-                                                sf_r->handle,
-                                                offInFile,
-                                                cbToWrite,
-                                                1 /*cPages*/);
-                AssertMsgStmt(pReq->Parms.cb32Write.u.value32 == cbToWrite || RT_FAILURE(vrc), /* lazy bird */
-                              ("%#x vs %#x\n", pReq->Parms.cb32Write, cbToWrite),
-                              vrc = VERR_WRITE_ERROR);
-                VbglR0PhysHeapFree(pReq);
-
-                if (RT_SUCCESS(vrc)) {
-                        /* Update the inode if we've extended the file. */
-                        /** @todo is this necessary given the cbToWrite calc above? */
-                        uint64_t const offEndOfWrite = offInFile + cbToWrite;
-                        if (   offEndOfWrite > cbFile
-                            && offEndOfWrite > i_size_read(inode))
-                                i_size_write(inode, offEndOfWrite);
-
-                        if (PageError(page))
-                                ClearPageError(page);
-
-                        err = 0;
-                } else {
-                        ClearPageUptodate(page);
-                        err = -EPROTO;
-                }
-        } else
-                err = -ENOMEM;
+        struct address_space *mapping   = page->mapping;
+        struct inode         *inode     = mapping->host;
+        struct sf_inode_info *sf_i      = GET_INODE_INFO(inode);
+        struct sf_handle     *pHandle   = sf_handle_find(sf_i, SF_HANDLE_F_WRITE, SF_HANDLE_F_APPEND);
+        int                   err;
+
+        SFLOGFLOW(("sf_writepage: inode=%p page=%p off=%#llx pHandle=%p (%#llx)\n",
+                   inode, page,(uint64_t)page->index << PAGE_SHIFT, pHandle, pHandle->hHost));
+
+        if (pHandle) {
+                struct sf_glob_info  *sf_g = GET_GLOB_INFO(inode->i_sb);
+                VBOXSFWRITEPGLSTREQ  *pReq = (VBOXSFWRITEPGLSTREQ *)VbglR0PhysHeapAlloc(sizeof(*pReq));
+                if (pReq) {
+                        uint64_t const cbFile    = i_size_read(inode);
+                        uint64_t const offInFile = (uint64_t)page->index << PAGE_SHIFT;
+                        uint32_t const cbToWrite = page->index != (cbFile >> PAGE_SHIFT) ? PAGE_SIZE
+                                                 : (uint32_t)cbFile & (uint32_t)PAGE_OFFSET_MASK;
+                        int            vrc;
+
+                        pReq->PgLst.offFirstPage = 0;
+                        pReq->PgLst.aPages[0]    = page_to_phys(page);
+                        vrc = VbglR0SfHostReqWritePgLst(sf_g->map.root,
+                                                        pReq,
+                                                        pHandle->hHost,
+                                                        offInFile,
+                                                        cbToWrite,
+                                                        1 /*cPages*/);
+                        AssertMsgStmt(pReq->Parms.cb32Write.u.value32 == cbToWrite || RT_FAILURE(vrc), /* lazy bird */
+                                      ("%#x vs %#x\n", pReq->Parms.cb32Write, cbToWrite),
+                                      vrc = VERR_WRITE_ERROR);
+                        VbglR0PhysHeapFree(pReq);
+
+                        if (RT_SUCCESS(vrc)) {
+                                /* Update the inode if we've extended the file. */
+                                /** @todo is this necessary given the cbToWrite calc above? */
+                                uint64_t const offEndOfWrite = offInFile + cbToWrite;
+                                if (   offEndOfWrite > cbFile
+                                    && offEndOfWrite > i_size_read(inode))
+                                        i_size_write(inode, offEndOfWrite);
+
+                                if (PageError(page))
+                                        ClearPageError(page);
+
+                                err = 0;
+                        } else {
+                                ClearPageUptodate(page);
+                                err = -EPROTO;
+                        }
+                } else
+                        err = -ENOMEM;
+                sf_handle_release(pHandle, sf_g, "sf_writepage");
+        } else {
+                static uint64_t volatile s_cCalls = 0;
+                if (s_cCalls++ < 16)
+                        printk("sf_writepage: no writable handle for %s..\n", sf_i->path->String.ach);
+                err = -EPROTO;
+        }
         unlock_page(page);
         return err;
@@ -1235 +1400 @@
 
 # if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 24)
-
-#  if 0 /* unused - see below */
-static int sf_reg_write_aux(const char *caller, struct sf_glob_info *sf_g,
-                            struct sf_reg_info *sf_r, void *buf,
-                            uint32_t * nwritten, uint64_t pos)
-{
-    /** @todo bird: yes, kmap() and kmalloc() input only. Since the buffer is
-     *        contiguous in physical memory (kmalloc or single page), we should
-     *        use a physical address here to speed things up. */
-        int rc = VbglR0SfWrite(&client_handle, &sf_g->map, sf_r->handle,
-                               pos, nwritten, buf,
-                               false /* already locked? */ );
-        if (RT_FAILURE(rc)) {
-                LogFunc(("VbglR0SfWrite failed. caller=%s, rc=%Rrc\n",
-                         caller, rc));
-                return -EPROTO;
-        }
-        return 0;
-}
-#  endif
-
 /**
  * Called when writing thru the page cache (which we shouldn't be doing).
@@ -1279 +1423 @@
         return simple_write_begin(file, mapping, pos, len, flags, pagep, fsdata);
 }
-
-/**
- * Called to complete a write thru the page cache (which we shouldn't be doing).
- */
-int sf_write_end(struct file *file, struct address_space *mapping, loff_t pos,
-                 unsigned len, unsigned copied, struct page *page, void *fsdata)
-{
-#  if 0 /** @todo r=bird: See sf_write_begin. */
-        struct inode *inode = mapping->host;
-        struct sf_glob_info *sf_g = GET_GLOB_INFO(inode->i_sb);
-        struct sf_reg_info *sf_r = file->private_data;
-        void *buf;
-        unsigned from = pos & (PAGE_SIZE - 1);
-        uint32_t nwritten = len;
-        int err;
-
-        TRACE();
-
-        buf = kmap(page);
-        err = sf_reg_write_aux(__func__, sf_g, sf_r, buf + from, &nwritten, pos);
-        kunmap(page);
-
-        if (err >= 0) {
-                if (!PageUptodate(page) && nwritten == PAGE_SIZE)
-                        SetPageUptodate(page);
-
-                pos += nwritten;
-                if (pos > inode->i_size)
-                        inode->i_size = pos;
-        }
-
-        unlock_page(page);
-#   if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 6, 0)
-        put_page(page);
-#   else
-        page_cache_release(page);
-#   endif
-        return nwritten;
-#  else
-        return simple_write_end(file, mapping, pos, len, copied, page, fsdata);
-#  endif
-}
-
 # endif /* KERNEL_VERSION >= 2.6.24 */
 
@@ -1357 +1458 @@
         .readpage = sf_readpage,
         .writepage = sf_writepage,
+        /** @todo Need .writepages if we want msync performance...  */
 # if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 5, 12)
         .set_page_dirty = __set_page_dirty_buffers,
@@ -1362 +1464 @@
 # if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 24)
         .write_begin = sf_write_begin,
-        .write_end = sf_write_end,
+        .write_end = simple_write_end,
 # else
         .prepare_write = simple_prepare_write,

trunk/src/VBox/Additions/linux/sharedfolders/vfsmod.c

@@ -59 +59 @@
 VBGLSFCLIENT client_handle;
 VBGLSFCLIENT g_SfClient;      /* temporary? */
+
 uint32_t g_fHostFeatures = 0; /* temporary? */
+
+spinlock_t g_SfHandleLock;
+
 
 /* forward declarations */
@@ -292 +296 @@
         sf_i->path->String.utf8[1] = 0;
         sf_i->force_reread = 0;
+        RTListInit(&sf_i->HandleList);
+#ifdef VBOX_STRICT
+        sf_i->u32Magic = SF_INODE_INFO_MAGIC;
+#endif
 
         err = sf_stat(__func__, sf_g, sf_i->path, &fsinfo, 0);
@@ -409 +417 @@
                 return;
 
+        Assert(sf_i->u32Magic == SF_INODE_INFO_MAGIC);
         BUG_ON(!sf_i->path);
         kfree(sf_i->path);
+        sf_handle_drop_chain(sf_i);
+# ifdef VBOX_STRICT
+        sf_i->u32Magic = SF_INODE_INFO_MAGIC_DEAD;
+# endif
         kfree(sf_i);
         SET_INODE_INFO(inode, NULL);
@@ -431 +444 @@
                 return;
 
+        Assert(sf_i->u32Magic == SF_INODE_INFO_MAGIC);
         BUG_ON(!sf_i->path);
         kfree(sf_i->path);
+        sf_handle_drop_chain(sf_i);
+# ifdef VBOX_STRICT
+        sf_i->u32Magic = SF_INODE_INFO_MAGIC_DEAD;
+# endif
         kfree(sf_i);
         SET_INODE_INFO(inode, NULL);
@@ -627 +645 @@
         }
 
+        /** @todo Init order is wrong, file system reigstration is the very last
+         *        thing we should do. */
+        spin_lock_init(&g_SfHandleLock);
         err = register_filesystem(&vboxsf_fs_type);
         if (err) {

trunk/src/VBox/Additions/linux/sharedfolders/vfsmod.h

@@ -35 +35 @@
 #endif
 
+#if 0 /* Enables strict checks. */
+# define RT_STRICT
+# define VBOX_STRICT
+#endif
+
 #define LOG_GROUP LOG_GROUP_SHARED_FOLDERS
 #include "the-linux-kernel.h"
+#include <iprt/list.h>
+#include <iprt/asm.h>
 #include <VBox/log.h>
 
@@ -69 +76 @@
 };
 
-/* per-inode information */
+/**
+ * For associating inodes with host handles.
+ *
+ * This is necessary for address_space_operations::sf_writepage and allows
+ * optimizing stat, lookups and other operations on open files and directories.
+ */
+struct sf_handle {
+        /** List entry (head sf_inode_info::HandleList). */
+        RTLISTNODE              Entry;
+        /** Host file/whatever handle. */
+        SHFLHANDLE              hHost;
+        /** SF_HANDLE_F_XXX */
+        uint32_t                fFlags;
+        /** Reference counter.
+         * Close the handle and free the structure when it reaches zero. */
+        uint32_t volatile       cRefs;
+#ifdef VBOX_STRICT
+        /** For strictness checks. */
+        struct sf_inode_info   *pInodeInfo;
+#endif
+};
+
+/** @name SF_HANDLE_F_XXX - Handle summary flags (sf_handle::fFlags).
+ * @{  */
+#define SF_HANDLE_F_READ        UINT32_C(0x00000001)
+#define SF_HANDLE_F_WRITE       UINT32_C(0x00000002)
+#define SF_HANDLE_F_APPEND      UINT32_C(0x00000004)
+#define SF_HANDLE_F_FILE        UINT32_C(0x00000010)
+#define SF_HANDLE_F_ON_LIST     UINT32_C(0x00000080)
+#define SF_HANDLE_F_MAGIC_MASK  UINT32_C(0xffffff00)
+#define SF_HANDLE_F_MAGIC       UINT32_C(0x75030700) /**< Maurice Ravel (1875-03-07). */
+#define SF_HANDLE_F_MAGIC_DEAD  UINT32_C(0x19371228)
+/** @} */
+
+/**
+ * VBox specific per-inode information.
+ */
 struct sf_inode_info {
-        /* which file */
+        /** Which file */
         SHFLSTRING *path;
-        /* some information was changed, update data on next revalidate */
+        /** Some information was changed, update data on next revalidate */
         int force_restat;
-        /* directory content changed, update the whole directory on next sf_getdent */
+        /** directory content changed, update the whole directory on next sf_getdent */
         int force_reread;
-        /* file structure, only valid between open() and release() */
-        struct file *file;
-        /* handle valid if a file was created with sf_create_aux until it will
-         * be opened with sf_reg_open() */
+
+        /** handle valid if a file was created with sf_create_aux until it will
+         * be opened with sf_reg_open()
+         * @todo r=bird: figure this one out...  */
         SHFLHANDLE handle;
+
+        /** List of open handles (struct sf_handle), protected by g_SfHandleLock. */
+        RTLISTANCHOR            HandleList;
+#ifdef VBOX_STRICT
+        uint32_t                u32Magic;
+# define SF_INODE_INFO_MAGIC            UINT32_C(0x18620822) /**< Claude Debussy */
+# define SF_INODE_INFO_MAGIC_DEAD       UINT32_C(0x19180325)
+#endif
 };
 
 struct sf_dir_info {
+        /** @todo sf_handle. */
         struct list_head info_list;
 };
@@ -96 +148 @@
 };
 
+/**
+ * VBox specific infor fore a regular file.
+ */
 struct sf_reg_info {
-        SHFLHANDLE handle;
+        /** Handle tracking structure. */
+        struct sf_handle        Handle;
 };
 
 /* globals */
 extern VBGLSFCLIENT client_handle;
+extern spinlock_t g_SfHandleLock;
+
 
 /* forward declarations */
@@ -111 +169 @@
 extern struct dentry_operations sf_dentry_ops;
 extern struct address_space_operations sf_reg_aops;
+
+extern void              sf_handle_drop_chain(struct sf_inode_info *pInodeInfo);
+extern struct sf_handle *sf_handle_find(struct sf_inode_info *pInodeInfo, uint32_t fFlagsSet, uint32_t fFlagsClear);
+extern uint32_t          sf_handle_release_slow(struct sf_handle *pHandle, struct sf_glob_info *sf_g, const char *pszCaller);
+extern void              sf_handle_append(struct sf_inode_info *pInodeInfo, struct sf_handle *pHandle);
+
+/**
+ * Releases a handle.
+ *
+ * @returns New reference count.
+ * @param   pHandle         The handle to release.
+ * @param   sf_g            The info structure for the shared folder associated
+ *                          with the handle.
+ * @param   pszCaller       The caller name (for logging failures).
+ */
+DECLINLINE(uint32_t) sf_handle_release(struct sf_handle *pHandle, struct sf_glob_info *sf_g, const char *pszCaller)
+{
+        uint32_t cRefs;
+
+        Assert((pHandle->fFlags & SF_HANDLE_F_MAGIC_MASK) == SF_HANDLE_F_MAGIC);
+        Assert(pHandle->pInodeInfo);
+        Assert(pHandle->pInodeInfo && pHandle->pInodeInfo->u32Magic == SF_INODE_INFO_MAGIC);
+
+        cRefs = ASMAtomicDecU32(&pHandle->cRefs);
+        Assert(cRefs < _64M);
+        if (cRefs)
+                return cRefs;
+        return sf_handle_release_slow(pHandle, sf_g, pszCaller);
+}
 
 extern void sf_init_inode(struct sf_glob_info *sf_g, struct inode *inode,
@@ -156 +243 @@
 #endif
 
-#define TRACE() LogFunc(("tracepoint\n"))
+#if 1
+# define TRACE()          LogFunc(("tracepoint\n"))
+# define SFLOGFLOW(aArgs) Log(aArgs)
+#else
+# define TRACE()          RTLogBackdoorPrintf("%s: tracepoint\n", __FUNCTION__)
+# define SFLOGFLOW(aArgs) RTLogBackdoorPrintf aArgs
+#endif
 
 /* Following casts are here to prevent assignment of void * to