[Fwd: Re: [PATCH] reiserfs: use balance_dirty_pages_ratelimited_nr in reiserfs_file_write]

[Fwd: Re: [PATCH] reiserfs: use balance_dirty_pages_ratelimited_nr in reiserfs_file_write]

[Hans Reiser]

[-- Attachment #1: Type: text/plain, Size: 423 bytes --]

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

[-- Attachment #2: Re: [PATCH] reiserfs: use balance_dirty_pages_ratelimited_nr in reiserfs_file_write --]
[-- Type: message/rfc822, Size: 2680 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 233 bytes --]

Hans, 

The patch works, however its effect is not visible.
Would you please forward it?

On Tuesday 28 February 2006 22:16, E.Gryaznova wrote:
> Zam said me about this patch, it is in my todo list for tomorrow.
>
> Thanks,
> Lena
>

[-- Attachment #2.1.2: reiserfs-reiserfs_file_write-use-balance_dirty_pages_ratelimited_nr.diff~ --]
[-- Type: application/x-trash, Size: 587 bytes --]

Re: [Fwd: Re: [PATCH] reiserfs: use balance_dirty_pages_ratelimited_nr in reiserfs_file_write]

[Andrew Morton]

Hans Reiser <reiser@namesys.com> 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.

Re: [Fwd: Re: [PATCH] reiserfs: use balance_dirty_pages_ratelimited_nr in reiserfs_file_write]

[Hans Reiser]

Andrew Morton wrote:

>Hans Reiser <reiser@namesys.com> 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.