Re: [RFC][PATCH 1/2] Add a super operation for writeback

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Daniel Phillips <daniel@phunq.net> writes:

> Hi Dave,
> On 06/02/2014 08:33 PM, Dave Chinner wrote:
>> On Mon, Jun 02, 2014 at 01:02:29PM -0700, Daniel Phillips wrote:
>>>
>>> Redirty_tail nearly works, but "if (!list_empty(&wb->b_dirty))" is
>>> not correct because the inode needs to end up on the dirty list
>>> whether it was already there or not.
>> redirty_tail() always moves the inode to the end of the dirty
>> list.

It doesn't move inode to end of the dirty if wb.b_dirty is empty
(I.e. it can move from wb.b_io to wb.b_dirty too).

BTW, this is called for like mark inode dirty process, not always
writeback path.

>>> This requirement is analogous to __mark_inode_dirty() and must
>>> tolerate similar races. At the microoptimization level, calling
>>> redirty_tail from inode_writeback_touch would be less efficient
>>> and more bulky. Another small issue is, redirty_tail does not
>>> always update the timestamp, which could trigger some bogus
>>> writeback events.
>> redirty_tail does not update the timestamp when it doesn't need to
>> change. If it needs to be changed because the current value would
>> violate the time ordering requirements of the list, it rewrites it.
>>
>> So there is essentially no functional difference between the new
>> function and redirty_tail....
>
> Hirofumi, would you care to comment?

It has difference.

Say, tail->dirtied_when == 1, inode->dirtied_when == 2, and now == 30
(tail->dirtied_when is expired at 31 with default config). In this case,
redirty_tail() doesn't update ->dirtied_when.

And if inode->dirtied_when is not updated to 30, expire time has
difference. I.e. 32 vs 60.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>