Ocfs2 devel - Oct 2019 - [PATCH] ocfs2: move the mlog from the spinlock to outside

? 2019/10/8 ??9:14, Joseph Qi ??:
>
> On 19/10/8 06:11, Andrew Morton wrote:
>> On Mon, 30 Sep 2019 11:17:39 +0800 Shuning Zhang <sunny.s.zhang at
oracle.com> wrote:
>>
>>> There is a potential task of deadlock. Because the mask
>>> is 0, the deadlock does not occur now. But There is a
>>> potential task. If someone change the mask of mlog, but
>>> forget to modify the order of the mlog and spin_unlock,
>>> There will be a potential deadlock.So I move the mlog
>>> from the spinlock to outsize.
>>>
>>> ...
>>>
>>> --- a/fs/ocfs2/dlmglue.c
>>> +++ b/fs/ocfs2/dlmglue.c
>>> @@ -2315,10 +2315,10 @@ static int ocfs2_inode_lock_update(struct
inode *inode,
>>>   
>>>   	spin_lock(&oi->ip_lock);
>>>   	if (oi->ip_flags & OCFS2_INODE_DELETED) {
>>> +		spin_unlock(&oi->ip_lock);
>>>   		mlog(0, "Orphaned inode %llu was deleted while we "
>>>   		     "were waiting on a lock. ip_flags = 0x%x\n",
>>>   		     (unsigned long long)oi->ip_blkno, oi->ip_flags);
>>> -		spin_unlock(&oi->ip_lock);
>>>   		status = -ENOENT;
>>>   		goto bail;
>>>   	}
>> The patch is obviously OK but I don't see any deadlock.  mlog()
doesn't
>> take any locks?
>>
> I guess Shuning refers the calling of printk with spin lock.
Yes, It is the calling of printk.

Sorry for the description is not clear enough. :)
>> _______________________________________________
>> Ocfs2-devel mailing list
>> Ocfs2-devel at oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
>>

On 19/10/8 13:00, sunnyZhang wrote:
> 
> ? 2019/10/8 ??9:14, Joseph Qi ??:
>>
>> On 19/10/8 06:11, Andrew Morton wrote:
>>> On Mon, 30 Sep 2019 11:17:39 +0800 Shuning Zhang <sunny.s.zhang
at oracle.com> wrote:
>>>
>>>> There is a potential task of deadlock. Because the mask
>>>> is 0, the deadlock does not occur now. But There is a
>>>> potential task. If someone change the mask of mlog, but
>>>> forget to modify the order of the mlog and spin_unlock,
>>>> There will be a potential deadlock.So I move the mlog
>>>> from the spinlock to outsize.
>>>>
>>>> ...
>>>>
>>>> --- a/fs/ocfs2/dlmglue.c
>>>> +++ b/fs/ocfs2/dlmglue.c
>>>> @@ -2315,10 +2315,10 @@ static int
ocfs2_inode_lock_update(struct inode *inode,
>>>> ? ????? spin_lock(&oi->ip_lock);
>>>> ????? if (oi->ip_flags & OCFS2_INODE_DELETED) {
>>>> +??????? spin_unlock(&oi->ip_lock);
>>>> ????????? mlog(0, "Orphaned inode %llu was deleted while
we "
>>>> ?????????????? "were waiting on a lock. ip_flags =
0x%x\n",
>>>> ?????????????? (unsigned long long)oi->ip_blkno,
oi->ip_flags);
>>>> -??????? spin_unlock(&oi->ip_lock);
>>>> ????????? status = -ENOENT;
>>>> ????????? goto bail;
>>>> ????? }
>>> The patch is obviously OK but I don't see any deadlock.? mlog()
doesn't
>>> take any locks?
>>>
>> I guess Shuning refers the calling of printk with spin lock.
> 
> Yes, It is the calling of printk.
> 
> Sorry for the description is not clear enough. :)
> 
IIUC, we can call printk with spinlock, no deadlock will happen.
So I have the same question, where is the "potential deadlock"?

Thanks,
Joseph