Subject: Re: ffs with UBC rewrite performance improvement
To: Isaku Yamahata <email@example.com>
From: Chuck Silvers <firstname.lastname@example.org>
Date: 05/27/2003 09:32:02
I'm also glad that someone is taking an interest in this, but we need to be
careful not to degrade the performance of extending writes in the process of
improving overwrites. your initial patch showed 18% lower throughput for
extending writes, I think we need to do better.
wiring pages only prevents them from being paged out by the pagedaemon,
it doesn't prevent them from being freed if (eg.) they are from a vnode
mapping and the file is truncated. to prevent the pages from being freed
you'd need to keep them locked (ie. PG_BUSY set). this might not cause
a problem in this context though, since VOP_WRITE will hold the vnode lock
exclusive, which will block truncation of the file being written.
you can only use pmap_kenter_pa() to create a writable mapping if you
previously ensure that there can be no other mappings at incompatible
cache aliases. this is usually done by removing all existing mappings
and keeping the page locked until after the writable kenter'd mapping is
removed. this can cause problems for wired user mappings, since their
pmap-level mappings will be removed when they shouldn't be. in particular,
it would cause problems for the wired mapping you added. :-)
it may be good enough in this case to use pmap_enter() instead.
overall, the idea of wiring the user pages to avoid deadlock while optimizing
overwrites seems ok, but it's more expensive in terms of cpu-time than we
need in the common case. for vnodes which have no mappings, it should be
safe to use the existing UBC_FAULTBUSY path, since we know that no one
(including the thread doing the VOP_WRITE()) can fault on pages for that
vnode and there is no possibility of bad cache aliases. right now we don't
track the number of mappings for a vnode, but that would be relatively easy
to add and it would use much less cpu. for overwrites of vnodes with mappings,
wiring the user pages may be the best solution.
your changes seem more complicated than they need to be. how do your
changed versions of the ubc_* interfaces behave differently from the
existing UBC_FAULTBUSY path?
On Fri, May 23, 2003 at 12:37:12AM +0900, Isaku Yamahata wrote:
> > because ufs_balloc_range_with_pages keeps pages busy,
> > it can deadlock if some of them are mapped to the userspace and
> > a page fault occurs on one of them in uiomove.
> I solved this deadlock by wiring down the usespace beforehand.
> I attach a new patch that is for NetBSD 1.6.1.
> Almost all part of this patch is same.
> Does this seem ok? or do I miss anything else?
> Isaku Yamahata <email@example.com>