* [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
@ 2020-01-14 16:23 madhuparnabhowmik04
2020-01-14 16:57 ` Jason Gunthorpe
0 siblings, 1 reply; 14+ messages in thread
From: madhuparnabhowmik04 @ 2020-01-14 16:23 UTC (permalink / raw)
To: mike.marciniszyn, dennis.dalessandro, jgg, paulmck
Cc: joel, frextrite, linux-kernel-mentees, rcu, linux-rdma,
linux-kernel, Madhuparna Bhowmik
From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
list_for_each_entry_rcu has built-in RCU and lock checking.
Pass cond argument to list_for_each_entry_rcu.
Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
---
drivers/infiniband/hw/hfi1/verbs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index 089e201d7550..22f2d4fd2577 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -515,7 +515,7 @@ static inline void hfi1_handle_packet(struct hfi1_packet *packet,
opa_get_lid(packet->dlid, 9B));
if (!mcast)
goto drop;
- list_for_each_entry_rcu(p, &mcast->qp_list, list) {
+ list_for_each_entry_rcu(p, &mcast->qp_list, list, lockdep_is_held(&(ibp->rvp.lock))) {
packet->qp = p->qp;
if (hfi1_do_pkey_check(packet))
goto drop;
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
2020-01-14 16:23 [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking madhuparnabhowmik04
@ 2020-01-14 16:57 ` Jason Gunthorpe
2020-01-14 17:00 ` Dennis Dalessandro
0 siblings, 1 reply; 14+ messages in thread
From: Jason Gunthorpe @ 2020-01-14 16:57 UTC (permalink / raw)
To: madhuparnabhowmik04
Cc: mike.marciniszyn, dennis.dalessandro, paulmck, joel, frextrite,
linux-kernel-mentees, rcu, linux-rdma, linux-kernel
On Tue, Jan 14, 2020 at 09:53:45PM +0530, madhuparnabhowmik04@gmail.com wrote:
> From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
>
> list_for_each_entry_rcu has built-in RCU and lock checking.
> Pass cond argument to list_for_each_entry_rcu.
>
> Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
> drivers/infiniband/hw/hfi1/verbs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
> index 089e201d7550..22f2d4fd2577 100644
> +++ b/drivers/infiniband/hw/hfi1/verbs.c
> @@ -515,7 +515,7 @@ static inline void hfi1_handle_packet(struct hfi1_packet *packet,
> opa_get_lid(packet->dlid, 9B));
> if (!mcast)
> goto drop;
> - list_for_each_entry_rcu(p, &mcast->qp_list, list) {
> + list_for_each_entry_rcu(p, &mcast->qp_list, list, lockdep_is_held(&(ibp->rvp.lock))) {
Okay, this looks reasonable
Mike, Dennis, is this the right lock to test?
Jason
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
2020-01-14 16:57 ` Jason Gunthorpe
@ 2020-01-14 17:00 ` Dennis Dalessandro
2020-01-14 18:24 ` Dennis Dalessandro
0 siblings, 1 reply; 14+ messages in thread
From: Dennis Dalessandro @ 2020-01-14 17:00 UTC (permalink / raw)
To: Jason Gunthorpe, madhuparnabhowmik04
Cc: mike.marciniszyn, paulmck, joel, frextrite, linux-kernel-mentees,
rcu, linux-rdma, linux-kernel
On 1/14/2020 11:57 AM, Jason Gunthorpe wrote:
> On Tue, Jan 14, 2020 at 09:53:45PM +0530, madhuparnabhowmik04@gmail.com wrote:
>> From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
>>
>> list_for_each_entry_rcu has built-in RCU and lock checking.
>> Pass cond argument to list_for_each_entry_rcu.
>>
>> Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
>> drivers/infiniband/hw/hfi1/verbs.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
>> index 089e201d7550..22f2d4fd2577 100644
>> +++ b/drivers/infiniband/hw/hfi1/verbs.c
>> @@ -515,7 +515,7 @@ static inline void hfi1_handle_packet(struct hfi1_packet *packet,
>> opa_get_lid(packet->dlid, 9B));
>> if (!mcast)
>> goto drop;
>> - list_for_each_entry_rcu(p, &mcast->qp_list, list) {
>> + list_for_each_entry_rcu(p, &mcast->qp_list, list, lockdep_is_held(&(ibp->rvp.lock))) {
>
> Okay, this looks reasonable
>
> Mike, Dennis, is this the right lock to test?
>
I'm looking at that right now actually, I don't think this is correct.
Wanted to talk to Mike before I send a response though.
-Denny
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
2020-01-14 17:00 ` Dennis Dalessandro
@ 2020-01-14 18:24 ` Dennis Dalessandro
2020-01-14 19:17 ` Joel Fernandes
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Dennis Dalessandro @ 2020-01-14 18:24 UTC (permalink / raw)
To: Jason Gunthorpe, madhuparnabhowmik04
Cc: mike.marciniszyn, paulmck, joel, frextrite, linux-kernel-mentees,
rcu, linux-rdma, linux-kernel
On 1/14/2020 12:00 PM, Dennis Dalessandro wrote:
> On 1/14/2020 11:57 AM, Jason Gunthorpe wrote:
>> On Tue, Jan 14, 2020 at 09:53:45PM +0530,
>> madhuparnabhowmik04@gmail.com wrote:
>>> From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
>>>
>>> list_for_each_entry_rcu has built-in RCU and lock checking.
>>> Pass cond argument to list_for_each_entry_rcu.
>>>
>>> Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
>>> drivers/infiniband/hw/hfi1/verbs.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/infiniband/hw/hfi1/verbs.c
>>> b/drivers/infiniband/hw/hfi1/verbs.c
>>> index 089e201d7550..22f2d4fd2577 100644
>>> +++ b/drivers/infiniband/hw/hfi1/verbs.c
>>> @@ -515,7 +515,7 @@ static inline void hfi1_handle_packet(struct
>>> hfi1_packet *packet,
>>> opa_get_lid(packet->dlid, 9B));
>>> if (!mcast)
>>> goto drop;
>>> - list_for_each_entry_rcu(p, &mcast->qp_list, list) {
>>> + list_for_each_entry_rcu(p, &mcast->qp_list, list,
>>> lockdep_is_held(&(ibp->rvp.lock))) {
>>
>> Okay, this looks reasonable
>>
>> Mike, Dennis, is this the right lock to test?
>>
>
> I'm looking at that right now actually, I don't think this is correct.
> Wanted to talk to Mike before I send a response though.
>
> -Denny
That's definitely going to throw a ton of lock dep messages. It's not
really the right lock either. Instead what we probably need to do is
what we do in the non-multicast part of the code and take the
rcu_read_lock().
I'd say hold off on this and we'll fix it right. Same goes for the qib one.
The rdmavt one though looks to be OK. I'll give it a test.
-Denny
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
2020-01-14 18:24 ` Dennis Dalessandro
@ 2020-01-14 19:17 ` Joel Fernandes
2020-01-14 19:41 ` Jason Gunthorpe
[not found] ` <CAF65HP0RsW5FMRRf5Lia2=MTKex-KwO7-_NsCUef94YKBg+pfA@mail.gmail.com>
2 siblings, 0 replies; 14+ messages in thread
From: Joel Fernandes @ 2020-01-14 19:17 UTC (permalink / raw)
To: madhuparnabhowmik04
Cc: dennis.dalessandro, Jason Gunthorpe, mike.marciniszyn, paulmck,
frextrite, linux-kernel-mentees, rcu, linux-rdma, linux-kernel
On Wed, Jan 15, 2020 at 12:04:58AM +0530, madhuparnabhowmik04@gmail.com wrote:
> From: Dennis Dalessandro <dennis.dalessandro@intel.com>
>
> On 1/14/2020 12:00 PM, Dennis Dalessandro wrote:
> > On 1/14/2020 11:57 AM, Jason Gunthorpe wrote:
> > > On Tue, Jan 14, 2020 at 09:53:45PM +0530,
> > > madhuparnabhowmik04@gmail.com wrote:
> > > > From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
> > > >
> > > > list_for_each_entry_rcu has built-in RCU and lock checking.
> > > > Pass cond argument to list_for_each_entry_rcu.
> > > >
> > > > Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
> > > > drivers/infiniband/hw/hfi1/verbs.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/infiniband/hw/hfi1/verbs.c
> > > > b/drivers/infiniband/hw/hfi1/verbs.c
> > > > index 089e201d7550..22f2d4fd2577 100644
> > > > +++ b/drivers/infiniband/hw/hfi1/verbs.c
> > > > @@ -515,7 +515,7 @@ static inline void hfi1_handle_packet(struct
> > > > hfi1_packet *packet,
> > > > opa_get_lid(packet->dlid, 9B));
> > > > if (!mcast)
> > > > goto drop;
> > > > - list_for_each_entry_rcu(p, &mcast->qp_list, list) {
> > > > + list_for_each_entry_rcu(p, &mcast->qp_list, list,
> > > > lockdep_is_held(&(ibp->rvp.lock))) {
> > >
> > > Okay, this looks reasonable
> > >
> > > Mike, Dennis, is this the right lock to test?
> > >
> >
> > I'm looking at that right now actually, I don't think this is correct.
> > Wanted to talk to Mike before I send a response though.
> >
> > -Denny
>
> That's definitely going to throw a ton of lock dep messages. It's not really
> the right lock either. Instead what we probably need to do is what we do in
> the non-multicast part of the code and take the rcu_read_lock().
>
> I'd say hold off on this and we'll fix it right. Same goes for the qib one.
>
> Alright, thank you for reviewing.
>
> The rdmavt one though looks to be OK. I'll give it a test.
Madhuparna, there seems to be an issue with your mail client where it is not
quoting text correctly, either there is a '>' missing or there are too many.
Can you look into it and figure what's wrong with it?
thanks,
- Joel
>
> Thank you,
> Madhuparna
>
> -Denny
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
2020-01-14 18:24 ` Dennis Dalessandro
2020-01-14 19:17 ` Joel Fernandes
@ 2020-01-14 19:41 ` Jason Gunthorpe
2020-01-14 19:46 ` Dennis Dalessandro
[not found] ` <CAF65HP0RsW5FMRRf5Lia2=MTKex-KwO7-_NsCUef94YKBg+pfA@mail.gmail.com>
2 siblings, 1 reply; 14+ messages in thread
From: Jason Gunthorpe @ 2020-01-14 19:41 UTC (permalink / raw)
To: Dennis Dalessandro
Cc: madhuparnabhowmik04, mike.marciniszyn, paulmck, joel, frextrite,
linux-kernel-mentees, rcu, linux-rdma, linux-kernel
On Tue, Jan 14, 2020 at 01:24:00PM -0500, Dennis Dalessandro wrote:
> On 1/14/2020 12:00 PM, Dennis Dalessandro wrote:
> > On 1/14/2020 11:57 AM, Jason Gunthorpe wrote:
> > > On Tue, Jan 14, 2020 at 09:53:45PM +0530,
> > > madhuparnabhowmik04@gmail.com wrote:
> > > > From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
> > > >
> > > > list_for_each_entry_rcu has built-in RCU and lock checking.
> > > > Pass cond argument to list_for_each_entry_rcu.
> > > >
> > > > Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
> > > > drivers/infiniband/hw/hfi1/verbs.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/infiniband/hw/hfi1/verbs.c
> > > > b/drivers/infiniband/hw/hfi1/verbs.c
> > > > index 089e201d7550..22f2d4fd2577 100644
> > > > +++ b/drivers/infiniband/hw/hfi1/verbs.c
> > > > @@ -515,7 +515,7 @@ static inline void hfi1_handle_packet(struct
> > > > hfi1_packet *packet,
> > > > opa_get_lid(packet->dlid, 9B));
> > > > if (!mcast)
> > > > goto drop;
> > > > - list_for_each_entry_rcu(p, &mcast->qp_list, list) {
> > > > + list_for_each_entry_rcu(p, &mcast->qp_list, list,
> > > > lockdep_is_held(&(ibp->rvp.lock))) {
> > >
> > > Okay, this looks reasonable
> > >
> > > Mike, Dennis, is this the right lock to test?
> > >
> >
> > I'm looking at that right now actually, I don't think this is correct.
> > Wanted to talk to Mike before I send a response though.
> >
> > -Denny
>
> That's definitely going to throw a ton of lock dep messages. It's not really
> the right lock either. Instead what we probably need to do is what we do in
> the non-multicast part of the code and take the rcu_read_lock().
Uh.. why is this using the _rcu varient without holding the rcu lock?
That is quite wrong already.
Jason
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
2020-01-14 19:41 ` Jason Gunthorpe
@ 2020-01-14 19:46 ` Dennis Dalessandro
0 siblings, 0 replies; 14+ messages in thread
From: Dennis Dalessandro @ 2020-01-14 19:46 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: madhuparnabhowmik04, mike.marciniszyn, paulmck, joel, frextrite,
linux-kernel-mentees, rcu, linux-rdma, linux-kernel
On 1/14/2020 2:41 PM, Jason Gunthorpe wrote:
> On Tue, Jan 14, 2020 at 01:24:00PM -0500, Dennis Dalessandro wrote:
>> On 1/14/2020 12:00 PM, Dennis Dalessandro wrote:
>>> On 1/14/2020 11:57 AM, Jason Gunthorpe wrote:
>>>> On Tue, Jan 14, 2020 at 09:53:45PM +0530,
>>>> madhuparnabhowmik04@gmail.com wrote:
>>>>> From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
>>>>>
>>>>> list_for_each_entry_rcu has built-in RCU and lock checking.
>>>>> Pass cond argument to list_for_each_entry_rcu.
>>>>>
>>>>> Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
>>>>> drivers/infiniband/hw/hfi1/verbs.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/infiniband/hw/hfi1/verbs.c
>>>>> b/drivers/infiniband/hw/hfi1/verbs.c
>>>>> index 089e201d7550..22f2d4fd2577 100644
>>>>> +++ b/drivers/infiniband/hw/hfi1/verbs.c
>>>>> @@ -515,7 +515,7 @@ static inline void hfi1_handle_packet(struct
>>>>> hfi1_packet *packet,
>>>>> opa_get_lid(packet->dlid, 9B));
>>>>> if (!mcast)
>>>>> goto drop;
>>>>> - list_for_each_entry_rcu(p, &mcast->qp_list, list) {
>>>>> + list_for_each_entry_rcu(p, &mcast->qp_list, list,
>>>>> lockdep_is_held(&(ibp->rvp.lock))) {
>>>>
>>>> Okay, this looks reasonable
>>>>
>>>> Mike, Dennis, is this the right lock to test?
>>>>
>>>
>>> I'm looking at that right now actually, I don't think this is correct.
>>> Wanted to talk to Mike before I send a response though.
>>>
>>> -Denny
>>
>> That's definitely going to throw a ton of lock dep messages. It's not really
>> the right lock either. Instead what we probably need to do is what we do in
>> the non-multicast part of the code and take the rcu_read_lock().
>
> Uh.. why is this using the _rcu varient without holding the rcu lock?
> That is quite wrong already.
>
Yep, seems like a bug to me. Patch forthcoming.
-Denny
^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <CAF65HP0RsW5FMRRf5Lia2=MTKex-KwO7-_NsCUef94YKBg+pfA@mail.gmail.com>]
* Re: [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
[not found] ` <CAF65HP0RsW5FMRRf5Lia2=MTKex-KwO7-_NsCUef94YKBg+pfA@mail.gmail.com>
@ 2020-02-14 17:24 ` Dennis Dalessandro
0 siblings, 0 replies; 14+ messages in thread
From: Dennis Dalessandro @ 2020-02-14 17:24 UTC (permalink / raw)
To: Madhuparna Bhowmik, Jason Gunthorpe
Cc: mike.marciniszyn, Paul E. McKenney, Joel Fernandes, Amol Grover,
linux-kernel-mentees, rcu, linux-rdma, linux-kernel
On 2/14/2020 10:43 AM, Madhuparna Bhowmik wrote:
>
>
> On Wed, Jan 15, 2020 at 12:05 AM <madhuparnabhowmik04@gmail.com
> <mailto:madhuparnabhowmik04@gmail.com>> wrote:
>
> From: Dennis Dalessandro <dennis.dalessandro@intel.com
> <mailto:dennis.dalessandro@intel.com>>
>
> On 1/14/2020 12:00 PM, Dennis Dalessandro wrote:
> > On 1/14/2020 11:57 AM, Jason Gunthorpe wrote:
> >> On Tue, Jan 14, 2020 at 09:53:45PM +0530,
> >> madhuparnabhowmik04@gmail.com
> <mailto:madhuparnabhowmik04@gmail.com> wrote:
> >>> From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com
> <mailto:madhuparnabhowmik04@gmail.com>>
> >>>
> >>> list_for_each_entry_rcu has built-in RCU and lock checking.
> >>> Pass cond argument to list_for_each_entry_rcu.
> >>>
> >>> Signed-off-by: Madhuparna Bhowmik
> <madhuparnabhowmik04@gmail.com <mailto:madhuparnabhowmik04@gmail.com>>
> >>> drivers/infiniband/hw/hfi1/verbs.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/infiniband/hw/hfi1/verbs.c
> >>> b/drivers/infiniband/hw/hfi1/verbs.c
> >>> index 089e201d7550..22f2d4fd2577 100644
> >>> +++ b/drivers/infiniband/hw/hfi1/verbs.c
> >>> @@ -515,7 +515,7 @@ static inline void hfi1_handle_packet(struct
> >>> hfi1_packet *packet,
> >>> opa_get_lid(packet->dlid, 9B));
> >>> if (!mcast)
> >>> goto drop;
> >>> - list_for_each_entry_rcu(p, &mcast->qp_list, list) {
> >>> + list_for_each_entry_rcu(p, &mcast->qp_list, list,
> >>> lockdep_is_held(&(ibp->rvp.lock))) {
> >>
> >> Okay, this looks reasonable
> >>
> >> Mike, Dennis, is this the right lock to test?
> >>
> >
> > I'm looking at that right now actually, I don't think this is
> correct.
> > Wanted to talk to Mike before I send a response though.
> >
> > -Denny
>
> That's definitely going to throw a ton of lock dep messages. It's not
> really the right lock either. Instead what we probably need to do is
> what we do in the non-multicast part of the code and take the
> rcu_read_lock().
>
> I'd say hold off on this and we'll fix it right. Same goes for the
> qib one.
>
> Alright, thank you for reviewing.
>
> The rdmavt one though looks to be OK. I'll give it a test.
>
> Hi,
> I just wanted to follow up on this.
> Any updates?
> Also, is the bug fixed now?
>
> Thank you,
> Madhuparna
>
> Thank you,
> Madhuparna
>
> -Denny
>
I've got a patch going through internal discussion and testing for
adding rcu read locking.
The RDMAVT patch, I was OK with going in, I guess I just mentioned that
in a reply rather than adding an RB tag. Let me go ahead and do that.
-Denny
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
@ 2020-01-07 19:29 madhuparnabhowmik04
2020-01-07 20:33 ` Jason Gunthorpe
0 siblings, 1 reply; 14+ messages in thread
From: madhuparnabhowmik04 @ 2020-01-07 19:29 UTC (permalink / raw)
To: dennis.dalessandro, mike.marciniszyn, dledford, paulmck
Cc: rcu, joel, frextrite, linux-kernel-mentees, linux-rdma,
linux-kernel, Madhuparna Bhowmik
From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
list_for_each_entry_rcu has built-in RCU and lock checking.
Pass cond argument to list_for_each_entry_rcu.
Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
---
drivers/infiniband/hw/hfi1/verbs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index 089e201d7550..e6abdbcb4ffb 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -515,7 +515,8 @@ static inline void hfi1_handle_packet(struct hfi1_packet *packet,
opa_get_lid(packet->dlid, 9B));
if (!mcast)
goto drop;
- list_for_each_entry_rcu(p, &mcast->qp_list, list) {
+ list_for_each_entry_rcu(p, &mcast->qp_list, list,
+ lock_is_held(&(ibp->rvp.lock).dep_map)) {
packet->qp = p->qp;
if (hfi1_do_pkey_check(packet))
goto drop;
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
2020-01-07 19:29 madhuparnabhowmik04
@ 2020-01-07 20:33 ` Jason Gunthorpe
2020-01-09 18:10 ` Jason Gunthorpe
2020-01-10 15:54 ` Joel Fernandes
0 siblings, 2 replies; 14+ messages in thread
From: Jason Gunthorpe @ 2020-01-07 20:33 UTC (permalink / raw)
To: madhuparnabhowmik04
Cc: dennis.dalessandro, mike.marciniszyn, dledford, paulmck, rcu,
joel, frextrite, linux-kernel-mentees, linux-rdma, linux-kernel
On Wed, Jan 08, 2020 at 12:59:12AM +0530, madhuparnabhowmik04@gmail.com wrote:
> From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
>
> list_for_each_entry_rcu has built-in RCU and lock checking.
> Pass cond argument to list_for_each_entry_rcu.
>
> Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
> drivers/infiniband/hw/hfi1/verbs.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
> index 089e201d7550..e6abdbcb4ffb 100644
> +++ b/drivers/infiniband/hw/hfi1/verbs.c
> @@ -515,7 +515,8 @@ static inline void hfi1_handle_packet(struct hfi1_packet *packet,
> opa_get_lid(packet->dlid, 9B));
> if (!mcast)
> goto drop;
> - list_for_each_entry_rcu(p, &mcast->qp_list, list) {
> + list_for_each_entry_rcu(p, &mcast->qp_list, list,
> + lock_is_held(&(ibp->rvp.lock).dep_map)) {
Why .dep_map? Does this compile?
Jason
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
2020-01-07 20:33 ` Jason Gunthorpe
@ 2020-01-09 18:10 ` Jason Gunthorpe
2020-01-10 15:54 ` Joel Fernandes
1 sibling, 0 replies; 14+ messages in thread
From: Jason Gunthorpe @ 2020-01-09 18:10 UTC (permalink / raw)
To: madhuparnabhowmik04
Cc: dennis.dalessandro, mike.marciniszyn, dledford, paulmck, rcu,
joel, frextrite, linux-kernel-mentees, linux-rdma, linux-kernel
On Wed, Jan 08, 2020 at 01:38:50PM +0530, madhuparnabhowmik04@gmail.com wrote:
> Alternatively, it can be lockdep_is_held(ibp->rvp.lock).
> Please refer to the macro(link below) and let me know if the usage of lock_is_held()
> in the patch is correct.
lock_is_held is the normal way to write this
Jason
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
2020-01-07 20:33 ` Jason Gunthorpe
2020-01-09 18:10 ` Jason Gunthorpe
@ 2020-01-10 15:54 ` Joel Fernandes
1 sibling, 0 replies; 14+ messages in thread
From: Joel Fernandes @ 2020-01-10 15:54 UTC (permalink / raw)
To: madhuparnabhowmik04
Cc: jgg, dennis.dalessandro, mike.marciniszyn, dledford, paulmck,
rcu, frextrite, linux-kernel-mentees, linux-rdma, linux-kernel
On Wed, Jan 08, 2020 at 01:35:07PM +0530, madhuparnabhowmik04@gmail.com wrote:
> From: Jason Gunthorpe <jgg@ziepe.ca>
>
> On Wed, Jan 08, 2020 at 12:59:12AM +0530, madhuparnabhowmik04@gmail.com wrote:
> > From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
> >
> > list_for_each_entry_rcu has built-in RCU and lock checking.
> > Pass cond argument to list_for_each_entry_rcu.
> >
> > Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
> > drivers/infiniband/hw/hfi1/verbs.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
> > index 089e201d7550..e6abdbcb4ffb 100644
> > +++ b/drivers/infiniband/hw/hfi1/verbs.c
> > @@ -515,7 +515,8 @@ static inline void hfi1_handle_packet(struct hfi1_packet *packet,
> > opa_get_lid(packet->dlid, 9B));
> > if (!mcast)
> > goto drop;
> > - list_for_each_entry_rcu(p, &mcast->qp_list, list) {
> > + list_for_each_entry_rcu(p, &mcast->qp_list, list,
> > + lock_is_held(&(ibp->rvp.lock).dep_map)) {
>
> Why .dep_map? Does this compile?
Yeah, have you really compiled this? Don't send patches without at least
compile testing !!
> Alternatively, it can be lockdep_is_held(ibp->rvp.lock).
> Please refer to the macro(link below) and let me know if the usage of lock_is_held()
> in the patch is correct.
>
> https://elixir.bootlin.com/linux/v5.5-rc2/source/include/linux/lockdep.h#L364
Please use lockdep_is_held(). Thanks.
thanks,
- Joel
>
> Thanks,
> Madhuparna
> Jason
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
@ 2020-01-07 17:35 madhuparnabhowmik04
2020-01-07 18:26 ` Jason Gunthorpe
0 siblings, 1 reply; 14+ messages in thread
From: madhuparnabhowmik04 @ 2020-01-07 17:35 UTC (permalink / raw)
To: dennis.dalessandro, mike.marciniszyn, dledford, paulmck
Cc: rcu, joel, frextrite, linux-kernel-mentees, linux-rdma,
linux-kernel, Madhuparna Bhowmik
From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
list_for_each_entry_rcu has built-in RCU and lock checking.
Pass cond argument to list_for_each_entry_rcu.
Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
---
drivers/infiniband/hw/hfi1/verbs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index 089e201d7550..cab2ff185240 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -515,7 +515,8 @@ static inline void hfi1_handle_packet(struct hfi1_packet *packet,
opa_get_lid(packet->dlid, 9B));
if (!mcast)
goto drop;
- list_for_each_entry_rcu(p, &mcast->qp_list, list) {
+ list_for_each_entry_rcu(p, &mcast->qp_list, list
+ lock_is_held(&(ibp->rvp.lock).dep_map)) {
packet->qp = p->qp;
if (hfi1_do_pkey_check(packet))
goto drop;
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking
2020-01-07 17:35 madhuparnabhowmik04
@ 2020-01-07 18:26 ` Jason Gunthorpe
0 siblings, 0 replies; 14+ messages in thread
From: Jason Gunthorpe @ 2020-01-07 18:26 UTC (permalink / raw)
To: madhuparnabhowmik04
Cc: dennis.dalessandro, mike.marciniszyn, dledford, paulmck, rcu,
joel, frextrite, linux-kernel-mentees, linux-rdma, linux-kernel
On Tue, Jan 07, 2020 at 11:05:08PM +0530, madhuparnabhowmik04@gmail.com wrote:
> From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
>
> list_for_each_entry_rcu has built-in RCU and lock checking.
> Pass cond argument to list_for_each_entry_rcu.
>
> Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
> drivers/infiniband/hw/hfi1/verbs.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
> index 089e201d7550..cab2ff185240 100644
> +++ b/drivers/infiniband/hw/hfi1/verbs.c
> @@ -515,7 +515,8 @@ static inline void hfi1_handle_packet(struct hfi1_packet *packet,
> opa_get_lid(packet->dlid, 9B));
> if (!mcast)
> goto drop;
> - list_for_each_entry_rcu(p, &mcast->qp_list, list) {
> + list_for_each_entry_rcu(p, &mcast->qp_list, list
> + lock_is_held(&(ibp->rvp.lock).dep_map)) {
This is missing a ',' and isn't indented properly. Does it even
compile?
The idea seems sound though.
Jason
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-02-14 17:25 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 16:23 [PATCH 1/3] infiniband: hw: hfi1: verbs.c: Use built-in RCU list checking madhuparnabhowmik04
2020-01-14 16:57 ` Jason Gunthorpe
2020-01-14 17:00 ` Dennis Dalessandro
2020-01-14 18:24 ` Dennis Dalessandro
2020-01-14 19:17 ` Joel Fernandes
2020-01-14 19:41 ` Jason Gunthorpe
2020-01-14 19:46 ` Dennis Dalessandro
[not found] ` <CAF65HP0RsW5FMRRf5Lia2=MTKex-KwO7-_NsCUef94YKBg+pfA@mail.gmail.com>
2020-02-14 17:24 ` Dennis Dalessandro
-- strict thread matches above, loose matches on Subject: below --
2020-01-07 19:29 madhuparnabhowmik04
2020-01-07 20:33 ` Jason Gunthorpe
2020-01-09 18:10 ` Jason Gunthorpe
2020-01-10 15:54 ` Joel Fernandes
2020-01-07 17:35 madhuparnabhowmik04
2020-01-07 18:26 ` Jason Gunthorpe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).