From cdcb7df44acf97dd43de273a134893297897a254 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Mon, 28 Jun 2010 11:22:45 +1000 Subject: [PATCH] Change dirty_inode. Again. Just set I_Dirty and obey that when we write_cluster. If a change to the inode has to happen after the checkpoint, it will come through setattr which will wait for the block to be written (in pin_dblock). Possibly mtime updates will slip through, but we could set S_NOCTIME and do the updates more transactionally outselves. Signed-off-by: NeilBrown --- README | 52 +++++++++++++++++++++++++++++++++++++++-- cluster.c | 8 ++++++- dir.c | 3 +++ inode.c | 69 ++++++++++++++----------------------------------------- 4 files changed, 77 insertions(+), 55 deletions(-) diff --git a/README b/README index 120ad15..2a41a35 100644 --- a/README +++ b/README @@ -4797,7 +4797,7 @@ DONE 5a/ index.c:1982. Data block with Phys and no UnincCredit DONE 5b/ phase_flip/pin_all_children/lafs_refile finds refcnt == 0; I guess we should getref/putref. - 5c/ dirty_inode might find InoIdx is allocated by datablock not +DONE 5c/ dirty_inode might find InoIdx is allocated but datablock not and doesn't cope well. DONE 5d/ At unmount, 16/1 is still pinned. @@ -4825,7 +4825,7 @@ Why have I no credits? Cleaning is racing with truncate, and that cannot happen!! - 7a/ block.c:507 in lafs_dirty_dblock - no credits for 0/2 +DONE 7a/ block.c:507 in lafs_dirty_dblock - no credits for 0/2 block.c:507: [cfa63c58]0/2(4348)r2F:Valid,Dirty,Writeback,PhysValid cluster(1) iblock(1) in touch_atime. I think I know this one. @@ -4917,6 +4917,11 @@ DONE 8/ looping in do_checkpoint 15c/ roll-forward should not update index if physaddr hasn't changed (roll_block) +DONE 15d/ What does I_Dirty mean - and implement it. + +15e/ setattr should queue an update for the inode metadata. + +15f/ include timestamp in cluster_head to set mtime/ctime properly on roll-forward? ## Items from 6 jul 2007. 16/ Update locking.doc @@ -5125,3 +5130,46 @@ DONE 8/ looping in do_checkpoint Normally we need a phase-lock when pinning a data block so that we don't lose the pinning before we dirty. But as we phase_flip these it doesn't matter... So just add that too the test?? + +28June2010 + Reflecting on 5c - dirty_inode might find InoIdx pre-allocated but + datablock not, and doesn't cope. + We either prealloc both, which seems clumsy, or always defer + to InoIdx if it is present and pinned. + lafs_prealloc does both Index and Data blocks for inode. + But Data could lose as writeout while index will replenish at + phase_flip, so maybe not a good idea. + If lafs_allocate_cluster finds a Dirty InoIdx it will copy the Dirty + credits across to the data block (on non-cleaning segments) so the + Data block doesn't need to have credits. + + dirty_inode gets called: + {__,}mark_inode_dirty{,_sync} + inode_{inc,dec}_link_count + [[various quota ops]] + inode_setattr + touch_atime + file_accessed + file_update_time + generic_file_...write + do_wp_page + + updates through inode_setattr go to lafs_setattr so the + data block will be pinpending and the checkpoint lock will be held. + + updates through inode_*_link_count happen in filesystem and the inode data + block is PinPending, or a block in the file is pinned and will be + dirty, so it will get written. + + updates through touch_atime or file_update_time are unexpected and + cannot be prepared for. file_update_time changes will be caught by + normal file writeout. atime changes will be lost until we get the + atime file working. + + So: + dirty_inode cannot change the block as it might be in writeout, and + it cannot lock anything as it might be in touch_atime which shouldn't + block and cannot fail. + So just set I_Dirty and use that to flush inode to db at writeout. + Any changes which must be in the next phase will come via setattr and + so will wait for incompatible changes to be written out. diff --git a/cluster.c b/cluster.c index de9a453..d2467c9 100644 --- a/cluster.c +++ b/cluster.c @@ -602,7 +602,6 @@ unsigned long long lafs_cluster_allocate(struct block *b, int cnum) if (test_bit(B_InoIdx, &b->flags)) { struct block *b2 = &lai->dblock->b; int credits = 0; - /* FIXME should I inode_fillblock here?? */ LAFS_BUG(!test_bit(B_Pinned, &b->flags), b); if (!test_bit(B_Pinned, &b2->flags)) { @@ -675,6 +674,13 @@ unsigned long long lafs_cluster_allocate(struct block *b, int cnum) return wc->cluster_seq; } + if (!test_bit(B_Index, &b->flags) && + LAFSI(b->inode)->type == TypeInodeFile) { + struct inode *myi = dblk(b)->my_inode; + if (test_and_clear_bit(I_Dirty, &LAFSI(myi)->iflags)) + lafs_inode_fillblock(myi); + } + /* FIXME these are no longer needed */ if (!test_bit(B_Valid, &b->flags)) printk("Not Valid in allocate %s\n", strblk(b)); diff --git a/dir.c b/dir.c index 4728b2f..1bb8576 100644 --- a/dir.c +++ b/dir.c @@ -684,6 +684,7 @@ retry: inode_inc_link_count(inode); lafs_inode_checkpin(inode); + lafs_dirty_dblock(inodb); clear_bit(B_PinPending, &inodb->b.flags); putdref(inodb, MKREF(inode_update)); @@ -750,6 +751,7 @@ retry: dir_delete_commit(&doh, fs, dir, de->d_name.name, de->d_name.len); lafs_checkpoint_unlock(fs); lafs_inode_checkpin(inode); + lafs_dirty_dblock(inodb); clear_bit(B_PinPending, &inodb->b.flags); putdref(inodb, MKREF(inode_update)); return 0; @@ -832,6 +834,7 @@ retry: dir_log_commit(&uh, fs, dir, &de->d_name, inode->i_ino, DIROP_UNLINK, NULL); dir_delete_commit(&doh, fs, dir, de->d_name.name, de->d_name.len); + lafs_dirty_dblock(inodb); clear_bit(B_PinPending, &inodb->b.flags); putdref(inodb, MKREF(inode_update)); lafs_inode_checkpin(inode); diff --git a/inode.c b/inode.c index 0877079..58e5a0c 100644 --- a/inode.c +++ b/inode.c @@ -856,56 +856,21 @@ void lafs_dirty_inode(struct inode *ino) { /* this is called in one of three cases: * 1/ by lafs internally when dblock or iblock is pinned and - * ready to be dirties + * ready to be dirtied * 2/ by writeout before requesting a write - to update mtime * 3/ by read to update atime * - * In 1/ we are free to dirty the block. - * In 2/ we can just update the block as it will get dirtied soon - * In 3/ we just punt for now and worry about it later + * As we don't know which, there is not much we can do. + * We mustn't update the data block as it could be in + * writeout and we cannot always wait safely. + * So require that anyone who really cares, dirties the datablock + * or a child themselves. + * When cluster_allocate eventually gets called, it will update + * the datablock from the inode. + * If an update has to wait for the next phase, lock_dblock + * (e.g. in setattr) will do that. */ - struct block *b; - struct datablock *db; - int nodirty = 0; - int ino_phase; - struct fs *fs = fs_from_inode(ino); - spin_lock(&ino->i_data.private_lock); - if (LAFSI(ino)->iblock) - b = &getiref_locked(LAFSI(ino)->iblock, - MKREF(dirty_inode))->b; - else if (LAFSI(ino)->dblock) - b = getref_locked(&LAFSI(ino)->dblock->b, MKREF(dirty_inode)); - else - b = NULL; - db = LAFSI(ino)->dblock; - spin_unlock(&ino->i_data.private_lock); - - BUG_ON(b==NULL); - - lafs_checkpoint_lock(fs); - if (test_bit(B_Pinned, &db->b.flags)) - ino_phase = !!test_bit(B_Phase1, &db->b.flags); - else if (test_bit(B_Pinned, &b->flags)) - ino_phase = !!test_bit(B_Phase1, &b->flags); - else { - ino_phase = fs->phase; - nodirty = 1; - } - - if (ino_phase == fs->phase) { - lafs_inode_fillblock(ino); - if (!nodirty) { - /* Pin to phase is safe as if it isn't - * pinned already, then InoIdx is Pinned, - * so all the prealloc is done - */ - lafs_pin_block(&db->b); - lafs_dirty_dblock(db); - } - } else - set_bit(I_Dirty, &LAFSI(ino)->iflags); - lafs_checkpoint_unlock(fs); - putref(b, MKREF(dirty_inode)); + set_bit(I_Dirty, &LAFSI(ino)->iflags); } int lafs_write_inode(struct inode *ino, int wait) @@ -1453,14 +1418,14 @@ again: lafs_checkpoint_unlock_wait(fs); goto again; } - /* inode_setattr calls lafs_dirty_inode, which updates - * the dblock as needed. + /* inode_setattr calls lafs_dirty_inode, which sets + * I_Dirty so the dblock will get updated. */ err = err ?: inode_setattr(ino, attr); - if (db) { - clear_bit(B_PinPending, &db->b.flags); - putdref(db, MKREF(setattr)); - } + if (!err) + lafs_dirty_dblock(db); + clear_bit(B_PinPending, &db->b.flags); + putdref(db, MKREF(setattr)); lafs_checkpoint_unlock(fs); return err; -- 2.39.5