Prev: Support for the pressure sensor BMP085 from Bosch Sensortec.
Next: RTC: Add JZ4740 RTC driver
From: Christoph Hellwig on 19 Jun 2010 06:50
On Fri, Jun 18, 2010 at 05:30:13PM -0700, Michael Rubin wrote:
> Adding the /sys/kernel/mm/writeback/writeback file. It contains data
> to help developers and applications gain visibility into writeback
> behaviour.
>
> # cat /sys/kernel/mm/writeback/writeback
> pages_dirtied: 3747
> pages_cleaned: 3618
> dirty_threshold: 816673
> bg_threshold: 408336

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.

> 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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Michael Rubin on 19 Jun 2010 13:50
Thanks for looking at this.

On Sat, Jun 19, 2010 at 3:44 AM, Christoph Hellwig <hch(a)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(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: Support for the pressure sensor BMP085 from Bosch Sensortec.
Next: RTC: Add JZ4740 RTC driver