Re: [patch 3/7] mm: make page writeback obey cpuset constraints

David Rientjes
2008-10-30T09:11:21Z

On Thu, 30 Oct 2008, Peter Zijlstra wrote:

> On Tue, 2008-10-28 at 12:18 -0700, David Rientjes wrote:
> > On Tue, 28 Oct 2008, Peter Zijlstra wrote:
> >
> > > > + is_subset = cpuset_populate_dirty_limits(dl, &dirtyable_memory,
> > > > + &nr_mapped, nodes);
> > > > + if (!is_subset) {
> > > > + dl->nr_dirty = global_page_state(NR_FILE_DIRTY);
> > > > + dl->nr_unstable = global_page_state(NR_UNSTABLE_NFS);
> > > > + dl->nr_writeback = global_page_state(NR_WRITEBACK);
> > > > + dirtyable_memory = determine_dirtyable_memory();
> > > > + nr_mapped = global_page_state(NR_FILE_MAPPED) +
> > > > + global_page_state(NR_ANON_PAGES);
> > > > + } else
> > > > + dirtyable_memory -= highmem_dirtyable_memory(nodes,
> > > > + dirtyable_memory);
> > >
> > > Why not fold that all into cpuset_populate_dirty_limits() ?
> > >
> >
> > cpuset_populate_dirty_limits() is a no-op on !CONFIG_CPUSETS kernels.
>
> Right, humm. Maybe introduce a populate_dirty_limits() and differentiate
> that between CONFIG_CPUSETS and not, and make it do everything.
>
> That would get rid of this fudge I think, no?
>

I agree it would look much cleaner and I actually did that originally, but
it requires adding #ifdef's for CONFIG_CPUSETS to mm/page-writeback.c
since highmem_dirtyable_memory() is static.

There's actually nothing cpuset-specific about
cpuset_populate_dirty_limits() and it could be used for !CONFIG_CPUSETS
kernels, but there's a performance benefit of using the global ZVC values
for the page stats as opposed to iterating over each node and adding their
respective values. So is_subset is only non-zero when the passed nodemask
excludes system nodes (the cpuset case), and we must iterate the node ZVC
values.

So perhaps the solution is to introduce populate_nodemask_dirty_limits()
and populate_global_dirty_limits() both in mm/page-writeback.c?