Re: [patch 01/52] kernel: add bl_list

Nick Piggin
2010-06-24T14:42:35Z

On Thu, Jun 24, 2010 at 08:04:22AM +0200, Eric Dumazet wrote:
> Le jeudi 24 juin 2010 à 13:02 +1000, npiggin@suse.de a écrit :
> > +static inline void hlist_bl_set_first(struct hlist_bl_head *h, struct hlist_bl_node *n)
> > +{
> > + h->first = (struct hlist_bl_node *)((unsigned long)n | ((unsigned long)h->first & 1UL));
>
> Hmm, shouldnt hlist_bl_set_first() be used only with bit lock held ?
>
> h->first = (struct hlist_bl_node *)((unsigned long)n | 1UL);
> > +}

I had it that way but changed it for some reason. Thinking about it
again though, you're right I'm sure (it could have been some other
bug in my code making me think I needed it).

Thanks.

> > +static inline void hlist_bl_set_first_rcu(struct hlist_bl_head *h, struct hlist_bl_node *n)
> > +{
> > + rcu_assign_pointer(h->first, (struct hlist_bl_node *)((unsigned long)n | ((unsigned long)h->first & 1UL)));
>
> Same question here.
>
> > +}
> > +
> > +static inline struct hlist_bl_node *hlist_bl_first_rcu(struct hlist_bl_head *h)
> > +{
> > + return (struct hlist_bl_node *)((unsigned long)rcu_dereference(h->first) & ~1UL);
> > +}
>
>
> Looks really nice Nick, maybe we should push this so that other
> subsystem can start using it.

Sure, if you have an interest in using it, it will be trivial to send
upstream. Should we merge it before any users appear? I don't know...