On Tue, Nov 08, 2022 at 04:47:23PM +0000, Martin Pieuchot wrote:
> On 08/11/22(Tue) 15:28, Klemens Nanni wrote:
> > After this mechanical move, I can unlock the individual SIOCG* in there.
>
> I'd suggest grabbing the KERNEL_LOCK() after NET_LOCK_SHARED().
> Otherwise you might spin for the first one then release it when going
> to sleep.
I can do that inside the first switch, but we must grab the net lock
before if_unit() which is called before grabbing the shared net lock.
I can shuffle this around if you really want, or I can simply move the
existing kernel lock further down and keep it in the same order just
like it is now already.
>
> > OK?
> >
> > Index: if.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/if.c,v
> > retrieving revision 1.667
> > diff -u -p -r1.667 if.c
> > --- if.c 8 Nov 2022 15:20:24 -0000 1.667
> > +++ if.c 8 Nov 2022 15:26:07 -0000
> > @@ -2426,33 +2426,43 @@ ifioctl_get(u_long cmd, caddr_t data)
> > size_t bytesdone;
> > const char *label;
> >
> > - KERNEL_LOCK();
> > -
> > switch(cmd) {
> > case SIOCGIFCONF:
> > + KERNEL_LOCK();
> > NET_LOCK_SHARED();
> > error = ifconf(data);
> > NET_UNLOCK_SHARED();
> > + KERNEL_UNLOCK();
> > return (error);
> > case SIOCIFGCLONERS:
> > + KERNEL_LOCK();
> > error = if_clone_list((struct if_clonereq *)data);
> > + KERNEL_UNLOCK();
> > return (error);
> > case SIOCGIFGMEMB:
> > + KERNEL_LOCK();
> > NET_LOCK_SHARED();
> > error = if_getgroupmembers(data);
> > NET_UNLOCK_SHARED();
> > + KERNEL_UNLOCK();
> > return (error);
> > case SIOCGIFGATTR:
> > + KERNEL_LOCK();
> > NET_LOCK_SHARED();
> > error = if_getgroupattribs(data);
> > NET_UNLOCK_SHARED();
> > + KERNEL_UNLOCK();
> > return (error);
> > case SIOCGIFGLIST:
> > + KERNEL_LOCK();
> > NET_LOCK_SHARED();
> > error = if_getgrouplist(data);
> > NET_UNLOCK_SHARED();
> > + KERNEL_UNLOCK();
> > return (error);
> > }
> > +
> > + KERNEL_LOCK();
> >
> > ifp = if_unit(ifr->ifr_name);
> > if (ifp == NULL) {
> >
>
