Re: [PATCH] IB/hfi1,IB/qib: Fix qp_stats sleep with rcu read lock held

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

 



On 8/10/2016 1:51 AM, ira.weiny wrote:
> On Tue, Aug 09, 2016 at 09:11:46PM +0300, Leon Romanovsky wrote:
>> On Tue, Aug 09, 2016 at 11:16:26AM -0400, ira.weiny@xxxxxxxxx wrote:
>>> From: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxx>
>>>
> 
> [snip]
> 
>>> diff --git a/drivers/infiniband/hw/hfi1/debugfs.c b/drivers/infiniband/hw/hfi1/debugfs.c
>>> index dbab9d9cc288..c35bef8dd5aa 100644
>>> --- a/drivers/infiniband/hw/hfi1/debugfs.c
>>> +++ b/drivers/infiniband/hw/hfi1/debugfs.c
>>> @@ -223,28 +223,35 @@ DEBUGFS_SEQ_FILE_OPEN(ctx_stats)
>>>  DEBUGFS_FILE_OPS(ctx_stats);
>>>  
>>>  static void *_qp_stats_seq_start(struct seq_file *s, loff_t *pos)
>>> -__acquires(RCU)
>>> +	__acquires(RCU)
>>>  {
>>>  	struct qp_iter *iter;
>>>  	loff_t n = *pos;
>>>  
>>> -	rcu_read_lock();
>>>  	iter = qp_iter_init(s->private);
>>> +
>>> +	/* stop calls rcu_read_unlock */
>>> +	rcu_read_lock();
>>
>> IMHO, it should be placed after your if(!iter) check below.
> 
> I know this seems weird but this makes the rcu locking "balanced".  Returning
> NULL here still calls "stop" which is going to call rcu_read_unlock.  If we
> move rcu_read_lock we need to maintain state to determine if we called lock or
> not.  This is easier.  To me it does seem odd that the sequence operation does
> not stop on its own when NULL is returned but that is the way it works.
> 
>>
>>> +
>>>  	if (!iter)
>>>  		return NULL;
>>>  
>>> -	while (n--) {
>>> +	if (qp_iter_next(iter)) {
>>> +		kfree(iter);
>>> +		return NULL;
>>> +	}
>>> +	while (n--)
>>>  		if (qp_iter_next(iter)) {
>>>  			kfree(iter);
>>>  			return NULL;
>>>  		}
>>
>> It looks like you forgot to remove the lines above.
> 
> Nope this replaces a call to qp_iter_next which was in qp_iter_init.
> 
> As the commit messages says we move the implict next's back into the respective
> start functions.
> 
> Ira

Thanks, applied.


-- 
Doug Ledford <dledford@xxxxxxxxxx>
    GPG Key ID: 0E572FDD

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]