[Ocfs2-devel] [patch 1/8] ocfs2: alloc_dinode counts and group bitmap should be update simultaneously

[Younger Liu]

On 2014/3/31 6:44, Mark Fasheh wrote:
> On Wed, Mar 19, 2014 at 02:09:59PM -0700, Andrew Morton wrote:
>> From: Younger Liu <younger.liucn at gmail.com>
>> Subject: ocfs2: alloc_dinode counts and group bitmap should be update simultaneously
>>
>> Updating alloc_dinode counts in ocfs2_alloc_dinode_update_counts() and
>> setting group bitmap in ocfs2_alloc_dinode_update_bitmap() have to be done
>> simultaneously.  Two cases are as follow:
>>
>> (1) If ocfs2_alloc_dinode_update_counts() fails, there is no need to
>>     set group bitmap.  This case has been considered.
>>
>> (2) If ocfs2_alloc_dinode_update_bitmap() fails, alloc_dinode counts
>>     should be rolled back.  Otherwise, some clusters would never be used. 
>>     This case is not considered.
>>
>> So, we combine two functions, and ensure simultaneity.
> 
> By 'ocfs2_alloc_dinode_update_bitmap' above, you mean
> ocfs2_block_group_set_bits() correct? I am going under that assumption.
> 
> 
> Anyway, I believe you're trying to fix a rare case where we might leak some
> space in that our chain counts might be a bit high. Basically the call to
> journal_access() in ocfs2_block_group_set_bits() has to fail. To be honest,
> this has existed for a while and hasn't ever been an issue.
> 
> That said, we can fix it but I don't like that your approach changes core
> bitmap handling code. Can you please do this by writing a simple fallback
> function to set the correct values on the bitmap dinode? Have that called in
> case of error from ocfs2_block_group_set_bits(). It doesn't need to make
> journal calls because that has already been set up for it so this is
> literally a couple lines of code.
> 
> If you like, you can take the pattern of:
> 
> call ocfs2_alloc_dinode_update_counts()
> call ocfs2_block_group_set_bits()
> if (error)
>    call ocfs2_rollback_alloc_dinode_counts()
>    ...
> 
> And put that into a master function.
> 
Hi Mark,
  Thanks for your review.
  It is not a good idea to change core code. I will resend a patch in a
monent.
     Younger
> Thanks,
> 	--Mark
> 
> 
> --
> Mark Fasheh
> 
>