]> git.neil.brown.name Git - LaFS.git/commitdiff
Combine cleaning and orphan list_heads.
authorNeilBrown <neilb@suse.de>
Sat, 14 Aug 2010 05:41:59 +0000 (15:41 +1000)
committerNeilBrown <neilb@suse.de>
Sat, 14 Aug 2010 07:58:20 +0000 (17:58 +1000)
A datablock is very rarely both an orphan and requiring cleaning, so
having two list_heads is a waste.

If is an orphan it will have full parent linkage and addresses already
so it will be handled promptly and removed from the cleaning list.

So arrange that if a block wants to be both, it is preferentially on
the cleaning list, and when removed from the cleaning list is gets
added back to the pending_orphan list in case it needs processing.

Note that only directory and inode blocks can ever be orphans so some
optimisation of spinlocks is possible.

Signed-off-by: NeilBrown <neilb@suse.de>
clean.c
orphan.c
state.h

diff --git a/clean.c b/clean.c
index aff7c95efd4cdea6499ea87f7a35dedd6c9dcd1e..641b6b483d6f7ad8940df0a4bb7ae7056d1c4847 100644 (file)
--- a/clean.c
+++ b/clean.c
@@ -295,8 +295,19 @@ static int try_clean(struct fs *fs, struct toclean *tc)
                                getdref(b, MKREF(cleaning));
                                igrab(ino);
                        }
-                       if (list_empty(&b->cleaning))
-                               list_add_tail(&b->cleaning, &tc->cleaning);
+                       if (LAFSI(ino)->type == TypeInodeFile ||
+                           LAFSI(ino)->type == TypeDir) {
+                               /* Could become an orphan just now, so need
+                                * to protect b->cleaning 
+                                */
+                               spin_lock(&fs->lock);
+                               list_move_tail(&b->cleaning, &tc->cleaning);
+                               spin_unlock(&fs->lock);
+                       } else {
+                               /* No locking needed */
+                               if (list_empty(&b->cleaning))
+                                       list_add_tail(&b->cleaning, &tc->cleaning);
+                       }
 
                        /* FIXME do I need a memory barrier. to ensure truncate
                         * sees the not-list_empty, and we see i_size? */
@@ -364,12 +375,22 @@ static int try_clean(struct fs *fs, struct toclean *tc)
                 * ref now
                 */
        done_cleaning:
+               clear_bit(B_Cleaning, &b->b.flags);
+
                list_del_init(&b->cleaning);
-               if (test_and_clear_bit(B_Cleaning, &b->b.flags)) {
-                       ino = b->b.inode;
-                       putdref(b, MKREF(cleaning));
-                       iput(ino);
+               if (test_bit(B_Orphan, &b->b.flags)) {
+                       spin_lock(&fs->lock);
+                       if (test_bit(B_Orphan, &b->b.flags) &&
+                           list_empty(&b->orphans)) {
+                               list_add(&b->orphans, &fs->pending_orphans);
+                               lafs_wake_thread(fs);
+                       }
+                       spin_unlock(&fs->lock);
                }
+
+               ino = b->b.inode;
+               putdref(b, MKREF(cleaning));
+               iput(ino);
                putref(cb, MKREF(clean2));
                if (rv)
                        goto out;
@@ -386,15 +407,26 @@ void lafs_unclean(struct datablock *db)
        if (!list_empty_careful(&db->cleaning)) {
                struct fs *fs = fs_from_inode(db->b.inode);
                mutex_lock(&fs->cleaner.lock);
-               if (!list_empty(&db->cleaning))
-                       list_del_init(&db->cleaning);
                if (test_and_clear_bit(B_Cleaning, &db->b.flags)) {
+                       /* This must be on the cleaner list, so
+                        * it is safe to delete without a spinlock
+                        */
+                       list_del_init(&db->cleaning);
                        putdref(db, MKREF(cleaning));
                        iput(db->b.inode);
                        if (test_and_clear_bit(B_Async, &db->b.flags)) {
                                putdref(db, MKREF(async));
                                lafs_wake_thread(fs);
                        }
+                       if (test_bit(B_Orphan, &db->b.flags)) {
+                               spin_lock(&fs->lock);
+                               if (test_bit(B_Orphan, &db->b.flags) &&
+                                   list_empty(&db->orphans)) {
+                                       list_add(&db->orphans, &fs->pending_orphans);
+                                       lafs_wake_thread(fs);
+                               }
+                               spin_unlock(&fs->lock);
+                       }
                }
                mutex_unlock(&fs->cleaner.lock);
        }
index 246e6ea2821026df6e01ece5da8dc5cd7aaef01c..30f18b8b474e1a416bc5dc1cc2656e0d6fe3c2d5 100644 (file)
--- a/orphan.c
+++ b/orphan.c
@@ -303,7 +303,7 @@ int lafs_make_orphan_nb(struct fs *fs, struct datablock *db, struct inode *ino)
        return err;
 }
 
-static int lafs_drop_orphan(struct fs *fs, struct datablock *db);
+static void lafs_drop_orphan(struct fs *fs, struct datablock *db);
 /*
  * When any processing of an orphan makes it not an orphan any more
  * (e.g. link is created for a file, directory block is cleaned)
@@ -554,7 +554,7 @@ long lafs_run_orphans(struct fs *fs)
        return timeout;
 }
 
-int lafs_drop_orphan(struct fs *fs, struct datablock *db)
+static void lafs_drop_orphan(struct fs *fs, struct datablock *db)
 {
        /* This block was an orphan but isn't any more.
         * Remove it from the list.
@@ -567,18 +567,13 @@ int lafs_drop_orphan(struct fs *fs, struct datablock *db)
        orphan_iput(ino);
 
        if (test_bit(B_Orphan, &db->b.flags))
-               return 0;
+               return;
        spin_lock(&fs->lock);
-       if (test_bit(B_Orphan, &db->b.flags) ||
-           list_empty(&db->orphans)) {
-               /* Is an orphan again, or it is already removed */
-               spin_unlock(&fs->lock);
-               return 0;
-       } else {
+       if (!test_bit(B_Orphan, &db->b.flags) &&
+           !list_empty_careful(&db->orphans) &&
+           !test_bit(B_Cleaning, &db->b.flags))
                list_del_init(&db->orphans);
-               spin_unlock(&fs->lock);
-               return 1;
-       }
+       spin_unlock(&fs->lock);
 }
 
 void lafs_add_orphan(struct fs *fs, struct datablock *db)
@@ -591,9 +586,8 @@ void lafs_add_orphan(struct fs *fs, struct datablock *db)
         */
        LAFS_BUG(!test_bit(B_Orphan, &db->b.flags), &db->b);
        spin_lock(&fs->lock);
-       if (list_empty(&db->orphans))
+       if (list_empty_careful(&db->orphans))
                list_add_tail(&db->orphans, &fs->pending_orphans);
-
        spin_unlock(&fs->lock);
        lafs_wake_thread(fs);
 }
@@ -604,7 +598,8 @@ void lafs_orphan_forget(struct fs *fs, struct datablock *db)
         * it just now.  When we do, lafs_add_orphan will be called */
        LAFS_BUG(!test_bit(B_Orphan, &db->b.flags), &db->b);
        spin_lock(&fs->lock);
-       if (!list_empty(&db->orphans))
+       if (!test_bit(B_Cleaning, &db->b.flags) &&
+           !list_empty_careful(&db->orphans))
                list_del_init(&db->orphans);
        spin_unlock(&fs->lock);
 }
diff --git a/state.h b/state.h
index 01b03d398b3ec75d19d80219ec54f083ad67de41..e155644942872ff76afad6fbba6171b2f21c974a 100644 (file)
--- a/state.h
+++ b/state.h
@@ -374,12 +374,18 @@ struct datablock {
        u32     orphan_slot;    /* slot in orphan file to record that this
                                 * block is an orphan
                                 */
-       struct list_head        orphans; /* linked list of blocks needing orphan
-                                         * processing.
-                                         */
-       struct list_head        cleaning; /* list of blocks being cleaned.
-                                          * Could share with orphans FIXME
+       union {
+               /* If a block is both an orphan and undergoing
+                * cleaning, it lives on the cleaning list until
+                * the cleaner has checked it.  It is then moved
+                * to the pending_orphans list.
+                */
+               struct list_head orphans; /* linked list of blocks needing orphan
+                                          * processing.
                                           */
+               struct list_head cleaning; /* list of blocks being cleaned.
+                                           */
+       };
        union {
                struct inode *my_inode; /* only valid for block holding an inode */
        };