From: NeilBrown Date: Mon, 21 Jun 2010 03:40:16 +0000 (+1000) Subject: Revise rule for inode data blocks as leafs. X-Git-Url: http://git.neil.brown.name/?a=commitdiff_plain;h=e396a0fd6f5e306cde87fd1790ddb703bcbf558f;p=LaFS.git Revise rule for inode data blocks as leafs. We cannot process an inode data block as a leaf before processing the InoIdx block. Previously we would unpin an inode data block if the InoIdx block should take priority. But that is problematic. Instead we simply take the inode data block off the leaf list. This means we have to put it back on when the InoIdx gets unpinned or phase-flipped. At same time, tidy up determination of 'is a leaf' as this is used both when adding something to a leaf list, and when taking something off. Signed-off-by: NeilBrown --- diff --git a/checkpoint.c b/checkpoint.c index 30007ee..6689afc 100644 --- a/checkpoint.c +++ b/checkpoint.c @@ -307,11 +307,13 @@ static int prepare_checkpoint(struct fs *fs) */ struct block *lafs_get_flushable(struct fs *fs, int phase) { - struct block *b; - struct inode *in; - struct lafs_inode *lai; + struct block *b = NULL; retry: + if (b) { + lafs_iounlock_block(b); + putref(b, MKREF(leaf)); + } spin_lock(&fs->lock); if (!list_empty(&fs->clean_leafs)) b = list_entry(fs->clean_leafs.next, struct block, lru); @@ -320,45 +322,22 @@ struct block *lafs_get_flushable(struct fs *fs, int phase) else b = NULL; - /* If this is an inode data block and the InoIdx block is - * also flushable (or will be soon), then de-pin this block. - * The InoIdx will hold and eventually pin it. - */ - if (b && !test_bit(B_Index, &b->flags) && - LAFSI(b->inode)->type == TypeInodeFile && - (in = dblk(b)->my_inode) && - (lai = LAFSI(in)) && - lai->iblock != NULL && - test_bit(B_Pinned, &lai->iblock->b.flags) && - (phase == !!test_bit(B_Phase1, &lai->iblock->b.flags) || - (phase == -1 && test_bit(B_Realloc, &lai->iblock->b.flags))) - ) { - /* Don't really want this to be pinned yet. When the - * InoIdx block gets processed we will get pinned again. - */ - list_del_init(&b->lru); - clear_bit(B_Pinned, &b->flags); - if (!test_bit(B_Root, &b->flags)) - atomic_dec(&b->parent->pincnt[!!test_bit(B_Phase1, &b->flags)]); - spin_unlock(&fs->lock); - putref(b, MKREF(leaf)); - goto retry; - } + if (b) /* the list counted a reference. Now we hold it */ list_del_init(&b->lru); spin_unlock(&fs->lock); - if (b) { - /* Need an iolock, but if the block is in writeback, - * we don't want it. - */ - lafs_iolock_block(b); - if (test_bit(B_Writeback, &b->flags)) { - lafs_iounlock_block(b); - putref(b, MKREF(leaf)); - goto retry; - } - } + if (!b) + return b; + + /* Need an iolock, but if the block is in writeback, + * or unpinned or otherwise not a leaf, we don't want it. + */ + lafs_iolock_block(b); + + if (!lafs_is_leaf(b, phase)) + goto retry; + return b; } @@ -382,9 +361,7 @@ static void do_checkpoint(void *data) * de-async them. */ - if (!test_bit(B_Pinned, &b->flags)) - /* Haven't refiled since we cleared that */ ; - else if (!!test_bit(B_Phase1, &b->flags) == oldphase) { + if (!!test_bit(B_Phase1, &b->flags) == oldphase) { if (test_bit(B_Index, &b->flags) && (iblk(b)->uninc_table.pending_cnt || iblk(b)->uninc)) { diff --git a/index.c b/index.c index fc3d0c1..0c1523e 100644 --- a/index.c +++ b/index.c @@ -498,6 +498,9 @@ void lafs_phase_flip(struct fs *fs, struct indexblock *ib) } lafs_inode_checkpin(ib->b.inode); } + if (test_bit(B_InoIdx, &ib->b.flags)) + /* maybe data block should go in leaf list now */ + lafs_refile(&LAFSI(ib->b.inode)->dblock->b, 0); lafs_iounlock_block(&ib->b); return; } @@ -579,6 +582,9 @@ void lafs_phase_flip(struct fs *fs, struct indexblock *ib) if (lafs_prealloc(&ib->b, ReleaseSpace) < 0) pin_all_children(ib); + /* maybe data block needs to be on leaf list */ + lafs_refile(&LAFSI(ib->b.inode)->dblock->b, 0); + // FIXME move Dirty/UnincCredit to data block } @@ -657,6 +663,47 @@ void lafs_phase_flip(struct fs *fs, struct indexblock *ib) * A block pointing to a block0 never has a parent, and an iblock has the same * parent as the dblock, so there can never be more than 2 outstanding blocks. */ +int lafs_is_leaf(struct block *b, int ph) +{ + /* This is pinned and not iolocked. Should it be on a leaf + * list? + */ + struct lafs_inode *lai; + + if (!test_bit(B_Pinned, &b->flags) || + test_bit(B_Writeback, &b->flags)) + return 0; + + if (test_bit(B_Index, &b->flags)) { + if (ph >= 0) + return atomic_read(&iblk(b)->pincnt[ph]) == 0; + return (atomic_read(&iblk(b)->pincnt[0]) + + atomic_read(&iblk(b)->pincnt[1])) == 0; + } + + /* This is a data block */ + if (LAFSI(b->inode)->type != TypeInodeFile || + dblk(b)->my_inode == NULL) + /* Non-inode data blocks are always leafs */ + return 1; + + lai = LAFSI(dblk(b)->my_inode); + spin_lock(&lai->vfs_inode.i_data.private_lock); + if (ph < 0) + ph = !!test_bit(B_Phase1, &b->flags); + if (lai->iblock && + test_bit(B_Pinned, &lai->iblock->b.flags) && + !!test_bit(B_Phase1, &lai->iblock->b.flags) == ph) { + /* iblock still pinned in the same phase, so + * this block isn't a leaf + */ + spin_unlock(&lai->vfs_inode.i_data.private_lock); + return 0; + } + spin_unlock(&lai->vfs_inode.i_data.private_lock); + return 1; +} + void lafs_refile(struct block *b, int dec) { struct block *next = NULL, *next_parent = NULL; @@ -791,12 +838,16 @@ void lafs_refile(struct block *b, int dec) b->inode->i_nlink) checkpin = b->inode; if (!test_bit(B_Root, &b->flags)) { + struct block *nb; atomic_dec(&b->parent->pincnt[ph]); - if (next_parent != &b->parent->b) { + if (test_bit(B_InoIdx, &b->flags)) + nb = &LAFSI(b->inode)->dblock->b; + else + nb = &b->parent->b; + if (next_parent != nb) { LAFS_BUG(next_parent, b); - next_parent = &b->parent->b; - atomic_inc(&next_parent - ->refcnt); + next_parent = nb; + atomic_inc(&nb->refcnt); } } } @@ -805,11 +856,8 @@ void lafs_refile(struct block *b, int dec) /* Make sure lru is correct */ if ((list_empty(&b->lru) || test_bit(B_OnFree, &b->flags)) && - test_bit(B_Pinned, &b->flags) && !test_bit(B_IOLock, &b->flags) && - !test_bit(B_Writeback, &b->flags) && - (!test_bit(B_Index, &b->flags) || - atomic_read(&iblk(b)->pincnt[ph]) == 0)) { + lafs_is_leaf(b, ph)) { if (test_and_clear_bit(B_OnFree, &b->flags)) { /* lock hashlock - but we have that */ list_del_init(&b->lru); diff --git a/lafs.h b/lafs.h index 91c6531..ce7c0ba 100644 --- a/lafs.h +++ b/lafs.h @@ -357,6 +357,7 @@ static inline int set_phase(struct block *b, int ph) * 'putref' drops the count and calls lafs_refile to see if anything * has changed. */ +int lafs_is_leaf(struct block *b, int ph); void lafs_refile(struct block *b, int dec); extern spinlock_t lafs_hash_lock;