| Message ID | BANLkTi=cDwLFSqaBt5szaFOaWmxYzkya9g@mail.gmail.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
Le mercredi 18 mai 2011 à 15:11 +0200, Jacek Luczak a écrit : > diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c > index faf71d1..6150ac5 100644 > --- a/net/sctp/bind_addr.c > +++ b/net/sctp/bind_addr.c > @@ -140,14 +140,12 @@ void sctp_bind_addr_init(struct sctp_bind_addr > *bp, __u16 port) > /* Dispose of the address list. */ > static void sctp_bind_addr_clean(struct sctp_bind_addr *bp) > { > - struct sctp_sockaddr_entry *addr; > - struct list_head *pos, *temp; > + struct sctp_sockaddr_entry *addr, *temp; > > /* Empty the bind address list. */ > - list_for_each_safe(pos, temp, &bp->address_list) { > - addr = list_entry(pos, struct sctp_sockaddr_entry, list); > - list_del(pos); > - kfree(addr); > + list_for_each_entry_safe(addr, temp, &bp->address_list, list) { > + list_del_rcu(&addr->list); > + call_rcu(&addr->rcu, sctp_local_addr_free); > SCTP_DBG_OBJCNT_DEC(addr); > } > } > > Does it now look good? > It seems fine yes ! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/18/2011 09:11 AM, Jacek Luczak wrote: > 2011/5/18 Eric Dumazet <eric.dumazet@gmail.com>: >> Le mercredi 18 mai 2011 à 14:47 +0200, Jacek Luczak a écrit : >> >>> OK then, at the end what Eric suggested is IMO valid: >>> >>> diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c >>> index faf71d1..0025d90 100644 >>> --- a/net/sctp/bind_addr.c >>> +++ b/net/sctp/bind_addr.c >>> @@ -144,10 +144,9 @@ static void sctp_bind_addr_clean(struct sctp_bind_addr *bp) >>> struct list_head *pos, *temp; >>> >>> /* Empty the bind address list. */ >>> - list_for_each_safe(pos, temp, &bp->address_list) { >>> - addr = list_entry(pos, struct sctp_sockaddr_entry, list); >>> - list_del(pos); >>> - kfree(addr); >>> + list_for_each_entry(pos, &bp->address_list, list) { >> >> a 'safe' version is needed here, since we remove items in iterator. > > Yep. > > diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c > index faf71d1..6150ac5 100644 > --- a/net/sctp/bind_addr.c > +++ b/net/sctp/bind_addr.c > @@ -140,14 +140,12 @@ void sctp_bind_addr_init(struct sctp_bind_addr > *bp, __u16 port) > /* Dispose of the address list. */ > static void sctp_bind_addr_clean(struct sctp_bind_addr *bp) > { > - struct sctp_sockaddr_entry *addr; > - struct list_head *pos, *temp; > + struct sctp_sockaddr_entry *addr, *temp; > > /* Empty the bind address list. */ > - list_for_each_safe(pos, temp, &bp->address_list) { > - addr = list_entry(pos, struct sctp_sockaddr_entry, list); > - list_del(pos); > - kfree(addr); > + list_for_each_entry_safe(addr, temp, &bp->address_list, list) { > + list_del_rcu(&addr->list); > + call_rcu(&addr->rcu, sctp_local_addr_free); > SCTP_DBG_OBJCNT_DEC(addr); > } > } > > Does it now look good? Yes. It should the fix the race. -vlad > > -Jacek > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2011/5/18 Vladislav Yasevich <vladislav.yasevich@hp.com>: > On 05/18/2011 09:11 AM, Jacek Luczak wrote: >> 2011/5/18 Eric Dumazet <eric.dumazet@gmail.com>: >>> Le mercredi 18 mai 2011 à 14:47 +0200, Jacek Luczak a écrit : >>> >>>> OK then, at the end what Eric suggested is IMO valid: >>>> >>>> diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c >>>> index faf71d1..0025d90 100644 >>>> --- a/net/sctp/bind_addr.c >>>> +++ b/net/sctp/bind_addr.c >>>> @@ -144,10 +144,9 @@ static void sctp_bind_addr_clean(struct sctp_bind_addr *bp) >>>> struct list_head *pos, *temp; >>>> >>>> /* Empty the bind address list. */ >>>> - list_for_each_safe(pos, temp, &bp->address_list) { >>>> - addr = list_entry(pos, struct sctp_sockaddr_entry, list); >>>> - list_del(pos); >>>> - kfree(addr); >>>> + list_for_each_entry(pos, &bp->address_list, list) { >>> >>> a 'safe' version is needed here, since we remove items in iterator. >> >> Yep. >> >> diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c >> index faf71d1..6150ac5 100644 >> --- a/net/sctp/bind_addr.c >> +++ b/net/sctp/bind_addr.c >> @@ -140,14 +140,12 @@ void sctp_bind_addr_init(struct sctp_bind_addr >> *bp, __u16 port) >> /* Dispose of the address list. */ >> static void sctp_bind_addr_clean(struct sctp_bind_addr *bp) >> { >> - struct sctp_sockaddr_entry *addr; >> - struct list_head *pos, *temp; >> + struct sctp_sockaddr_entry *addr, *temp; >> >> /* Empty the bind address list. */ >> - list_for_each_safe(pos, temp, &bp->address_list) { >> - addr = list_entry(pos, struct sctp_sockaddr_entry, list); >> - list_del(pos); >> - kfree(addr); >> + list_for_each_entry_safe(addr, temp, &bp->address_list, list) { >> + list_del_rcu(&addr->list); >> + call_rcu(&addr->rcu, sctp_local_addr_free); >> SCTP_DBG_OBJCNT_DEC(addr); >> } >> } >> >> Does it now look good? > > Yes. It should the fix the race. > Thanks guys then for your guidance. I will repost final patch. -Jacek -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c index faf71d1..6150ac5 100644 --- a/net/sctp/bind_addr.c +++ b/net/sctp/bind_addr.c @@ -140,14 +140,12 @@ void sctp_bind_addr_init(struct sctp_bind_addr *bp, __u16 port) /* Dispose of the address list. */ static void sctp_bind_addr_clean(struct sctp_bind_addr *bp) { - struct sctp_sockaddr_entry *addr; - struct list_head *pos, *temp; + struct sctp_sockaddr_entry *addr, *temp; /* Empty the bind address list. */ - list_for_each_safe(pos, temp, &bp->address_list) { - addr = list_entry(pos, struct sctp_sockaddr_entry, list); - list_del(pos); - kfree(addr); + list_for_each_entry_safe(addr, temp, &bp->address_list, list) { + list_del_rcu(&addr->list); + call_rcu(&addr->rcu, sctp_local_addr_free); SCTP_DBG_OBJCNT_DEC(addr); }