Re: [PATCH 04/10] AXFS: axfs_inode.c

[Arnd Bergmann <arnd@arndb.de>]

On Thursday 21 August 2008, Jared Hulbert wrote:> +       array_index = AXFS_GET_INODE_ARRAY_INDEX(sbi, ino_number);> +       array_index += page->index;> +> +       node_index = AXFS_GET_NODE_INDEX(sbi, array_index);> +       node_type = AXFS_GET_NODE_TYPE(sbi, array_index);> +> +       if (node_type == Compressed) {> +               /* node is in compessed region */> +               cnode_offset = AXFS_GET_CNODE_OFFSET(sbi, node_index);> +               cnode_index = AXFS_GET_CNODE_INDEX(sbi, node_index);> +               down_write(&sbi->lock);> +               if (cnode_index != sbi->current_cnode_index) {> +                       /* uncompress only necessary if different cblock */> +                       ofs = AXFS_GET_CBLOCK_OFFSET(sbi, cnode_index);> +                       len = AXFS_GET_CBLOCK_OFFSET(sbi, cnode_index + 1);> +                       len -= ofs;> +                       axfs_copy_data(sb, cblk1, &(sbi->compressed), ofs, len);> +                       axfs_uncompress_block(cblk0, cblk_size, cblk1, len);> +                       sbi->current_cnode_index = cnode_index;> +               }> +               downgrade_write(&sbi->lock);> +               max_len = cblk_size - cnode_offset;> +               len = max_len > PAGE_CACHE_SIZE ? PAGE_CACHE_SIZE : max_len;> +               src = (void *)((unsigned long)cblk0 + cnode_offset);> +               memcpy(pgdata, src, len);> +               up_read(&sbi->lock);
This looks very nice, but could use some comments about how the data isactually stored on disk. It took me some time to figure out that it actuallyallows to do tail merging into compressed blocks, which I was about to suggestyou implement ;-). Cramfs doesn't have them, and I found that they are themain reason why squashfs compresses better than cramfs, besides the defaultblock size, which you can change on either one.
Have you seen any benefit of the rwsem over a simple mutex? I would guessthat you can never even get into the situation where you get concurrentreaders since I haven't found a single down_read() in your code, onlydowngrade_write().
	Arnd <><ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥