> + unsigned compressed_extent_size;
It kind of jumps out that this mentions neither that it's the max nor
that it's in KB. How about max_compressed_extent_kb?
> + fs_info->compressed_extent_size = 128;
I'd put a DEFAULT_MAX_EXTENTS up by the MAX_ definition instead of using
a raw 128 here.
> + unsigned long max_compressed = root->fs_info->compressed_extent_size *
> 1024;
> + unsigned long max_uncompressed = root->fs_info->compressed_extent_size
> * 1024;
(so max_compressed is in bytes)
> nr_pages = (end >> PAGE_CACHE_SHIFT) - (start >> PAGE_CACHE_SHIFT) + 1;
> - nr_pages = min(nr_pages, (128 * 1024UL) / PAGE_CACHE_SIZE);
> + nr_pages = min(nr_pages, (max_compressed * 1024UL) / PAGE_CACHE_SIZE);
(and now that expression adds another * 1024, allowing {128,512}MB
extents :))
> * We also want to make sure the amount of IO required to do
> * a random read is reasonably small, so we limit the size of
> - * a compressed extent to 128k.
> + * a compressed extent (default of 128k).
Just drop the value so that this comment doesn't need to be updated
again.
- * a compressed extent to 128k.
+ * a compressed extent.
> + {Opt_compr_extent_size, "compressed_extent_size=%d"},
It's even more important to make the exposed option self-documenting
than it was to get the fs_info member right.
> + if ((intarg == 128) || (intarg == 512)) {
> + info->compressed_extent_size = intarg;
> + printk(KERN_INFO "btrfs: compressed extent size
> %d\n",
> + info->compressed_extent_size);
> + } else {
> + printk(KERN_INFO "btrfs: "
> + "Invalid compressed extent size,"
> + " using default.\n");
I'd print the default value when it's used and would include a unit in
both.
> + if (btrfs_test_opt(root, COMPR_EXTENT_SIZE))
> + seq_printf(seq, ",compressed_extent_size=%d",
> + (unsigned long long)info->compressed_extent_size);
The (ull) cast doesn't match the %d format and wouldn't be needed if it
was printed with %u.
- z
--
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
