On Thu, Jan 31, 2013 at 06:03:06AM -0700, Alex Lyakas wrote: > Hi, > I want to check if any of the below issues are worth/should be fixed: > > # btrfs_ioctl_snap_destroy() does not commit a transaction. As a > result, user can ask to delete a subvol, he receives "ok" back. Even > if user does "btrfs sub list", > he will not see the deleted subvol (even though the transaction was > not committed yet). But if a crash happens, ORPHAN_ITEM will not > re-appear after crash. > So after crash, the subvolume still exists perfectly fine (happened > couple of times here).
Same thing happens to normal unlinks, I don't see a reason to have different rules for subvols. > > # btrfs_drop_snapshot() does not commit a transaction after > btrfs_del_orphan_item(). So if the subvol deletion completed in one go > (did not have to detach and re-attach to transaction, thus committing > the ORPHAN_ITEM and drop_progress/level), then after crash ORPHAN_ITEM > will not be in the tree, and subvolume still exists. > Again same thing happens with normal files. > # btrfs_drop_snapshot() checks btrfs_should_end_transaction(), and > then does btrfs_end_transaction_throttle() and > btrfs_start_transaction(). However, it looks like it can rejoin the > same transaction if transaction was not not blocked yet. Minor issue, > perhaps? No if we didn't block then its ok and we wait longer, we only throttle to give the transaction stuff a chance to commit, so if the join logic decides its ok to go on then we're good. > > # umount may get delayed because of pending-for-deletion subvolumes: > btrfs_commit_super() locks the cleaner_mutex, so it will wait for the > cleaner to complete. > On the other hand, cleaner will not give up until it completes > processing all its splice. If currently cleaner is not running, then > btrfs_commit_super() > calls btrfs_clean_old_snapshots() directly. So does it make sense: > - btrfs_commit_super() will not call btrfs_clean_old_snapshots() > - close_ctree() calls kthread_stop(cleaner_kthread) early, and cleaner > thread periodically checks if it needs to exit I don't quite follow this, but sure? Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
