lkml.org 
[lkml]   [2010]   [Jun]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 1/3] writeback: Creating /sys/kernel/mm/writeback/writeback
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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2010-06-19 19:47    [W:1.051 / U:0.692 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site