On Thu, 2016-01-21 at 17:37 -0200, Marcelo Ricardo Leitner wrote: > On Thu, Jan 21, 2016 at 11:27:36AM -0800, Eric Dumazet wrote: > > On Fri, 2016-01-22 at 01:49 +0800, Xin Long wrote: > > > Previously, before rhashtable, /proc assoc listing was done by > > > read-locking the entire hash entry and dumping all assocs at once, so we > > > were sure that the assoc wasn't freed because it wouldn't be possible to > > > remove it from the hash meanwhile. > > > > > > Now we use rhashtable to list transports, and dump entries one by one. > > > That is, now we have to check if the assoc is still a good one, as the > > > transport we got may be being freed. > > > > > > Signed-off-by: Xin Long <lucien....@gmail.com> > > > --- > > > net/sctp/proc.c | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/net/sctp/proc.c b/net/sctp/proc.c > > > index 684c5b3..c74a810 100644 > > > --- a/net/sctp/proc.c > > > +++ b/net/sctp/proc.c > > > @@ -380,6 +380,8 @@ static int sctp_assocs_seq_show(struct seq_file *seq, > > > void *v) > > > } > > > > > > transport = (struct sctp_transport *)v; > > > > What protects you from this structure already being freed ? > > rcu, rhashtable_walk_start() at sctp_assocs_seq_start() starts an > (implicit from this POV) rcu_read_lock() for us which is unlocked only > when the walking is terminated, thus covering this _show. > > > > + if (!sctp_transport_hold(transport)) > > > + return 0; > > > > If this is rcu, then you do not need to increment the refcount, and > > decrement it later. > > It's an implicit hold on sctp asoc. > > This code is using contents from asoc pointer, which is not proctected > by rcu. As transport has a hold on the asoc, it's enough to just hold > the transport and not the asoc too, as we had to do in the previous > patch.
Then it means fast path also need to do this sctp_transport_hold() ? If sctp_association_put() was called from sctp_transport_destroy_rcu() (ie after rcu grace period), you would not need to increment/decrement the transport refcount. Normally, RCU protection does not need to change the refcount, unless we need to keep an object alive after escaping the rcu section.
