Thanks for looking at this.
On Sat, Jun 19, 2010 at 3:44 AM, Christoph Hellwig <hch@xxxxxx> 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-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
