Re: vmpage race and deadlock

[Masao Uebayashi <uebayasi%tombi.co.jp@localhost>]

On Sun, Nov 28, 2010 at 09:03:30AM +0000, Mindaugas Rasiukevicius wrote:
> Juergen Hannken-Illjes <hannken%eis.cs.tu-bs.de@localhost> wrote:
> > Some weeks ago I started my fs stress test (5 x fsstress+fsx+dbench)
> > on a log enabled ffs1 file system backed by md(4).
> > 
> > Usually within hours I get a deadlock where a thread is waiting on
> > "genput" but the page in question is neither BUSY nor WANTED.  I suppose
> > I tracked (*1) it down to three places, where we change page flags
> > without holding the object lock.  With this diff (*2) in place the test
> > runs for > 48 hours.
> 
> Cool!

Indeed.

(It's a shame I couldn't find this while working on genfs.)

> 
> > Using atomic ops here is not possible as flags is a 16bit value.
> > 
> 
> You cannot use them at one place; in such case all uses i.e. protocol
> would need to be converted to atomic ops.

Wrapping pg->flags manipulation with accessor macros would help
migration.

> 
> > @@ -637,8 +639,9 @@ loopdone:
> >                 npages << PAGE_SHIFT, 0, cred);
> >             UVMHIST_LOG(ubchist, "gop_alloc off 0x%x/0x%x -> %d",
> >                 startoffset, npages << PAGE_SHIFT, error,0);
> >             if (!error) {
> > +                   mutex_enter(&uobj->vmobjlock);
> >                     for (i = 0; i < npages; i++) {
> >                             struct vm_page *pg = pgs[i];
> >  
> >                             if (pg == NULL) {
> > @@ -647,8 +650,9 @@ loopdone:
> >                             pg->flags &= ~(PG_CLEAN|PG_RDONLY);
> >                             UVMHIST_LOG(ubchist, "mark dirty pg %p",
> >                                 pg,0,0,0);
> >                     }
> > +                   mutex_exit(&uobj->vmobjlock);
> >             }
> >     }
> >     if (!glocked) {
> >             genfs_node_unlock(vp);
> 
> I think this loop can actually be moved after mutex_enter(&uobj->vmobjlock)
> at line 660 (with condition check, of course).

We can abuse lower bits of struct vm_page * pointers, to remember
while slots are holes.