Re: Re: Re: [PATCH] RDMA/siw: Fix IPv6 addr_list locking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



-----"Jason Gunthorpe" <jgg@xxxxxxxx> wrote: -----

>To: "Bernard Metzler" <BMT@xxxxxxxxxxxxxx>
>From: "Jason Gunthorpe" <jgg@xxxxxxxx>
>Date: 08/26/2019 05:14PM
>Cc: linux-rdma@xxxxxxxxxxxxxxx, bvanassche@xxxxxxx,
>dledford@xxxxxxxxxx
>Subject: [EXTERNAL] Re: Re: [PATCH] RDMA/siw: Fix IPv6 addr_list
>locking
>
>On Mon, Aug 26, 2019 at 03:12:20PM +0000, Bernard Metzler wrote:
>> 
>> >To: "Bernard Metzler" <bmt@xxxxxxxxxxxxxx>
>> >From: "Jason Gunthorpe" <jgg@xxxxxxxx>
>> >Date: 08/26/2019 04:25PM
>> >Cc: linux-rdma@xxxxxxxxxxxxxxx, bvanassche@xxxxxxx,
>> >dledford@xxxxxxxxxx
>> >Subject: [EXTERNAL] Re: [PATCH] RDMA/siw: Fix IPv6 addr_list
>locking
>> >
>> >On Mon, Aug 26, 2019 at 04:17:40PM +0200, Bernard Metzler wrote:
>> >> Walking the address list of an inet6_dev requires
>> >> appropriate locking. Since the called function
>> >> siw_listen_address() may sleep, we have to use
>> >> rtnl_lock() + rcu_read_lock_bh() instead of
>> >> read_lock_bh().
>> >
>> >What is the RCU for if you have RTNL?
>> >
>> 
>> Frankly, I looked around in net/ipv6 and found, if not
>> rwlocked, addr_list walking to be rcu protected, even
>> if rtnl_lock()'d (e.g. addrconf_verify_rtnl()).
>> 
>> You are saying this is useless and overdone, since all
>> changes to that list are rtnl_lock protected right?
>> I was not sure about that.
>
>I'm not sure, I thought it was the case that rtnl protected the
>address lists on write.
>
>> For the IPv4 case further up, we also take the rtnl_lock,
>> and RCU-deref the address pointer (via
>> in_dev_for_each_ifa_rtnl()).
>
>That uses rtnl_derference which calls into rcu_dereference_protected
>which is saying 'this RCU protected data is being read outside RCU
>because we are holding the write side lock'
>
>Which means it is locked by RTNL
>
OK, makes sense.

So this is probably really overdone.

Thanks,
Bernard.




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux