Hi Chris, On Wed, 17 Dec 2008 21:52:55 -0500, Chris Mason wrote: > Hello, > > I've been meaning to read through nilfs for a while, and I grabbed the > code today to take a look. The code is very clean and it ran well out > of the box here.
Thank you very much for helpful comments! > One problem I hit early on was that nilfs doesn't seem to zero out the > block device during mkfs. So after mkfs.nilfs2, mount still found my > old btrfs filesystem. It would be a good idea to zero out the first and > last 1MB on the device (except for the first sector which might have the > partition table). Okay, I'll take this idea in mkfs.nilfs2. > I haven't dug too deeply in yet, but if there are parts you're most > interested in comments on, please let me know. Well, I feel that the following two matters are particularlly questionable and need to be checked: - struct the_nilfs: NILFS allows users to mount snapshots without making additional devices or volumes. This is achieved by sharing a block device among multiple mount instances (i.e. super_block structs). the_nilfs struct is used for this sharing. This approach seems to be peculiar to nilfs, and I feel it needs attention. - ioctl: Ioctl interface (routines and structures) were implemented in an own way. These seems to be checked whether to comply with the rules of ioctl design. > It looks like nilfs_writepage ignores WB_SYNC_NONE, which is used by > do_sync_mapping_range(). Thanks! I didn't notice that this function was added. Uum, it seems to require reconsidering the way to initiate writing of data pages. > nilfs_page_mkwrite doesn't seem to dirty the page? block_page_mkwrite > does more, including checks against i_size and others. I got it. The design of this function seems so old. It should be revised to do checks and block allocations previously at this function. > readpages and O_DIRECT both set b_size on the map_bh to reflect the > total size of the region they are trying to map. It seems like an easy > optimization to map more in nilfs_get_block. Yes, that's so true! Actually, I have unfinished patches to do this. This would decrease the numbers of get_block callbacks and b-tree lookups. And, as you pointed out, this seems one of the easiest optimization in some todos on performance. > Hopefully this helps, I'll try to send a few more ideas after I get to > know the code better. > > -chris Thanks, I've also started to read btrfs. I'll see it during the Christmas holidays ;) With regards, Ryusuke Konishi _______________________________________________ users mailing list [email protected] https://www.nilfs.org/mailman/listinfo/users
