Re: [PATCH 2/4] writeback: allow for dirty metadata accounting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Josef,

as we talked off line, I think the idea of maintaining a byte counter
and rounding in balance_dirty_pages() is the best way to do this. And
Jan spotted all the actual bugs, so I only have a few nitpicks :)

On Tue, Sep 20, 2016 at 04:57:46PM -0400, Josef Bacik wrote:
> @@ -44,12 +44,13 @@ void show_mem(unsigned int filter)
>  {
>  	struct zone *zone;
>  
> -	pr_err("Active:%lu inactive:%lu dirty:%lu writeback:%lu unstable:%lu free:%lu\n slab:%lu mapped:%lu pagetables:%lu bounce:%lu pagecache:%lu swap:%lu\n",
> +	pr_err("Active:%lu inactive:%lu dirty:%lu metadata_dirty:%lu writeback:%lu unstable:%lu free:%lu\n slab:%lu mapped:%lu pagetables:%lu bounce:%lu pagecache:%lu swap:%lu\n",
>  	       (global_node_page_state(NR_ACTIVE_ANON) +
>  		global_node_page_state(NR_ACTIVE_FILE)),
>  	       (global_node_page_state(NR_INACTIVE_ANON) +
>  		global_node_page_state(NR_INACTIVE_FILE)),
>  	       global_node_page_state(NR_FILE_DIRTY),
> +	       global_node_page_state(NR_METADATA_DIRTY),
>  	       global_node_page_state(NR_WRITEBACK),

Print NR_METADATA_WRITEBACK here as well?

> @@ -51,6 +51,8 @@ static DEVICE_ATTR(cpumap,  S_IRUGO, node_read_cpumask, NULL);
>  static DEVICE_ATTR(cpulist, S_IRUGO, node_read_cpulist, NULL);
>  
>  #define K(x) ((x) << (PAGE_SHIFT - 10))
> +#define BtoK(x) ((x) >> 10)
> +
>  static ssize_t node_read_meminfo(struct device *dev,
>  			struct device_attribute *attr, char *buf)
>  {
> @@ -99,7 +101,9 @@ static ssize_t node_read_meminfo(struct device *dev,
>  #endif
>  	n += sprintf(buf + n,
>  		       "Node %d Dirty:          %8lu kB\n"
> +		       "Node %d MetadataDirty:	%8lu kB\n"
>  		       "Node %d Writeback:      %8lu kB\n"
> +		       "Node %d MetaWriteback:  %8lu kB\n"

Between the enums and stat printing, the naming is kind of all over
the place. How about NR_META_DIRTY_BYTES and NR_META_WRITEBACK_BYTES
as a separate group than the existing dirty & writeback stats?

 	n += sprintf(buf + n,
 		       "Node %d Dirty:          %8lu kB\n"
 		       "Node %d Writeback:      %8lu kB\n"
+		       "Node %d MetaDirty:	%8lu kB\n"
+		       "Node %d MetaWriteback:  %8lu kB\n"

>  		       "Node %d FilePages:      %8lu kB\n"
>  		       "Node %d Mapped:         %8lu kB\n"
>  		       "Node %d AnonPages:      %8lu kB\n"
> @@ -119,7 +123,9 @@ static ssize_t node_read_meminfo(struct device *dev,
>  #endif
>  			,
>  		       nid, K(node_page_state(pgdat, NR_FILE_DIRTY)),
> +		       nid, BtoK(node_page_state(pgdat, NR_METADATA_DIRTY_BYTES)),
>  		       nid, K(node_page_state(pgdat, NR_WRITEBACK)),
> +		       nid, BtoK(node_page_state(pgdat, NR_METADATA_WRITEBACK_BYTES)),
>  		       nid, K(node_page_state(pgdat, NR_FILE_PAGES)),
>  		       nid, K(node_page_state(pgdat, NR_FILE_MAPPED)),
>  		       nid, K(node_page_state(pgdat, NR_ANON_MAPPED)),
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 56c8fda..aafdb11 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1801,6 +1801,7 @@ static struct wb_writeback_work *get_next_work_item(struct bdi_writeback *wb)
>  	return work;
>  }
>  
> +#define BtoP(x) ((x) >> PAGE_SHIFT)

Might be more readable inline:

> @@ -1809,6 +1810,7 @@ static unsigned long get_nr_dirty_pages(void)
>  {
>  	return global_node_page_state(NR_FILE_DIRTY) +
>  		global_node_page_state(NR_UNSTABLE_NFS) +
> +		BtoP(global_node_page_state(NR_METADATA_DIRTY_BYTES)) +

		global_node_page_state(NR_META_DIRTY_BYTES) / PAGE_SIZE +

>  		get_nr_dirty_inodes();
>  }

> @@ -80,7 +81,9 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
>  		"SwapTotal:      %8lu kB\n"
>  		"SwapFree:       %8lu kB\n"
>  		"Dirty:          %8lu kB\n"
> +		"MetadataDirty:  %8lu kB\n"
>  		"Writeback:      %8lu kB\n"
> +		"MetaWriteback:  %8lu kB\n"

 		"Dirty:          %8lu kB\n"
 		"Writeback:      %8lu kB\n"
+		"MetaDirty:      %8lu kB\n"
+		"MetaWriteback:  %8lu kB\n"


>  		"AnonPages:      %8lu kB\n"
>  		"Mapped:         %8lu kB\n"
>  		"Shmem:          %8lu kB\n"
> @@ -139,7 +142,9 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
>  		K(i.totalswap),
>  		K(i.freeswap),
>  		K(global_node_page_state(NR_FILE_DIRTY)),
> +		BtoK(global_node_page_state(NR_METADATA_DIRTY_BYTES)),
>  		K(global_node_page_state(NR_WRITEBACK)),
> +		BtoK(global_node_page_state(NR_META_WRITEBACK_BYTES)),

		K(global_node_page_state(NR_META_WRITEBACK_BYTES / PAGE_SIZE)),

and drop BtoK?

> @@ -34,6 +34,8 @@ typedef int (congested_fn)(void *, int);
>  enum wb_stat_item {
>  	WB_RECLAIMABLE,
>  	WB_WRITEBACK,
> +	WB_METADATA_DIRTY_BYTES,
> +	WB_METADATA_WRITEBACK_BYTES,

	WB_META_DIRTY_BYTES,
	WB_META_WRITEBACK_BYTES,

etc.

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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]