On 28/05/20(Thu) 14:59, Vitaliy Makkoveev wrote:
> socket(2) layer is already protected by solock(). It grabs NET_LOCK()
> for inet{,6}(4) sockets, but all other sockets are still under
> KERNEL_LOCK().
>
> I guess solock is already placed everythere it's required. Also `struct
> file' is already mp-safe.
>
> Diff below introduces rwlock `unp_lock'. It's like NET_LOCK()'s
> `netlock' but for PF_UNIX sockets. This lock protects the whole PF_UNIX
> layer. Vnode layer accessed in "connect" "bind" and "desconnect" cases
> is still under KERNEL_LOCK().
>
> I'am experimenting with his diff since 19.04.2020 and my test systems,
> include Gnome workstaion are stable, without any issue.
Looks great, more tests are required :)
Your diff has many trailing spaces, could you remove them? Commonly
used editors or diff programs have a way to highlight them :)
One comment:
> Index: sys/kern/uipc_usrreq.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.142
> diff -u -p -r1.142 uipc_usrreq.c
> --- sys/kern/uipc_usrreq.c 16 Jul 2019 21:41:37 -0000 1.142
> +++ sys/kern/uipc_usrreq.c 1 May 2020 13:47:11 -0000
> @@ -409,12 +418,17 @@ unp_detach(struct unpcb *unp)
> {
> struct vnode *vp;
>
> + rw_assert_wrlock(&unp_lock);
> +
> LIST_REMOVE(unp, unp_link);
> if (unp->unp_vnode) {
> + /* this is an unlocked cleaning of `v_socket', but it's safe */
> unp->unp_vnode->v_socket = NULL;
> vp = unp->unp_vnode;
> unp->unp_vnode = NULL;
> + KERNEL_LOCK();
> vrele(vp);
> + KERNEL_UNLOCK();
Why is it safe? That's what the comment should explain :) If the code
doesn't take the lock that implies it is not required. Why it is not
required is not obvious.
