Hi,All.
I am a newer to linux kernel.I have a question about the ext2_get_branch.
static Indirect *ext2_get_branch(struct inode *inode,
int depth,
int *offsets,
Indirect chain[4],
int *err)
{
struct super_block *sb = inode->i_sb;
Indirect *p = chain;
struct buffer_head *bh;
*err = 0;
/* i_data is not going away, no lock needed */
add_chain (chain, NULL, EXT2_I(inode)->i_data + *offsets);
if (!p->key)
goto no_block;
while (--depth) {
bh = sb_bread(sb, le32_to_cpu(p->key));
if (!bh)
goto failure;
read_lock(&EXT2_I(inode)->i_meta_lock);
if (!verify_chain(chain, p))
goto changed;
add_chain(++p, bh, (__le32*)bh->b_data + *++offsets);
read_unlock(&EXT2_I(inode)->i_meta_lock);
if (!p->key)
goto no_block;
}
return NULL;
changed:
read_unlock(&EXT2_I(inode)->i_meta_lock);
brelse(bh);
*err = -EAGAIN;
goto no_block;
failure:
*err = -EIO;
no_block:
return p;
}
Can it be changed like this?:
static Indirect *ext2_get_branch(struct inode *inode,
int depth,
int *offsets,
Indirect chain[4],
int *err)
{
struct super_block *sb = inode->i_sb;
Indirect *p = chain;
struct buffer_head *bh;
*err = 0;
/* i_data is not going away, no lock needed */
add_chain (chain, NULL, EXT2_I(inode)->i_data + *offsets);
if (!p->key)
goto no_block;
while (--depth) {
bh = sb_bread(sb, le32_to_cpu(p->key));
if (!bh)
goto failure;
- read_lock(&EXT2_I(inode)->i_meta_lock);
if (!verify_chain(chain, p))
goto changed;
add_chain(++p, bh, (__le32*)bh->b_data + *++offsets);
- read_unlock(&EXT2_I(inode)->i_meta_lock);
if (!p->key)
goto no_block;
}
+ read_lock(&EXT2_I(inode)->i_meta_lock);
+ if (!verify_chain(chain, p))
+ goto changed;
+ add_chain(++p, bh, (__le32*)bh->b_data + *++offsets);
return NULL;
changed:
- read_unlock(&EXT2_I(inode)->i_meta_lock);
brelse(bh);
*err = -EAGAIN;
goto no_block;
failure:
*err = -EIO;
no_block:
return p;
}
I think in the while loop it doesn't need read_lock.When verify_chain() is doing
the verify , a writer change the content,the verity_chain will find the change
or at last the change will be found out of the while loop.