I suspect that when someone did the search and replace when creating
balance_dirty_pages_ratelimited_nr they failed to read the code and
realize this code path was already effectively ratelimited. The result
is they made it excessively infrequent (every 1MB if ratelimit is 8) in
its calling balance_dirty_pages.
If anyone has ever seen this as an actual problem on a real system, I
would be curious to hear of it.
Hans
Hans Reiser <[email protected]> wrote:
>
> I suspect that when someone did the search and replace when creating
> balance_dirty_pages_ratelimited_nr they failed to read the code and
> realize this code path was already effectively ratelimited. The result
> is they made it excessively infrequent (every 1MB if ratelimit is 8) in
> its calling balance_dirty_pages.
?? There's been no change to balance_dirty_pages_ratelimited(). I merely
widened the interface a bit: introduced the new
balance_dirty_pages_ratelimited_nr() and did
static inline void
balance_dirty_pages_ratelimited(struct address_space *mapping)
{
balance_dirty_pages_ratelimited_nr(mapping, 1);
}
That being said, if reiserfs has `number of pages' in hand then yes, it
makes sense to migrate over to balance_dirty_pages_ratelimited_nr().
> If anyone has ever seen this as an actual problem on a real system, I
> would be curious to hear of it.
No, I wouldn't expect it to make much difference.
All that gunk is there just to avoid calling the expensive
get_writeback_state() once per set_page_dirty().
Inaccuracy here will introduce the possibility that we'll transiently dirty
more memory than dirty_ratio permits, but it'll only be a little bit (eight
times the amount of memory which is dirtied per balance_dirty_pages_ratelimited()
call).
That's a small amount of memory. But if you have 1000 filesystems mounted
and they all do the same thing at the same time, things could get a bit
sticky. Your patch will (greatly) reduce the possibility of even that
scenario.
Andrew Morton wrote:
>Hans Reiser <[email protected]> wrote:
>
>
>>I suspect that when someone did the search and replace when creating
>>balance_dirty_pages_ratelimited_nr they failed to read the code and
>>realize this code path was already effectively ratelimited. The result
>>is they made it excessively infrequent (every 1MB if ratelimit is 8) in
>>its calling balance_dirty_pages.
>>
>>
>
>?? There's been no change to balance_dirty_pages_ratelimited(). I merely
>widened the interface a bit: introduced the new
>balance_dirty_pages_ratelimited_nr() and did
>
>
>
So we were not originally using balance_dirty() in place of
balance_dirty_pages_ratelimited?
At any rate, the change is obviously better, I think we all agree on that.