On Wed, 09 Aug 2006 18:20:26 -0700
Mingming Cao <cmm@xxxxxxxxxx> wrote:
>
> Add extent map support to ext4. Patch from Alex Tomas.
>
> On disk extents format:
> /*
> * this is extent on-disk structure
> * it's used at the bottom of the tree
> */
> struct ext3_extent {
> __le32 ee_block; /* first logical block extent covers */
> __le16 ee_len; /* number of blocks covered by extent */
> __le16 ee_start_hi; /* high 16 bits of physical block */
> __le32 ee_start; /* low 32 bigs of physical block */
> };
>
>From a quick scan:
- The code is very poorly commented. I'd want to spend a lot of time
reviewing this implementation, but not in its present state.
- Far, far too many inlines
- overly-terse variable naming
- There are several places which appear to be putting block numbers into
an `int'.
- Needs kmalloc()->kzalloc() conversion
- replace all brelse() calls with put_bh(). Because brelse() is
old-fashioned, has a weird name and neelessly permits a NULL arg.
In fact it would be beter to convert JBD and ext3 to put_bh before
copying it all over.
- The open-coded __clear_bit(BH_New, ...) in ext4_ext_get_blocks is a bit
nasty. We can live with nasty, but are we sure that it isn't buggy??
- It has about 7,000 instances of
if ((lhs = expression)) {
whereas the preferred coding style is
lhs = expression;
if (lhs) {
- The existing comments could benefit from some rework by a native English
speaker.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
