Re: [patch] AF_UNIX: notify peer on close

From: Julian Anastasov (uli@linux.tu-varna.acad.bg)
Date: Sat Mar 04 2000 - 12:22:21 EST

        Hello,

On Sat, 4 Mar 2000 kuznet@ms2.inr.ac.ru wrote:

> Hello!
>
>
> Oops!
>
> > - sock_orphan() must be called after notification, i.e. don't
> > reset sk->sleep before notification
>
> No-no-no! I simply messed sk and skpair both there and in unix_shutdown
> (fixed recently). Leave sock_orphan on its place, it orphans sk, not skpair.

        May be sock_orphan(sk) must be after
wake_up_interruptible_all(&sk->protinfo.af_unix.peer_wait); ?

>
>
> > AF_UNIX rt events now work for me. Is this patch
> > correct ?
>
> Alas, not quite.
>
>
> - sock_orphan(sk);
>
> It was OK.
>
>
> - sk->state_change(skpair);
>
> Julian, I have already told you that sk->state_change cannot
> be replaced with data_ready, it is true for error_report() as well.
> Please, look at the function sock_def_wakeup() and compare it to another ones.
> See?

        I just found the difference. It is not the same as 2.2.
>
> Mama... I did even worse thing. Locking is broken! Bad, bad, bad.
> sock_wake_async() was called without callback_lock.
> And unix_shutdown is broken too...
>
> Seems, this combination is correct.
>
> skpair->state_change(skpair);
> read_lock(&skpair->callback_lock);
> sock_wake_async(skpair->socket,0,POLL_HUP);
> read_lock(&skpair->callback_lock);

        This is working in my tests:

--- linux/net/unix/af_unix.c.orig Sat Mar 4 10:08:02 2000
+++ linux/net/unix/af_unix.c Sat Mar 4 18:53:41 2000
@@ -356,8 +356,10 @@
                         if (!skb_queue_empty(&sk->receive_queue) || embrion)
                                 skpair->err = ECONNRESET;
                         unix_state_wunlock(skpair);
- sk->state_change(skpair);
- sock_wake_async(sk->socket,1,POLL_HUP);
+ skpair->state_change(skpair);
+ read_lock(&skpair->callback_lock);
+ sock_wake_async(skpair->socket,0,POLL_HUP);
+ read_lock(&skpair->callback_lock);
                 }
                 sock_put(skpair); /* It may now die */
                 unix_peer(sk) = NULL;

>
> Note, that POLL_HUP translates to POLLHUP|POLLERR in any case.
> (which looks very silly, we could write simply POLLHUP|POLLERR 8)8)
> I have no idea, why it is indirected via band_table).

        I think, nobody uses si_band. si_code is enough (POLL_xxx).

Regards 

--
Julian Anastasov <uli@linux.tu-varna.acad.bg>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

This archive was generated by hypermail 2b29 : Tue Mar 07 2000 - 21:00:17 EST