On Tue, Jan 19, 2016 at 9:08 PM, Marcelo Ricardo Leitner <marcelo.leitner@xxxxxxxxx> wrote: > Em 19-01-2016 17:55, Vlad Yasevich escreveu: >> >> On 01/19/2016 02:31 PM, Marcelo Ricardo Leitner wrote: >>> >>> Em 19-01-2016 16:37, Vlad Yasevich escreveu: >>>> >>>> On 01/19/2016 10:59 AM, Marcelo Ricardo Leitner wrote: >>>>> >>>>> Yes, not thrilled here either about connect-to-self. >>>>> >>>>> But there is a big difference on how both works. For rx we can just >>>>> look for wanted skbs >>>>> in rx queue, as they aren't going anywhere, but for tx I don't think we >>>>> can easily block >>>>> sctp_wfree() call because that may be happening on another CPU (or am I >>>>> mistaken here? >>>>> sctp still doesn't have RFS but even irqbalance could affect this >>>>> AFAICT) and more than >>>>> one skb may be in transit at a time. >>>> >>>> >>>> The way it's done now, we wouldn't have to block sctp_wfree. Chunks are >>>> released under >>>> lock when they are acked, so we are OK here. The tx completions will >>>> just put 1 byte back >>>> to the socket associated with the tx'ed skb, and that should still be ok >>>> as >>>> sctp_packet_release_owner will call sk_free(). >>> >>> >>> Please let me rephrase it. I'm actually worried about the asoc->base.sk >>> part of the story >>> and how it's fetched in sctp_wfree(). I think we can update that sk >>> pointer after >>> sock_wfree() has fetched it but not used it yet, possibly leading to >>> accounting it twice, >>> one during migration and one on sock_wfree. >>> In sock_wfree() it will update some sk stats like sk->sk_wmem_alloc, >>> among others. >> >> >> sctp_wfree() is only used on skbs that were created as sctp chunks to be >> transmitted. >> Right now, these skbs aren't actually submitted to the IP or to nic to be >> transmitted. >> They are queued at the association level (either in transports or in the >> outqueue). >> They are only freed during ACK processing. >> >> The ACK processing happens under a socket lock and thus asoc->base.sk can >> not move. >> >> The migration process also happens under a socket lock. As a result, >> during migration >> we are guaranteed the chunk queues remain consistent and that >> asoc->base.sk linkage >> remains consistent. In fact, if you look at the sctp_sock_migrate, we >> lock both >> sockets when we reassign the assoc->base.sk so we know both sockets are >> properly locked. >> >> So, I am not sure that what you are worried about can happen. Please feel >> free to >> double-check the above of course. > > > Ohh, right. That makes sense. I'll rework the patch. Thanks Vlad. Hi Marcelo, Any updates on this? I still see the leak. -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
