Hello,
On Thu, May 20, 2021 at 11:28:19AM +0200, Claudio Jeker wrote:
</snip>
>
> One way to reduce the problems with copyout(9) is to use uvm_vslock() to
> lock the pages. This is what sysctl does but it comes with its own set of
> issues.
>
using uvm_vslock() did not come to my mind, when I was thinking of
how to fix it.
> In general exporting large collections from the kernel needs to be
> rethought. The system should not grab a lock for a long time to
> serve a userland process.
>
> > > Diff below moves copyout() at line 1784 outside of protection of both
> > > locks.
> > > The approach I took is relatively straightforward:
> > >
> > > let DIOCGETSTATES to allocate hold_states array, which will keep
> > > references to states.
> > >
> > > grab locks and take references, keep those references in hold
> > > array.
> > >
> > > drop locks, export states and do copyout, while walking
> > > array of references.
> > >
> > > drop references, release hold_states array.
> > >
> > > does it make sense? If we agree that this approach makes sense
> >
> > In my opinion it does. The other approach would be to (ab)use the
> > NET_LOCK() to serialize updates, like bluhm@'s diff does. Both
> > approaches have pros and cons.
> >
>
> I really think adding more to the NET_LOCK() is a step in the wrong
> direction. It will just creap into everything and grow the size of the
> kernel lock. For me the cons outweight the pros.
>
that's also my understanding. I would like to remove all pf
configuration (a.k.a. pfioctl()) outside of NET_LOCK() scope.
I mean it still be fine for pf_test() if caller will hold NET_LOCK().
pf_test() caller must accept a fact that pf_test() may need to grab
pf's internal locks to do its housekeeping properly.
thanks and
regards
sashan
