All of lore.kernel.org
 help / color / mirror / Atom feed
* [Ocfs2-devel] [PATCH] ocfs2: move the mlog from the spinlock to outside
@ 2019-09-30  3:17 Shuning Zhang
  2019-10-07 22:11 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Shuning Zhang @ 2019-09-30  3:17 UTC (permalink / raw)
  To: ocfs2-devel

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.

Signed-off-by: Shuning Zhang <sunny.s.zhang@oracle.com>
---
 fs/ocfs2/dlmglue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 1420723..9960e61 100644
--- 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;
 	}
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Ocfs2-devel] [PATCH] ocfs2: move the mlog from the spinlock to outside
  2019-09-30  3:17 [Ocfs2-devel] [PATCH] ocfs2: move the mlog from the spinlock to outside Shuning Zhang
@ 2019-10-07 22:11 ` Andrew Morton
  2019-10-08  1:14   ` Joseph Qi
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2019-10-07 22:11 UTC (permalink / raw)
  To: ocfs2-devel

On Mon, 30 Sep 2019 11:17:39 +0800 Shuning Zhang <sunny.s.zhang@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?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Ocfs2-devel] [PATCH] ocfs2: move the mlog from the spinlock to outside
  2019-10-07 22:11 ` Andrew Morton
@ 2019-10-08  1:14   ` Joseph Qi
  2019-10-08  5:00     ` sunnyZhang
  0 siblings, 1 reply; 6+ messages in thread
From: Joseph Qi @ 2019-10-08  1:14 UTC (permalink / raw)
  To: ocfs2-devel



On 19/10/8 06:11, Andrew Morton wrote:
> On Mon, 30 Sep 2019 11:17:39 +0800 Shuning Zhang <sunny.s.zhang@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.

> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Ocfs2-devel] [PATCH] ocfs2: move the mlog from the spinlock to outside
  2019-10-08  1:14   ` Joseph Qi
@ 2019-10-08  5:00     ` sunnyZhang
  2019-10-08  9:51       ` Joseph Qi
  0 siblings, 1 reply; 6+ messages in thread
From: sunnyZhang @ 2019-10-08  5:00 UTC (permalink / raw)
  To: ocfs2-devel


? 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@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
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Ocfs2-devel] [PATCH] ocfs2: move the mlog from the spinlock to outside
  2019-10-08  5:00     ` sunnyZhang
@ 2019-10-08  9:51       ` Joseph Qi
  2019-10-09  7:39         ` sunnyZhang
  0 siblings, 1 reply; 6+ messages in thread
From: Joseph Qi @ 2019-10-08  9:51 UTC (permalink / raw)
  To: 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@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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Ocfs2-devel] [PATCH] ocfs2: move the mlog from the spinlock to outside
  2019-10-08  9:51       ` Joseph Qi
@ 2019-10-09  7:39         ` sunnyZhang
  0 siblings, 0 replies; 6+ messages in thread
From: sunnyZhang @ 2019-10-09  7:39 UTC (permalink / raw)
  To: ocfs2-devel


? 2019/10/8 ??5:51, Joseph Qi ??:
>
> 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@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"?

I apology for my stupidity.

I have always had a wrong memory, I think printk() can sleep.

@Andrew Please can you help me to withdraw this patch?


Thanks

> Thanks,
> Joseph
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-10-09  7:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-30  3:17 [Ocfs2-devel] [PATCH] ocfs2: move the mlog from the spinlock to outside Shuning Zhang
2019-10-07 22:11 ` Andrew Morton
2019-10-08  1:14   ` Joseph Qi
2019-10-08  5:00     ` sunnyZhang
2019-10-08  9:51       ` Joseph Qi
2019-10-09  7:39         ` sunnyZhang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.