| Submitter | Thomas Graf |
|---|---|
| Date | Dec. 6, 2012, 7:25 p.m. |
| Message ID | <93b74d9f3e36e0009e8ca77192f417bc9f3e8dea.1354821623.git.tgraf@suug.ch> |
| Download | mbox | patch |
| Permalink | /patch/204305/ |
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
Comments
On 12/06/2012 02:25 PM, Thomas Graf wrote: > peer.transport_addr_list is currently only protected by sk_sock > which is inpractical to acquire for procfs dumping purposes. > > This patch adds RCU protection allowing for the procfs readers to > enter RCU read-side critical sections. > > Modification of the list continues to be serialized via sk_lock. > > V2: Use list_del_rcu() in sctp_association_free() to be safe > Skip transports marked dead when dumping for procfs > > Cc: Vlad Yasevich <vyasevich@gmail.com> > Cc: Neil Horman <nhorman@tuxdriver.com> > Signed-off-by: Thomas Graf <tgraf@suug.ch> Acked-by: Vlad Yasevich <vyasevich@gmail.com> -vlad > --- > include/net/sctp/structs.h | 2 ++ > net/sctp/associola.c | 6 +++--- > net/sctp/proc.c | 14 ++++++++++++-- > net/sctp/transport.c | 18 +++++++++++++----- > 4 files changed, 30 insertions(+), 10 deletions(-) > > diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h > index 64158aa..5d6987b 100644 > --- a/include/net/sctp/structs.h > +++ b/include/net/sctp/structs.h > @@ -948,6 +948,8 @@ struct sctp_transport { > > /* 64-bit random number sent with heartbeat. */ > __u64 hb_nonce; > + > + struct rcu_head rcu; > }; > > struct sctp_transport *sctp_transport_new(struct net *, const union sctp_addr *, > diff --git a/net/sctp/associola.c b/net/sctp/associola.c > index b1ef3bc..81b1236 100644 > --- a/net/sctp/associola.c > +++ b/net/sctp/associola.c > @@ -445,7 +445,7 @@ void sctp_association_free(struct sctp_association *asoc) > /* Release the transport structures. */ > list_for_each_safe(pos, temp, &asoc->peer.transport_addr_list) { > transport = list_entry(pos, struct sctp_transport, transports); > - list_del(pos); > + list_del_rcu(pos); > sctp_transport_free(transport); > } > > @@ -565,7 +565,7 @@ void sctp_assoc_rm_peer(struct sctp_association *asoc, > sctp_assoc_update_retran_path(asoc); > > /* Remove this peer from the list. */ > - list_del(&peer->transports); > + list_del_rcu(&peer->transports); > > /* Get the first transport of asoc. */ > pos = asoc->peer.transport_addr_list.next; > @@ -765,7 +765,7 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc, > peer->state = peer_state; > > /* Attach the remote transport to our asoc. */ > - list_add_tail(&peer->transports, &asoc->peer.transport_addr_list); > + list_add_tail_rcu(&peer->transports, &asoc->peer.transport_addr_list); > asoc->peer.transport_count++; > > /* If we do not yet have a primary path, set one. */ > diff --git a/net/sctp/proc.c b/net/sctp/proc.c > index 06b05ee..8c19e97 100644 > --- a/net/sctp/proc.c > +++ b/net/sctp/proc.c > @@ -162,15 +162,20 @@ static void sctp_seq_dump_remote_addrs(struct seq_file *seq, struct sctp_associa > struct sctp_af *af; > > primary = &assoc->peer.primary_addr; > - list_for_each_entry(transport, &assoc->peer.transport_addr_list, > + rcu_read_lock(); > + list_for_each_entry_rcu(transport, &assoc->peer.transport_addr_list, > transports) { > addr = &transport->ipaddr; > + if (transport->dead) > + continue; > + > af = sctp_get_af_specific(addr->sa.sa_family); > if (af->cmp_addr(addr, primary)) { > seq_printf(seq, "*"); > } > af->seq_dump_addr(seq, addr); > } > + rcu_read_unlock(); > } > > static void * sctp_eps_seq_start(struct seq_file *seq, loff_t *pos) > @@ -441,12 +446,16 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) > head = &sctp_assoc_hashtable[hash]; > sctp_local_bh_disable(); > read_lock(&head->lock); > + rcu_read_lock(); > sctp_for_each_hentry(epb, node, &head->chain) { > if (!net_eq(sock_net(epb->sk), seq_file_net(seq))) > continue; > assoc = sctp_assoc(epb); > - list_for_each_entry(tsp, &assoc->peer.transport_addr_list, > + list_for_each_entry_rcu(tsp, &assoc->peer.transport_addr_list, > transports) { > + if (tsp->dead) > + continue; > + > /* > * The remote address (ADDR) > */ > @@ -492,6 +501,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) > } > } > > + rcu_read_unlock(); > read_unlock(&head->lock); > sctp_local_bh_enable(); > > diff --git a/net/sctp/transport.c b/net/sctp/transport.c > index 206cf52..1295aec 100644 > --- a/net/sctp/transport.c > +++ b/net/sctp/transport.c > @@ -163,13 +163,11 @@ void sctp_transport_free(struct sctp_transport *transport) > sctp_transport_put(transport); > } > > -/* Destroy the transport data structure. > - * Assumes there are no more users of this structure. > - */ > -static void sctp_transport_destroy(struct sctp_transport *transport) > +static void sctp_transport_destroy_rcu(struct rcu_head *head) > { > - SCTP_ASSERT(transport->dead, "Transport is not dead", return); > + struct sctp_transport *transport; > > + transport = container_of(head, struct sctp_transport, rcu); > if (transport->asoc) > sctp_association_put(transport->asoc); > > @@ -180,6 +178,16 @@ static void sctp_transport_destroy(struct sctp_transport *transport) > SCTP_DBG_OBJCNT_DEC(transport); > } > > +/* Destroy the transport data structure. > + * Assumes there are no more users of this structure. > + */ > +static void sctp_transport_destroy(struct sctp_transport *transport) > +{ > + SCTP_ASSERT(transport->dead, "Transport is not dead", return); > + > + call_rcu(&transport->rcu, sctp_transport_destroy_rcu); > +} > + > /* Start T3_rtx timer if it is not already running and update the heartbeat > * timer. This routine is called every time a DATA chunk is sent. > */ > -- 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 Thu, Dec 06, 2012 at 03:07:51PM -0500, Vlad Yasevich wrote: > On 12/06/2012 02:25 PM, Thomas Graf wrote: > >peer.transport_addr_list is currently only protected by sk_sock > >which is inpractical to acquire for procfs dumping purposes. > > > >This patch adds RCU protection allowing for the procfs readers to > >enter RCU read-side critical sections. > > > >Modification of the list continues to be serialized via sk_lock. > > > >V2: Use list_del_rcu() in sctp_association_free() to be safe > > Skip transports marked dead when dumping for procfs > > > >Cc: Vlad Yasevich <vyasevich@gmail.com> > >Cc: Neil Horman <nhorman@tuxdriver.com> > >Signed-off-by: Thomas Graf <tgraf@suug.ch> > > Acked-by: Vlad Yasevich <vyasevich@gmail.com> > > -vlad > Acked-by: Neil Horman <nhorman@tuxdriver.com> > >--- > > include/net/sctp/structs.h | 2 ++ > > net/sctp/associola.c | 6 +++--- > > net/sctp/proc.c | 14 ++++++++++++-- > > net/sctp/transport.c | 18 +++++++++++++----- > > 4 files changed, 30 insertions(+), 10 deletions(-) > > > >diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h > >index 64158aa..5d6987b 100644 > >--- a/include/net/sctp/structs.h > >+++ b/include/net/sctp/structs.h > >@@ -948,6 +948,8 @@ struct sctp_transport { > > > > /* 64-bit random number sent with heartbeat. */ > > __u64 hb_nonce; > >+ > >+ struct rcu_head rcu; > > }; > > > > struct sctp_transport *sctp_transport_new(struct net *, const union sctp_addr *, > >diff --git a/net/sctp/associola.c b/net/sctp/associola.c > >index b1ef3bc..81b1236 100644 > >--- a/net/sctp/associola.c > >+++ b/net/sctp/associola.c > >@@ -445,7 +445,7 @@ void sctp_association_free(struct sctp_association *asoc) > > /* Release the transport structures. */ > > list_for_each_safe(pos, temp, &asoc->peer.transport_addr_list) { > > transport = list_entry(pos, struct sctp_transport, transports); > >- list_del(pos); > >+ list_del_rcu(pos); > > sctp_transport_free(transport); > > } > > > >@@ -565,7 +565,7 @@ void sctp_assoc_rm_peer(struct sctp_association *asoc, > > sctp_assoc_update_retran_path(asoc); > > > > /* Remove this peer from the list. */ > >- list_del(&peer->transports); > >+ list_del_rcu(&peer->transports); > > > > /* Get the first transport of asoc. */ > > pos = asoc->peer.transport_addr_list.next; > >@@ -765,7 +765,7 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc, > > peer->state = peer_state; > > > > /* Attach the remote transport to our asoc. */ > >- list_add_tail(&peer->transports, &asoc->peer.transport_addr_list); > >+ list_add_tail_rcu(&peer->transports, &asoc->peer.transport_addr_list); > > asoc->peer.transport_count++; > > > > /* If we do not yet have a primary path, set one. */ > >diff --git a/net/sctp/proc.c b/net/sctp/proc.c > >index 06b05ee..8c19e97 100644 > >--- a/net/sctp/proc.c > >+++ b/net/sctp/proc.c > >@@ -162,15 +162,20 @@ static void sctp_seq_dump_remote_addrs(struct seq_file *seq, struct sctp_associa > > struct sctp_af *af; > > > > primary = &assoc->peer.primary_addr; > >- list_for_each_entry(transport, &assoc->peer.transport_addr_list, > >+ rcu_read_lock(); > >+ list_for_each_entry_rcu(transport, &assoc->peer.transport_addr_list, > > transports) { > > addr = &transport->ipaddr; > >+ if (transport->dead) > >+ continue; > >+ > > af = sctp_get_af_specific(addr->sa.sa_family); > > if (af->cmp_addr(addr, primary)) { > > seq_printf(seq, "*"); > > } > > af->seq_dump_addr(seq, addr); > > } > >+ rcu_read_unlock(); > > } > > > > static void * sctp_eps_seq_start(struct seq_file *seq, loff_t *pos) > >@@ -441,12 +446,16 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) > > head = &sctp_assoc_hashtable[hash]; > > sctp_local_bh_disable(); > > read_lock(&head->lock); > >+ rcu_read_lock(); > > sctp_for_each_hentry(epb, node, &head->chain) { > > if (!net_eq(sock_net(epb->sk), seq_file_net(seq))) > > continue; > > assoc = sctp_assoc(epb); > >- list_for_each_entry(tsp, &assoc->peer.transport_addr_list, > >+ list_for_each_entry_rcu(tsp, &assoc->peer.transport_addr_list, > > transports) { > >+ if (tsp->dead) > >+ continue; > >+ > > /* > > * The remote address (ADDR) > > */ > >@@ -492,6 +501,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) > > } > > } > > > >+ rcu_read_unlock(); > > read_unlock(&head->lock); > > sctp_local_bh_enable(); > > > >diff --git a/net/sctp/transport.c b/net/sctp/transport.c > >index 206cf52..1295aec 100644 > >--- a/net/sctp/transport.c > >+++ b/net/sctp/transport.c > >@@ -163,13 +163,11 @@ void sctp_transport_free(struct sctp_transport *transport) > > sctp_transport_put(transport); > > } > > > >-/* Destroy the transport data structure. > >- * Assumes there are no more users of this structure. > >- */ > >-static void sctp_transport_destroy(struct sctp_transport *transport) > >+static void sctp_transport_destroy_rcu(struct rcu_head *head) > > { > >- SCTP_ASSERT(transport->dead, "Transport is not dead", return); > >+ struct sctp_transport *transport; > > > >+ transport = container_of(head, struct sctp_transport, rcu); > > if (transport->asoc) > > sctp_association_put(transport->asoc); > > > >@@ -180,6 +178,16 @@ static void sctp_transport_destroy(struct sctp_transport *transport) > > SCTP_DBG_OBJCNT_DEC(transport); > > } > > > >+/* Destroy the transport data structure. > >+ * Assumes there are no more users of this structure. > >+ */ > >+static void sctp_transport_destroy(struct sctp_transport *transport) > >+{ > >+ SCTP_ASSERT(transport->dead, "Transport is not dead", return); > >+ > >+ call_rcu(&transport->rcu, sctp_transport_destroy_rcu); > >+} > >+ > > /* Start T3_rtx timer if it is not already running and update the heartbeat > > * timer. This routine is called every time a DATA chunk is sent. > > */ > > > > -- 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
Patch
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h index 64158aa..5d6987b 100644 --- a/include/net/sctp/structs.h +++ b/include/net/sctp/structs.h @@ -948,6 +948,8 @@ struct sctp_transport { /* 64-bit random number sent with heartbeat. */ __u64 hb_nonce; + + struct rcu_head rcu; }; struct sctp_transport *sctp_transport_new(struct net *, const union sctp_addr *, diff --git a/net/sctp/associola.c b/net/sctp/associola.c index b1ef3bc..81b1236 100644 --- a/net/sctp/associola.c +++ b/net/sctp/associola.c @@ -445,7 +445,7 @@ void sctp_association_free(struct sctp_association *asoc) /* Release the transport structures. */ list_for_each_safe(pos, temp, &asoc->peer.transport_addr_list) { transport = list_entry(pos, struct sctp_transport, transports); - list_del(pos); + list_del_rcu(pos); sctp_transport_free(transport); } @@ -565,7 +565,7 @@ void sctp_assoc_rm_peer(struct sctp_association *asoc, sctp_assoc_update_retran_path(asoc); /* Remove this peer from the list. */ - list_del(&peer->transports); + list_del_rcu(&peer->transports); /* Get the first transport of asoc. */ pos = asoc->peer.transport_addr_list.next; @@ -765,7 +765,7 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc, peer->state = peer_state; /* Attach the remote transport to our asoc. */ - list_add_tail(&peer->transports, &asoc->peer.transport_addr_list); + list_add_tail_rcu(&peer->transports, &asoc->peer.transport_addr_list); asoc->peer.transport_count++; /* If we do not yet have a primary path, set one. */ diff --git a/net/sctp/proc.c b/net/sctp/proc.c index 06b05ee..8c19e97 100644 --- a/net/sctp/proc.c +++ b/net/sctp/proc.c @@ -162,15 +162,20 @@ static void sctp_seq_dump_remote_addrs(struct seq_file *seq, struct sctp_associa struct sctp_af *af; primary = &assoc->peer.primary_addr; - list_for_each_entry(transport, &assoc->peer.transport_addr_list, + rcu_read_lock(); + list_for_each_entry_rcu(transport, &assoc->peer.transport_addr_list, transports) { addr = &transport->ipaddr; + if (transport->dead) + continue; + af = sctp_get_af_specific(addr->sa.sa_family); if (af->cmp_addr(addr, primary)) { seq_printf(seq, "*"); } af->seq_dump_addr(seq, addr); } + rcu_read_unlock(); } static void * sctp_eps_seq_start(struct seq_file *seq, loff_t *pos) @@ -441,12 +446,16 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) head = &sctp_assoc_hashtable[hash]; sctp_local_bh_disable(); read_lock(&head->lock); + rcu_read_lock(); sctp_for_each_hentry(epb, node, &head->chain) { if (!net_eq(sock_net(epb->sk), seq_file_net(seq))) continue; assoc = sctp_assoc(epb); - list_for_each_entry(tsp, &assoc->peer.transport_addr_list, + list_for_each_entry_rcu(tsp, &assoc->peer.transport_addr_list, transports) { + if (tsp->dead) + continue; + /* * The remote address (ADDR) */ @@ -492,6 +501,7 @@ static int sctp_remaddr_seq_show(struct seq_file *seq, void *v) } } + rcu_read_unlock(); read_unlock(&head->lock); sctp_local_bh_enable(); diff --git a/net/sctp/transport.c b/net/sctp/transport.c index 206cf52..1295aec 100644 --- a/net/sctp/transport.c +++ b/net/sctp/transport.c @@ -163,13 +163,11 @@ void sctp_transport_free(struct sctp_transport *transport) sctp_transport_put(transport); } -/* Destroy the transport data structure. - * Assumes there are no more users of this structure. - */ -static void sctp_transport_destroy(struct sctp_transport *transport) +static void sctp_transport_destroy_rcu(struct rcu_head *head) { - SCTP_ASSERT(transport->dead, "Transport is not dead", return); + struct sctp_transport *transport; + transport = container_of(head, struct sctp_transport, rcu); if (transport->asoc) sctp_association_put(transport->asoc); @@ -180,6 +178,16 @@ static void sctp_transport_destroy(struct sctp_transport *transport) SCTP_DBG_OBJCNT_DEC(transport); } +/* Destroy the transport data structure. + * Assumes there are no more users of this structure. + */ +static void sctp_transport_destroy(struct sctp_transport *transport) +{ + SCTP_ASSERT(transport->dead, "Transport is not dead", return); + + call_rcu(&transport->rcu, sctp_transport_destroy_rcu); +} + /* Start T3_rtx timer if it is not already running and update the heartbeat * timer. This routine is called every time a DATA chunk is sent. */
peer.transport_addr_list is currently only protected by sk_sock which is inpractical to acquire for procfs dumping purposes. This patch adds RCU protection allowing for the procfs readers to enter RCU read-side critical sections. Modification of the list continues to be serialized via sk_lock. V2: Use list_del_rcu() in sctp_association_free() to be safe Skip transports marked dead when dumping for procfs Cc: Vlad Yasevich <vyasevich@gmail.com> Cc: Neil Horman <nhorman@tuxdriver.com> Signed-off-by: Thomas Graf <tgraf@suug.ch> --- include/net/sctp/structs.h | 2 ++ net/sctp/associola.c | 6 +++--- net/sctp/proc.c | 14 ++++++++++++-- net/sctp/transport.c | 18 +++++++++++++----- 4 files changed, 30 insertions(+), 10 deletions(-)