VONWORKLST problems with cached ubc mappings

I am investigating some observed problems on msdosfs in
sources from
about 2006-04-04.  This shows up as either writes not
getting flushed
to disk, or else the debug assertion "(vp->v_flag
& VONWORKLST)"
failing in genfs_putpages.

I've done some investigation and I think the following his
happening.

A vnode gets written with msdofs_write, this faults pages in
to
a ubc_alloc mapping and the mapping gets cached.  The fault
marks
the vnode VONWORKLST.  When these pages are eventually
cleaned, the
VONWORKLST flag gets appropriately cleared.

Later, a second msdosfs_write to the same pages calls
ubc_alloc
and gets the previously cached mapping.  Since it finds the
cached mapping and UBC_FAULTBUSY is not used, there is no
getpages call, and the pages are dirtied without the vnode
being tagged with VONWORKLST.

Now the vnode has dirty pages without being marked
VONWORKLST.
If it does eventually get flushed, this results in the DEBUG
panic.

I suspect that this problem still exists, and it also might
occur on
other filesystems when pages are rewritten.  Has anyone else
seen
this?

Darrin

> A vnode gets written with msdofs_write, this faults
pages in to
> a ubc_alloc mapping and the mapping gets cached.  The
fault marks
> the vnode VONWORKLST.  When these pages are eventually
cleaned, the
> VONWORKLST flag gets appropriately cleared.
> 
> Later, a second msdosfs_write to the same pages calls
ubc_alloc
> and gets the previously cached mapping.  Since it finds
the
> cached mapping and UBC_FAULTBUSY is not used, there is
no
> getpages call, and the pages are dirtied without the
vnode
> being tagged with VONWORKLST.
> 
> Now the vnode has dirty pages without being marked
VONWORKLST.
> If it does eventually get flushed, this results in the
DEBUG panic.

genfs_putpages makes pages read-only by calling
pmap_page_protect
so that further attempts of making pages dirty, including
ubc, will
involve getpages.  (at least it was my intention.)

YAMAMOTO Takashi

yamtmwd.biglobe.ne.jp (YAMAMOTO Takashi) writes:

> > A vnode gets written with msdofs_write, this
faults pages in to
> > a ubc_alloc mapping and the mapping gets cached. 
The fault marks
> > the vnode VONWORKLST.  When these pages are
eventually cleaned, the
> > VONWORKLST flag gets appropriately cleared.
> > 
> > Later, a second msdosfs_write to the same pages
calls ubc_alloc
> > and gets the previously cached mapping.  Since it
finds the
> > cached mapping and UBC_FAULTBUSY is not used,
there is no
> > getpages call, and the pages are dirtied without
the vnode
> > being tagged with VONWORKLST.
> > 
> > Now the vnode has dirty pages without being marked
VONWORKLST.
> > If it does eventually get flushed, this results in
the DEBUG panic.
> 
> genfs_putpages makes pages read-only by calling
pmap_page_protect
> so that further attempts of making pages dirty,
including ubc, will
> involve getpages.  (at least it was my intention.)

> 
> YAMAMOTO Takashi

Ah, thanks for pointing this out.  With this in mind, I've
tested even
more and determined that the vnode never gets on the
VONWORKLST or has
a VM_PROT_WRITE getpages call at all.  I'm now thinking that
the page
gets mapped writable for the purposes of reading in its
original data,
and never gets write protected.  Since its never gets
VONWORKLST at
all, subsequent putpages calls don't write protect it
(cleanall == 0)

I will continue to investigate how the pages are getting
mapped
writable and will send more information tomorrow.

Thanks,
Darrin

> Ah, thanks for pointing this out.  With this in mind,
I've tested even
> more and determined that the vnode never gets on the
VONWORKLST or has
> a VM_PROT_WRITE getpages call at all.  I'm now thinking
that the page
> gets mapped writable for the purposes of reading in its
original data,
> and never gets write protected.  Since its never gets
VONWORKLST at
> all, subsequent putpages calls don't write protect it
(cleanall == 0)

UVM_OBJ_NEEDS_WRITEFAULT is for it.
ah, ubc_fault seems missing the check.  it's a bug.
the attached patch should fix it. (not tested)

> I will continue to investigate how the pages are
getting mapped
> writable and will send more information tomorrow.

PR/33513 can be related as well.

YAMAMOTO Takashi
Index: uvm_bio.c
============================================================
=======
RCS file: /cvsroot/src/sys/uvm/uvm_bio.c,v
retrieving revision 1.48
diff -u -p -r1.48 uvm_bio.c
--- uvm_bio.c	3 Sep 2006 21:33:33 -0000	1.48
+++ uvm_bio.c	28 Sep 2006 11:56:17 -0000
 -42,6
+42,7  __KERNEL_RCSID(0, "$NetBSD$");
 #include <sys/systm.h>
 #include <sys/malloc.h>
 #include <sys/kernel.h>
+#include <sys/vnode.h>
 
 #include <uvm/uvm.h>
 
 -356,8
+370,9  again:
 		 * is marked as PG_RDONLY.
 		 */
 
-		rdonly = (access_type & VM_PROT_WRITE) == 0
&&
-		    (pg->flags & PG_RDONLY) != 0;
+		rdonly = ((access_type & VM_PROT_WRITE) == 0
&&
+		    (pg->flags & PG_RDONLY) != 0) ||
+		    UVM_OBJ_NEEDS_WRITEFAULT(uobj);
 		KASSERT((pg->flags & PG_RDONLY) == 0 ||
 		    (access_type & VM_PROT_WRITE) == 0 ||
 		    pg->offset < umap->writeoff ||

yamtmwd.biglobe.ne.jp (YAMAMOTO Takashi) writes:
> 
> UVM_OBJ_NEEDS_WRITEFAULT is for it.
> ah, ubc_fault seems missing the check.  it's a bug.
> the attached patch should fix it. (not tested)

This patch does address the problem I was seeing, and
appears
to be working ok.  I will let you know if I see anything
else unusual.

Thank you,
Darrin