Re: serious performance regression in .41

[christos%astron.com@localhost (Christos Zoulas)]

In article <69CE1CE4-6C54-4E17-802D-679A6A165F11%eis.cs.tu-bs.de@localhost>,
J. Hannken-Illjes <hannken%eis.cs.tu-bs.de@localhost> wrote:
>
>Ok, see me convinced.

Great :-)

>First, selector and context should be passed to vfs_vnode_iterator_init
>as they are invariant.  We may (ab)use v_mount and v_data to avoid
>allocating a state like we do with v_usecount now.

ok.

>This cannot work, the selector would run on reclaiming and
>marker nodes.  Change the while() loop to
>
>       while (ISSET(vp->v_iflag, VI_MARKER) ||
>           ISSET(vp->v_iflag, VI_XLOCK) ||
>           (f && !(*f)(cl, vp))) {
>
>No need to test for VI_CHANGING as the selector runs with
>v_interlock held.

changed.

>
>> +
>>              while ((vp->v_iflag & VI_MARKER) != 0) {
>>                      mutex_exit(vp->v_interlock);
>>                      vp = TAILQ_NEXT(vp, v_mntvnodes);
>>                      if (vp == NULL) {
>>                              mutex_exit(&mntvnode_lock);
>> -                            *vpp = NULL;
>> -                            return false;
>> +                            return NULL;
>>                      }
>>                      mutex_enter(vp->v_interlock);
>>              }

The above  is now removed.

><snip>
>> Index: sys/nfs/nfs_subs.c
>> ===================================================================
>> RCS file: /cvsroot/src/sys/nfs/nfs_subs.c,v
><snip>
>> +    ctx.mp = mp;
>> +    while ((vp = vfs_vnode_iterator_next(marker, nfs_clearcommit_selector,
>> +        &ctx))) {
>>              vrele(vp);
>>      }
>
>Change to
>
>       vp = vfs_vnode_iterator_next(...);
>       KASSERT(vp == NULL);
>
>as the filter always returns false?

Yes, changed.

>For ffs accessing the inode is safe (see ffs_reclaim).  I didn't check
>the other uses though.

We should review all the selector calls. I don't think that calling
VOP_GETATTR is ok. If it is not, why doesn't it KASSSET?

>... and as always: Anita run, kernel version bump and man page update.

Committed, thanks!

christos