Re: [PATCH 1/3] writeback: Creating /sys/kernel/mm/writeback/writeback

[Michael Rubin <mrubin@google.com>]

Thanks for looking at this.

On Sat, Jun 19, 2010 at 3:44 AM, Christoph Hellwig <hch@lst.de> wrote:
> I'm fine with exposting this. but the interface is rather awkward.
> These kinds of multiple value per file interface require addition
> parsing and are a pain to extend.  Please do something like
>
> /proc/sys/vm/writeback/
>
>                        pages_dirtied
>                        pages_cleaned
>                        dirty_threshold
>                        background_threshold
>
> where you can just read the value from the file.

Cool. This is kind of funny. In the google tree I implemented this in
the same multi-file-one-value-in-file manner. The debate on one file
for all vs that style was heated. And I changed it before sending
upstream. I really don't care either way. So I will just change the
patch and move the values to that location

Do you mind explaining why something would go in /proc/ vs /sys? I
thought the idea was to not put things in /proc anymore.

>> diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
>> index c920164..84b0181 100644
>> --- a/fs/nilfs2/segment.c
>> +++ b/fs/nilfs2/segment.c
>> @@ -1598,8 +1598,10 @@ nilfs_copy_replace_page_buffers(struct page *page, struct list_head *out)
>>       } while (bh = bh->b_this_page, bh2 = bh2->b_this_page, bh != head);
>>       kunmap_atomic(kaddr, KM_USER0);
>>
>> -     if (!TestSetPageWriteback(clone_page))
>> +     if (!TestSetPageWriteback(clone_page)) {
>>               inc_zone_page_state(clone_page, NR_WRITEBACK);
>> +             inc_zone_page_state(clone_page, NR_PAGES_ENTERED_WRITEBACK);
>> +     }
>>       unlock_page(clone_page);
>
> I'm not very happy about having this opencoded in a filesystem.

I wasn't excited about this section either. What does opencoded mean?
Do you mean it should not be exposed to specific fs code?

mrubin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>