Skip to contentSkip to conversation

ocfs2: Wrap inode block reads in a dedicated function.

PATCH New Joel Becker 2 participants 2 messages Oct 15, 2008 1 message not in the archive OCFS2

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

PATCH 1/1 ocfs2: Wrap inode block reads in a dedicated function.

[Oops, properly Cc'd now]

Quoted 4 lines · Mark Fasheh

Review int ocfs2_populate_inode(struct inode *inode, struct ocfs2_dinode *fe,

Quoted 1 line · Joel Becker
(quoted diff) +0 -0
@@ -232,25 +231,17 @@ int ocfs2_populate_inode(struct inode *inode, struct ocfs2_dinode *fe,

These changes to ocfs2_populate_inode() look great.

I thought you'd like that ;-)

Quoted 7 lines

Review int ocfs2_populate_inode(struct inode *inode, struct ocfs2_dinode *fe, · line 233

Quoted 3 lines
(quoted diff) +3 -0
+int ocfs2_read_inode_block(struct inode *inode, u64 blkno,
+			   struct buffer_head **bh)
+{

Can we get rid of the blkno argument here, and just get it from OCFS2_I(inode)->ip_blkno? Getting it out of ocfs2_read_inode_block_full() might be nice too, but not as big a deal since it isn't called as much.

I did that patch right before the concall. I just had to verify
my hunch that all callers had a valid ip_blkno. Attached ;-)

Quoted 3 lines

API-wise, this is a very nice change. It makes the checks we have more consistent. I think it'd be worth the time though to figure out how to execute the validation code only when we've actually read the disk.

I'm glad you like the API (I'm almost done with group
descriptors now too). Regarding 'only on read', when I have the callback in ocfs2_read_blocks() to handle that, I can easily lift all of the validation there (instead of just the checksum). Basically, I'm refactoring all the metadata reads before I do the read_blocks() hook. It seems a much easier to follow series.

Joel

Signature · 16 lines
"The nice thing about egotists is that they don't talk about other
 people."
         - Lucille S. Harper

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ocfs2-Callers-of-ocfs2_read_inode_block-don-t-nee.patch
Type: text/x-diff
Size: 0 bytes
Desc: not available
Url : http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20081014/e55f6a24/attachment.bin 
#2 lore raw

PATCH 1/1 ocfs2: Wrap inode block reads in a dedicated function.

Quoted 9 lines · Joel Becker
Quoted 3 lines

API-wise, this is a very nice change. It makes the checks we have more consistent. I think it'd be worth the time though to figure out how to execute the validation code only when we've actually read the disk.

I'm glad you like the API (I'm almost done with group
descriptors now too). Regarding 'only on read', when I have the callback in ocfs2_read_blocks() to handle that, I can easily lift all of the validation there (instead of just the checksum). Basically, I'm refactoring all the metadata reads before I do the read_blocks() hook.

Ahhh, ok great. So the end result is that we'll get the consistent checks *and* we'll only be doing them when it matters. Great!

Quoted 1 line

It seems a much easier to follow series.

Yeah definitely, I see that now.

	--Mark

--
Mark Fasheh