Is this a bug?

Is this a bug?

[Ding Dinghua]

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

Re: Is this a bug?

[Amir Goldstein]

On Sat, Apr 2, 2011 at 1:05 AM, Ding Dinghua <dingdinghua85@gmail.com> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Is this a bug?

[Ding Dinghua]

2011/4/3 Amir Goldstein <amir73il@gmail.com>:
> On Sat, Apr 2, 2011 at 1:05 AM, Ding Dinghua <dingdinghua85@gmail.com> 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 majordomo@vger.kernel.org
>> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Is this a bug?

[Yongqiang Yang]

On Sun, Apr 3, 2011 at 5:24 PM, Ding Dinghua <dingdinghua85@gmail.com> wrote:
> 2011/4/3 Amir Goldstein <amir73il@gmail.com>:
>> On Sat, Apr 2, 2011 at 1:05 AM, Ding Dinghua <dingdinghua85@gmail.com> 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 majordomo@vger.kernel.org
>>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Best Wishes
Yongqiang Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Is this a bug?

[Amir Goldstein]

On Sun, Apr 3, 2011 at 7:51 AM, Yongqiang Yang <xiaoqiangnk@gmail.com> wrote:
> On Sun, Apr 3, 2011 at 5:24 PM, Ding Dinghua <dingdinghua85@gmail.com> wrote:
>> 2011/4/3 Amir Goldstein <amir73il@gmail.com>:
>>> On Sat, Apr 2, 2011 at 1:05 AM, Ding Dinghua <dingdinghua85@gmail.com> 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 majordomo@vger.kernel.org
>>>> 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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>
>
> --
> Best Wishes
> Yongqiang Yang
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html