Handle fsync of inodes correctly

get rid of lafs_write_inode as it doesn't do the right thing.
Instead, create updates for inode changes only when
fsync is called on an inode.  The only other time we
flush out an inode is 'sync()' which does a checkpoint
so achieves the same with not updates in clusters.

Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/file.c b/file.c
index 722e811fb2a3291e8bb890184720b2ebd62d044e..4c4b36d495dc83c996e1ef6431cb64e47ee21692 100644 (file)
--- a/file.c
+++ b/file.c
@@ -364,6 +364,20 @@ static void lafs_sync_page(struct page *page)
        }
 }
 
+static int
+lafs_sync_file(struct file *file, struct dentry *de, int datasync)
+{
+       int err;
+       err = lafs_sync_inode(de->d_inode, 0);
+
+       /* FIXME I ignore datasync, just like file_fsync */
+       err = write_inode_now(de->d_inode, 0) ?: err;
+
+       if (err == 0)
+               lafs_sync_inode(de->d_inode, 1);
+       return err;
+}
+
 const struct file_operations lafs_file_file_operations = {
        .llseek         = generic_file_llseek,
        .read           = do_sync_read,
@@ -374,7 +388,7 @@ const struct file_operations lafs_file_file_operations = {
        .mmap           = generic_file_mmap,
        .open           = generic_file_open,
 /*     .release        = lafs__release_file,*/
-/*     .fsync          = lafs_sync_file, */
+       .fsync          = lafs_sync_file,
        .splice_read    = generic_file_splice_read,
        .splice_write   = generic_file_splice_write,
 };

diff --git a/inode.c b/inode.c
index be202829803f543400f59cd95dcf7164e0277dcb..234aa8e8601d6079690d1118431aaa3ba0d6ab5f 100644 (file)
--- a/inode.c
+++ b/inode.c
@@ -990,62 +990,56 @@ void lafs_dirty_inode(struct inode *ino)
        ino->i_sb->s_dirt = 1;
 }
 
-int lafs_write_inode(struct inode *ino, int wait)
+int lafs_sync_inode(struct inode *ino, int wait)
 {
-       /* If inode has been changed, we want to write out
-        * an update record for this inode.
-        * If 'wait' we wait for the last update record to
-        * complete.
-        * If we cannot (could not) create an update record,
+       /* fsync has been called on this file so we need
+        * to sync any inode updates to the next cluster.
+        *
+        * If we cannot create an update record,
         * we wait for a phase change, which writes everything
         * out.
-        *
-        * This is called with wait==0 before doing a sync.
-        * ->sync_fs gets called before we are called again with wait==1.
-        * ->sync_fs will flush all inodes so the second call should have
-        * nothing to do.
-        * So if wait==0, we do nothing(later).
         */
        struct datablock *b;
        struct fs *fs = fs_from_inode(ino);
+       struct update_handle uh;
+       int err;
 
-       if (wait == 0)
+       if (wait) {
+               if (LAFSI(ino)->update_cluster > 1)
+                       lafs_cluster_wait(fs, LAFSI(ino)->update_cluster);
+               if (LAFSI(ino)->update_cluster == 1) {
+                       lafs_checkpoint_lock(fs);
+                       lafs_checkpoint_unlock_wait(fs);
+               }
                return 0;
+       }
 
+       LAFSI(ino)->update_cluster = 0;
+       if (!test_bit(I_Dirty, &LAFSI(ino)->iflags))
+               return 0;
        b = lafs_inode_dblock(ino, SYNC, MKREF(write_inode));
-       if (IS_ERR(b)) /* FIXME should I mark something clean?? */
+       if (IS_ERR(b))
                return PTR_ERR(b);
 
-       /* FIXME this needs lots of thought... I think.
-        * In what circumstance do we do an update, and in
-        * what circumstance do we just write the block,
-        * and how do we move dirty flags around when we do that?
-        * And these iblock references should either go or be locked.
-        */
-       if (test_bit(B_Dirty, &b->b.flags)
-           && !test_bit(B_Pinned, &b->b.flags)
-           && LAFSI(ino)->iblock != NULL) {
-               struct update_handle uh;
-               int err;
-               LAFSI(ino)->update_cluster = 0;
-               BUG_ON(LAFSI(ino)->iblock == NULL);
-
-               err = lafs_cluster_update_prepare(&uh, fs, LAFS_INODE_LOG_SIZE);
-               if (err)
+       lafs_iolock_written(&b->b);
+       lafs_inode_fillblock(ino);
+       lafs_iounlock_block(&b->b);
+
+       err = lafs_cluster_update_prepare(&uh, fs, LAFS_INODE_LOG_SIZE);
+       if (err)
+               lafs_cluster_update_abort(&uh);
+       else {
+               lafs_checkpoint_lock(fs);
+               if (lafs_cluster_update_pin(&uh) == 0) {
+                       if (test_and_clear_bit(B_Dirty, &b->b.flags))
+                               lafs_space_return(fs, 1);
+                       LAFSI(ino)->update_cluster =
+                               lafs_cluster_update_commit
+                               (&uh, b, LAFS_INODE_LOG_START,
+                                LAFS_INODE_LOG_SIZE);
+               } else  
                        lafs_cluster_update_abort(&uh);
-               else {
-                       lafs_checkpoint_lock(fs);
-                       if (lafs_cluster_update_pin(&uh) == 0) {
-                               if (test_and_clear_bit(B_Dirty, &b->b.flags))
-                                       lafs_space_return(fs, 1);
-                               LAFSI(ino)->update_cluster =
-                                       lafs_cluster_update_commit
-                                       (&uh, b, LAFS_INODE_LOG_START,
-                                        LAFS_INODE_LOG_SIZE);
-                       } else
-                               lafs_cluster_update_abort(&uh);
-                       lafs_checkpoint_unlock(fs);
-               }
+               lafs_checkpoint_unlock(fs);
        }
        if (test_bit(B_Dirty, &b->b.flags)) {
                /* FIXME need to write out the data block...
@@ -1053,18 +1047,12 @@ int lafs_write_inode(struct inode *ino, int wait)
                 */
        }
 
-       if (wait) {
-               if (LAFSI(ino)->update_cluster)
-                       lafs_cluster_wait(fs, LAFSI(ino)->update_cluster);
-               else {
-                       struct indexblock *ib;
-                       lafs_checkpoint_lock(fs);
-                       ib = LAFSI(ino)->iblock;
-                       if (test_bit(B_Dirty, &ib->b.flags))
-                               lafs_checkpoint_unlock_wait(fs);
-                       else
-                               lafs_checkpoint_unlock(fs);
-               }
+       if (LAFSI(ino)->update_cluster == 0) {
+               lafs_checkpoint_lock(fs);
+               if (test_bit(B_Dirty, &b->b.flags))
+                       LAFSI(ino)->update_cluster = 1;
+               lafs_checkpoint_start(fs);
+               lafs_checkpoint_unlock(fs);
        }
        putdref(b, MKREF(write_inode));
        return 0; /* FIXME should I return some error message??? */
@@ -1078,6 +1066,8 @@ void lafs_inode_fillblock(struct inode *ino)
        struct datablock *db = li->dblock;
        struct la_inode *lai;
 
+       clear_bit(I_Dirty, &LAFSI(ino)->iflags);
+
        lai = map_dblock(db);
        lai->data_blocks = cpu_to_le32(li->cblocks);
        lai->index_blocks = cpu_to_le32(li->ciblocks);

diff --git a/lafs.h b/lafs.h
index f51d0a961f45333f28f505e5d60ebe07feb90684..0331018a5dc6e4e8057c1ae4afbfd7966b68401c 100644 (file)
--- a/lafs.h
+++ b/lafs.h
@@ -143,7 +143,7 @@ void lafs_inode_checkpin(struct inode *ino);
 void lafs_clear_inode(struct inode *ino);
 void lafs_delete_inode(struct inode *ino);
 void lafs_dirty_inode(struct inode *ino);
-int lafs_write_inode(struct inode *ino, int wait);
+int lafs_sync_inode(struct inode *ino, int wait);
 struct inode *lafs_new_inode(struct fs *fs, struct inode *dir, int type,
                             int inum, int mode, struct datablock **inodbp);
 int lafs_lock_inode(struct inode *ino);

diff --git a/super.c b/super.c
index 57147672040b61ee61939c373c980d5689590cd3..e3104da5517be9dddf1d4a72db2bb4cf561679d1 100644 (file)
--- a/super.c
+++ b/super.c
@@ -1060,7 +1060,7 @@ static struct super_operations lafs_sops = {
        .destroy_inode  = lafs_destroy_inode,  /* Inverse of 'alloc_inode' */
        /* Don't use read_inode */
        .dirty_inode    = lafs_dirty_inode,
-       .write_inode    = lafs_write_inode,
+       /* .write_inode not needed */
        /* put_inode ?? */
        .drop_inode     = lafs_drop_inode,
        /* drop_inode ?? */                     /* default will call delete or forget