kernel - Fix pager bug in vm_fault and UFS and have UFS use vop_stdgetpages
authorMatthew Dillon <dillon@apollo.backplane.com>
Mon, 18 Jan 2010 17:57:51 +0000 (09:57 -0800)
committerMatthew Dillon <dillon@apollo.backplane.com>
Mon, 18 Jan 2010 17:57:51 +0000 (09:57 -0800)
* UFS must call vtruncbuf() before b*write()ing the buffer related to
  a shortened file's truncation point.  vtruncbuf() can invalidate pages
  which are part of the buffer cache buffer.  The b*write() revalidates
  those pages.  If we fail to revalidate the pages we can end up with a
  buffer containing invalid pages with B_CACHE set.

* UFS was using its own getpages code in certain cases.  Adjust it to
  use vop_stdgetpages() in all cases by default.

  vop_stdgetpages() uses UIO_NOCOPY VOP_READ()s to fill in missing data
  and to perform read-ahead operations.  It should properly deal with
  partially valid and partially dirty pages as well as deal with file
  holes.

* vm_fault can pass a valid page to the readrest: code if PG_RAM is set.
  We cannot free this page if the underlying vnode contains a hole at
  that location.  Call vm_pager_get_page() unconditionally.

  This fixes fsx failures for both NFS and UFS (when UFS is using
  vop_stdgetpages()).

sys/vfs/hammer/hammer_vnops.c
sys/vfs/ufs/ffs_alloc.c
sys/vfs/ufs/ffs_balloc.c
sys/vfs/ufs/ffs_inode.c
sys/vfs/ufs/ffs_subr.c
sys/vfs/ufs/ufs_readwrite.c
sys/vm/vm_fault.c
sys/vm/vm_object.c

index 9b27196..c7582eb 100644 (file)
@@ -2183,6 +2183,17 @@ hammer_vop_setattr(struct vop_setattr_args *ap)
                         * If truncating we have to clean out a portion of
                         * the last block on-disk.  We do this in the
                         * front-end buffer cache.
+                        *
+                        * NOTE: Calling bdwrite() (or bwrite/bawrite) on
+                        *       the buffer will clean its pages.  This
+                        *       is necessary to set the valid bits on
+                        *       pages which vtruncbuf() may have cleared
+                        *       via vnode_pager_setsize().
+                        *
+                        *       If we don't do this the bp can be left
+                        *       with invalid pages and B_CACHE set,
+                        *       creating a situation where getpages can
+                        *       fail.
                         */
                        aligned_size = (vap->va_size + (blksize - 1)) &
                                       ~(int64_t)(blksize - 1);
index 3e83219..7743bc7 100644 (file)
@@ -419,6 +419,7 @@ ffs_reallocblks(struct vop_reallocblks_args *ap)
         * Find the preferred location for the cluster.
         */
        pref = ffs_blkpref(ip, start_lbn, soff, sbap);
+
        /*
         * If the block range spans two block maps, get the second map.
         */
index 694ab36..9d73a54 100644 (file)
 #include "ffs_extern.h"
 
 /*
- * Balloc defines the structure of filesystem storage
- * by allocating the physical blocks on a device given
- * the inode and the logical block number in a file.
- *
  * ffs_balloc(struct vnode *a_vp, ufs_daddr_t a_lbn, int a_size,
  *           struct ucred *a_cred, int a_flags, struct buf *a_bpp)
+ *
+ * Balloc defines the structure of filesystem storage by allocating
+ * the physical blocks on a device given the inode and the logical
+ * block number in a file.
+ *
+ * NOTE: B_CLRBUF - this flag tells balloc to clear invalid portions
+ *      of the buffer.  However, any dirty bits will override missing
+ *      valid bits.  This case occurs when writable mmaps are truncated
+ *      and then extended.
  */
 int
 ffs_balloc(struct vop_balloc_args *ap)
@@ -172,6 +177,9 @@ ffs_balloc(struct vop_balloc_args *ap)
                                }
                                bp->b_bio2.bio_offset = fsbtodoff(fs, nb);
                        } else {
+                               /*
+                                * NOTE: ffs_realloccg() issues a bread().
+                                */
                                error = ffs_realloccg(ip, lbn,
                                    ffs_blkpref(ip, lbn, (int)lbn,
                                        &ip->i_db[0]), osize, nsize, cred, &bp);
index d8057d3..b845cbf 100644 (file)
@@ -217,8 +217,8 @@ ffs_truncate(struct vnode *vp, off_t length, int flags, struct ucred *cred)
                aflags = B_CLRBUF;
                if (flags & IO_SYNC)
                        aflags |= B_SYNC;
-               error = VOP_BALLOC(ovp, length - 1, 1,
-                   cred, aflags, &bp);
+               /* BALLOC reallocates fragment at old EOF */
+               error = VOP_BALLOC(ovp, length - 1, 1, cred, aflags, &bp);
                if (error)
                        return (error);
                oip->i_size = length;
@@ -238,7 +238,16 @@ ffs_truncate(struct vnode *vp, off_t length, int flags, struct ucred *cred)
         * zero'ed in case it ever becomes accessible again because
         * of subsequent file growth. Directories however are not
         * zero'ed as they should grow back initialized to empty.
+        *
+        * The vtruncbuf() must be issued prior the b*write() of
+        * the buffer straddling the truncation point.  The b*write()
+        * calls vfs_clean_bio() which revalidates VM pages which
+        * may have been invalidated by the vtruncbuf().  Otherwise
+        * we may wind up with B_CACHE set on a buf containing invalid
+        * pages which will really mess up getpages.
         */
+       allerror = vtruncbuf(ovp, length, fs->fs_bsize);
+
        offset = blkoff(fs, length);
        if (offset == 0) {
                oip->i_size = length;
@@ -305,7 +314,9 @@ ffs_truncate(struct vnode *vp, off_t length, int flags, struct ucred *cred)
        for (i = NDADDR - 1; i > lastblock; i--)
                oip->i_db[i] = 0;
        oip->i_flag |= IN_CHANGE | IN_UPDATE;
-       allerror = ffs_update(ovp, 1);
+       error = ffs_update(ovp, 1);
+       if (error && allerror == 0)
+               allerror = error;
        
        /*
         * Having written the new inode to disk, save its new configuration
@@ -317,8 +328,7 @@ ffs_truncate(struct vnode *vp, off_t length, int flags, struct ucred *cred)
        bcopy((caddr_t)oldblks, (caddr_t)&oip->i_db[0], sizeof oldblks);
        oip->i_size = osize;
 
-       error = vtruncbuf(ovp, length, fs->fs_bsize);
-       if (error && (allerror == 0))
+       if (error && allerror == 0)
                allerror = error;
 
        /*
index 19f488a..aac23e7 100644 (file)
@@ -121,7 +121,8 @@ ffs_blkatoff_ra(struct vnode *vp, off_t uoffset, char **res, struct buf **bpp,
 
        if (next_loffset >= ip->i_size) {
                /*
-                * Do not do readahead if this is the last block
+                * Do not do readahead if this is the last block,
+                * bsize might represent a fragment.
                 */
                error = bread(vp, base_loffset, bsize, &bp);
        } else if ((vp->v_mount->mnt_flag & MNT_NOCLUSTERR) == 0) {
index d5d55f6..4467d2c 100644 (file)
@@ -59,7 +59,7 @@ extern int ffs_rawread(struct vnode *vp, struct uio *uio, int *workdone);
 #endif
 
 SYSCTL_DECL(_vfs_ffs);
-static int getpages_uses_bufcache = 0;
+static int getpages_uses_bufcache = 1;
 SYSCTL_INT(_vfs_ffs, OID_AUTO, getpages_uses_bufcache, CTLFLAG_RW, &getpages_uses_bufcache, 0, "");
 
 /*
@@ -178,8 +178,7 @@ ffs_read(struct vop_read_args *ap)
                /*
                 * otherwise use the general form
                 */
-               error = uiomove((char *)bp->b_data + blkoffset, 
-                               (int)xfersize, uio);
+               error = uiomove(bp->b_data + blkoffset, (int)xfersize, uio);
 
                if (error)
                        break;
@@ -318,10 +317,26 @@ ffs_write(struct vop_write_args *ap)
                if (uio->uio_offset + xfersize > ip->i_size)
                        vnode_pager_setsize(vp, uio->uio_offset + xfersize);
 
+#if 0
                /*      
-                * We must perform a read-before-write if the transfer
-                * size does not cover the entire buffer, or if doing
-                * a dummy write to flush the buffer.
+                * If doing a dummy write to flush the buffer for a
+                * putpages we must perform a read-before-write to
+                * fill in any missing spots and clear any invalid
+                * areas.  Otherwise a multi-page buffer may not properly
+                * flush.
+                *
+                * We must clear any invalid areas
+                */
+               if (uio->uio_segflg == UIO_NOCOPY) {
+                       error = ffs_blkatoff(vp, uio->uio_offset, NULL, &bp);
+                       if (error)
+                               break;
+                       bqrelse(bp);
+               }
+#endif
+
+               /*
+                * We must clear invalid areas.
                 */
                if (xfersize < fs->fs_bsize || uio->uio_segflg == UIO_NOCOPY)
                        flags |= B_CLRBUF;
@@ -329,7 +344,7 @@ ffs_write(struct vop_write_args *ap)
                        flags &= ~B_CLRBUF;
 /* XXX is uio->uio_offset the right thing here? */
                error = VOP_BALLOC(vp, uio->uio_offset, xfersize,
-                   ap->a_cred, flags, &bp);
+                                  ap->a_cred, flags, &bp);
                if (error != 0)
                        break;
                /*
@@ -355,8 +370,7 @@ ffs_write(struct vop_write_args *ap)
                if (size < xfersize)
                        xfersize = size;
 
-               error =
-                   uiomove((char *)bp->b_data + blkoffset, (int)xfersize, uio);
+               error = uiomove(bp->b_data + blkoffset, (int)xfersize, uio);
                if ((ioflag & (IO_VMIO|IO_DIRECT)) && 
                    (LIST_FIRST(&bp->b_dep) == NULL)) {
                        bp->b_flags |= B_RELBUF;
@@ -380,7 +394,7 @@ ffs_write(struct vop_write_args *ap)
                } else if (xfersize + blkoffset == fs->fs_bsize) {
                        if ((vp->v_mount->mnt_flag & MNT_NOCLUSTERW) == 0) {
                                bp->b_flags |= B_CLUSTEROK;
-                               cluster_write(bp, (off_t)ip->i_size, vp->v_mount->mnt_stat.f_iosize, seqcount);
+                               cluster_write(bp, (off_t)ip->i_size, fs->fs_bsize, seqcount);
                        } else {
                                bawrite(bp);
                        }
index f1c66b8..67fad1c 100644 (file)
@@ -1055,8 +1055,9 @@ vm_fault_object(struct faultstate *fs,
 readrest:
                /*
                 * We have found an invalid or partially valid page, a
-                * potentially fully valid page with a read-ahead mark,
-                * or we have allocated a new page.
+                * page with a read-ahead mark which might be partially or
+                * fully valid (and maybe dirty too), or we have allocated
+                * a new page.
                 *
                 * Attempt to fault-in the page if there is a chance that the
                 * pager has it, and potentially fault in additional pages
@@ -1065,7 +1066,6 @@ readrest:
                 * We are NOT in splvm here and if TRYPAGER is true then
                 * fs.m will be non-NULL and will be PG_BUSY for us.
                 */
-
                if (TRYPAGER(fs)) {
                        int rv;
                        int seqaccess;
@@ -1159,13 +1159,16 @@ skip:
                         * object).  If it does so it is responsible for
                         * cleaning up the passed page and properly setting
                         * the new page PG_BUSY.
+                        *
+                        * If we got here through a PG_RAM read-ahead
+                        * mark the page may be partially dirty and thus
+                        * not freeable.  Don't bother checking to see
+                        * if the pager has the page because we can't free
+                        * it anyway.  We have to depend on the get_page
+                        * operation filling in any gaps whether there is
+                        * backing store or not.
                         */
-                       if (vm_pager_has_page(fs->object, pindex)) {
-                               rv = vm_pager_get_page(fs->object, &fs->m,
-                                                      seqaccess);
-                       } else {
-                               rv = VM_PAGER_FAIL;
-                       }
+                       rv = vm_pager_get_page(fs->object, &fs->m, seqaccess);
 
                        if (rv == VM_PAGER_OK) {
                                /*
index e8515d5..b4e9c52 100644 (file)
@@ -1581,6 +1581,11 @@ vm_object_page_remove_callback(vm_page_t p, void *data)
        /*
         * Wired pages cannot be destroyed, but they can be invalidated
         * and we do so if clean_only (limit) is not set.
+        *
+        * WARNING!  The page may be wired due to being part of a buffer
+        *           cache buffer, and the buffer might be marked B_CACHE.
+        *           This is fine as part of a truncation but VFSs must be
+        *           sure to fix the buffer up when re-extending the file.
         */
        if (p->wire_count != 0) {
                vm_page_protect(p, VM_PROT_NONE);