2011-04-02 08:05:51

by Ding Dinghua

[permalink] [raw]
Subject: Is this a bug?

When truncate files and free blocks, the following codes make me puzzled:

void ext4_free_blocks(handle_t *handle, struct inode *inode,
struct buffer_head *bh, ext4_fsblk_t block,
unsigned long count, int flags)
{
if (flags & EXT4_FREE_BLOCKS_FORGET) {
struct buffer_head *tbh = bh;
int i;

BUG_ON(bh && (count > 1));

for (i = 0; i < count; i++) {
if (!bh)
tbh = sb_find_get_block(inode->i_sb,
block + i);
if (unlikely(!tbh))
continue;
ext4_forget(handle, flags & EXT4_FREE_BLOCKS_METADATA,
inode, tbh, block + i);
}
}
}

I notice that ext4_forget mainly do two things:
a) call jbd2_journa_forget to forget the modification of some buffer head
b) or deal with the revoke issue
however, if we are freeing data blocks && ext4_forget get into branch a),
tbh is not the buffer_head which journal took care of in ext4_write_begin,
so i'm puzzled with this.

Could anyone explain it to me? Thanks.

--
Ding Dinghua


2011-04-02 16:32:50

by Amir Goldstein

[permalink] [raw]
Subject: Re: Is this a bug?

On Sat, Apr 2, 2011 at 1:05 AM, Ding Dinghua <[email protected]> wrote:
> When truncate files and free blocks, the following codes make me puzzled:
>
> void ext4_free_blocks(handle_t *handle, struct inode *inode,
> ? ? ? ? ? ? ? ? ? ? ?struct buffer_head *bh, ext4_fsblk_t block,
> ? ? ? ? ? ? ? ? ? ? ?unsigned long count, int flags)
> {
> ? ? ? ?if (flags & EXT4_FREE_BLOCKS_FORGET) {
> ? ? ? ? ? ? ? ?struct buffer_head *tbh = bh;
> ? ? ? ? ? ? ? ?int i;
>
> ? ? ? ? ? ? ? ?BUG_ON(bh && (count > 1));
>
> ? ? ? ? ? ? ? ?for (i = 0; i < count; i++) {
> ? ? ? ? ? ? ? ? ? ? ? ?if (!bh)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tbh = sb_find_get_block(inode->i_sb,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?block + i);
> ? ? ? ? ? ? ? ? ? ? ? ?if (unlikely(!tbh))
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue;
> ? ? ? ? ? ? ? ? ? ? ? ?ext4_forget(handle, flags & EXT4_FREE_BLOCKS_METADATA,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?inode, tbh, block + i);
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
> }
>
> I notice that ext4_forget mainly do two things:
> ? ?a) ?call jbd2_journa_forget to forget the modification of some buffer head
> ? ?b) ?or deal with the revoke issue
> however, if we are freeing data blocks && ext4_forget get into branch a),

Simple. we don't pass the FORGET flag when freeing data blocks,
only when freeing metadata blocks, which may have been journalled
already.
I am not sure about the journal=data case through.

> tbh is not the buffer_head which journal took care of in ext4_write_begin,
> so i'm puzzled with this.
>
> Could anyone explain it to me? Thanks.
>
> --
> Ding Dinghua
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

2011-04-03 09:24:03

by Ding Dinghua

[permalink] [raw]
Subject: Re: Is this a bug?

2011/4/3 Amir Goldstein <[email protected]>:
> On Sat, Apr 2, 2011 at 1:05 AM, Ding Dinghua <[email protected]> wrote:
>> When truncate files and free blocks, the following codes make me puzzled:
>>
>> void ext4_free_blocks(handle_t *handle, struct inode *inode,
>> ? ? ? ? ? ? ? ? ? ? ?struct buffer_head *bh, ext4_fsblk_t block,
>> ? ? ? ? ? ? ? ? ? ? ?unsigned long count, int flags)
>> {
>> ? ? ? ?if (flags & EXT4_FREE_BLOCKS_FORGET) {
>> ? ? ? ? ? ? ? ?struct buffer_head *tbh = bh;
>> ? ? ? ? ? ? ? ?int i;
>>
>> ? ? ? ? ? ? ? ?BUG_ON(bh && (count > 1));
>>
>> ? ? ? ? ? ? ? ?for (i = 0; i < count; i++) {
>> ? ? ? ? ? ? ? ? ? ? ? ?if (!bh)
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tbh = sb_find_get_block(inode->i_sb,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?block + i);
>> ? ? ? ? ? ? ? ? ? ? ? ?if (unlikely(!tbh))
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue;
>> ? ? ? ? ? ? ? ? ? ? ? ?ext4_forget(handle, flags & EXT4_FREE_BLOCKS_METADATA,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?inode, tbh, block + i);
>> ? ? ? ? ? ? ? ?}
>> ? ? ? ?}
>> }
>>
>> I notice that ext4_forget mainly do two things:
>> ? ?a) ?call jbd2_journa_forget to forget the modification of some buffer head
>> ? ?b) ?or deal with the revoke issue
>> however, if we are freeing data blocks && ext4_forget get into branch a),
>
> Simple. we don't pass the FORGET flag when freeing data blocks,
> only when freeing metadata blocks, which may have been journalled
> already.
> I am not sure about the journal=data case through.
Thanks for reply. The reason for me to dip into this issue is journal=data mode.
>
>> tbh is not the buffer_head which journal took care of in ext4_write_begin,
>> so i'm puzzled with this.
>>
>> Could anyone explain it to me? Thanks.
>>
>> --
>> Ding Dinghua
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>>
>



--
Ding Dinghua

2011-04-03 14:51:40

by Yongqiang Yang

[permalink] [raw]
Subject: Re: Is this a bug?

On Sun, Apr 3, 2011 at 5:24 PM, Ding Dinghua <[email protected]> wrote:
> 2011/4/3 Amir Goldstein <[email protected]>:
>> On Sat, Apr 2, 2011 at 1:05 AM, Ding Dinghua <[email protected]> wrote:
>>> When truncate files and free blocks, the following codes make me puzzled:
>>>
>>> void ext4_free_blocks(handle_t *handle, struct inode *inode,
>>> ? ? ? ? ? ? ? ? ? ? ?struct buffer_head *bh, ext4_fsblk_t block,
>>> ? ? ? ? ? ? ? ? ? ? ?unsigned long count, int flags)
>>> {
>>> ? ? ? ?if (flags & EXT4_FREE_BLOCKS_FORGET) {
>>> ? ? ? ? ? ? ? ?struct buffer_head *tbh = bh;
>>> ? ? ? ? ? ? ? ?int i;
>>>
>>> ? ? ? ? ? ? ? ?BUG_ON(bh && (count > 1));
>>>
>>> ? ? ? ? ? ? ? ?for (i = 0; i < count; i++) {
>>> ? ? ? ? ? ? ? ? ? ? ? ?if (!bh)
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tbh = sb_find_get_block(inode->i_sb,
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?block + i);
>>> ? ? ? ? ? ? ? ? ? ? ? ?if (unlikely(!tbh))
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue;
>>> ? ? ? ? ? ? ? ? ? ? ? ?ext4_forget(handle, flags & EXT4_FREE_BLOCKS_METADATA,
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?inode, tbh, block + i);
>>> ? ? ? ? ? ? ? ?}
>>> ? ? ? ?}
>>> }
>>>
>>> I notice that ext4_forget mainly do two things:
>>> ? ?a) ?call jbd2_journa_forget to forget the modification of some buffer head
>>> ? ?b) ?or deal with the revoke issue
>>> however, if we are freeing data blocks && ext4_forget get into branch a),
>>
>> Simple. we don't pass the FORGET flag when freeing data blocks,
>> only when freeing metadata blocks, which may have been journalled
>> already.
>> I am not sure about the journal=data case through.
> Thanks for reply. The reason for me to dip into this issue is journal=data mode.
I think data processing in journal=data mode is similar to metadata
processing in joutnal=ordered mode.
>>
>>> tbh is not the buffer_head which journal took care of in ext4_write_begin,
>>> so i'm puzzled with this.
>>>
>>> Could anyone explain it to me? Thanks.
>>>
>>> --
>>> Ding Dinghua
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>>> the body of a message to [email protected]
>>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>>>
>>
>
>
>
> --
> Ding Dinghua
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>



--
Best Wishes
Yongqiang Yang

2011-04-03 15:44:35

by Amir Goldstein

[permalink] [raw]
Subject: Re: Is this a bug?

On Sun, Apr 3, 2011 at 7:51 AM, Yongqiang Yang <[email protected]> wrote:
> On Sun, Apr 3, 2011 at 5:24 PM, Ding Dinghua <[email protected]> wrote:
>> 2011/4/3 Amir Goldstein <[email protected]>:
>>> On Sat, Apr 2, 2011 at 1:05 AM, Ding Dinghua <[email protected]> wrote:
>>>> When truncate files and free blocks, the following codes make me puzzled:
>>>>
>>>> void ext4_free_blocks(handle_t *handle, struct inode *inode,
>>>> ? ? ? ? ? ? ? ? ? ? ?struct buffer_head *bh, ext4_fsblk_t block,
>>>> ? ? ? ? ? ? ? ? ? ? ?unsigned long count, int flags)
>>>> {
>>>> ? ? ? ?if (flags & EXT4_FREE_BLOCKS_FORGET) {
>>>> ? ? ? ? ? ? ? ?struct buffer_head *tbh = bh;
>>>> ? ? ? ? ? ? ? ?int i;
>>>>
>>>> ? ? ? ? ? ? ? ?BUG_ON(bh && (count > 1));
>>>>
>>>> ? ? ? ? ? ? ? ?for (i = 0; i < count; i++) {
>>>> ? ? ? ? ? ? ? ? ? ? ? ?if (!bh)
>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tbh = sb_find_get_block(inode->i_sb,
>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?block + i);
>>>> ? ? ? ? ? ? ? ? ? ? ? ?if (unlikely(!tbh))
>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?continue;
>>>> ? ? ? ? ? ? ? ? ? ? ? ?ext4_forget(handle, flags & EXT4_FREE_BLOCKS_METADATA,
>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?inode, tbh, block + i);
>>>> ? ? ? ? ? ? ? ?}
>>>> ? ? ? ?}
>>>> }
>>>>
>>>> I notice that ext4_forget mainly do two things:
>>>> ? ?a) ?call jbd2_journa_forget to forget the modification of some buffer head
>>>> ? ?b) ?or deal with the revoke issue
>>>> however, if we are freeing data blocks && ext4_forget get into branch a),
>>>
>>> Simple. we don't pass the FORGET flag when freeing data blocks,
>>> only when freeing metadata blocks, which may have been journalled
>>> already.
>>> I am not sure about the journal=data case through.
>> Thanks for reply. The reason for me to dip into this issue is journal=data mode.
> I think data processing in journal=data mode is similar to metadata
> processing in joutnal=ordered mode.

not exactly. Ding is right to be puzzled, because journal=data
journals inode page cache buffers and not block device buffers, like
metadata.
However, I think what happens is that prior to ext4_clear_blocks, which
invokes ext4_free_blocks with the FORGET flag, ext4_invalidatepage
will have been called already and forget about data buffers which were modified
in the current running transaction.
ext4_free_blocks will then handle revoke of data blocks which were modified in
previous transaction. for example, in orphan cleanup, ext4_free_blocks will be
will be called from ext4_truncate, but there will be no pages to invalidate.

this is my guess. I haven't studied this case thoroughly.

>>>
>>>> tbh is not the buffer_head which journal took care of in ext4_write_begin,
>>>> so i'm puzzled with this.
>>>>
>>>> Could anyone explain it to me? Thanks.
>>>>
>>>> --
>>>> Ding Dinghua
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>
>>
>>
>> --
>> Ding Dinghua
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to [email protected]
>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>>
>
>
>
> --
> Best Wishes
> Yongqiang Yang
>