[#LU-22] ldlm_resource::lr_lvb_data is protected by wrong lock

[LU-22] ldlm_resource::lr_lvb_data is protected by wrong lock Created: 14/Dec/10 Updated: 07/Apr/11 Resolved: 07/Apr/11
Status:	Resolved
Project:	Lustre
Component/s:	None
Affects Version/s:	None
Fix Version/s:	Lustre 2.1.0

Type:

Bug

Priority:

Minor

Reporter:

Liang Zhen (Inactive)

Assignee:

Zhenyu Xu

Resolution:

Fixed

Votes:

Labels:

None

Attachments:

0001-b24336-ldlm_resource-lr_lvb_data-is-protected-by-wro.patch

Severity:

Bugzilla ID:

24,336

Rank (Obsolete):

5085

Description

I found lr_lvb_data is protected by resource lock (lock_res_and_lock) at some places, i.e:
ldlm_server_completion_ast()
lock_res_and_lock(lock);
memcpy(lvb, lock->l_resource->lr_lvb_data,
lock->l_resource->lr_lvb_len);
unlock_res_and_lock(lock);

but what I learnt from the code is, it should be protected by lr_lvb_sem.

Comments

Comment by Liang Zhen (Inactive) [ 15/Dec/10 ]

patch is at here:
http://review.whamcloud.com/143

it's also in inspection on bugzilla

Comment by Liang Zhen (Inactive) [ 21/Dec/10 ]

patch landed on master

Comment by Liang Zhen (Inactive) [ 21/Dec/10 ]

patch landed on master

Comment by Liang Zhen (Inactive) [ 22/Dec/10 ]

I found the fix is not completed, there are more issues in filter_intent_policy, which still access lr_lvb_data with holding resource lock, we can't fix it easily by changing it to lr_lvb_sem.
I need a little more time to survey before posting a patch.

Comment by Liang Zhen (Inactive) [ 22/Dec/10 ]

I think we probably should just use lr_lvb_sem for serialize lvbo_init/lvbo_fini with other threads(pin/unpin the inode), and use resource::lr_lock to protect update/access of resource::lr_lvb_data.
I don't see any reason we redo those job for finding inode in lvbo_update (seems Andreas thinking the same, comment #85 of bug 22107), if we removed those code for finding inode in lvbo_update, then it's not a complex change to fix the issue.

Comment by Liang Zhen (Inactive) [ 23/Dec/10 ]

reassign to bobijam, as I still got a few pending things at hand...

Comment by Zhenyu Xu [ 23/Dec/10 ]

after some discussion with fan & liang, we'd use lr_lock to protect lr_lvbo_data while lr_lvbo_sem is only for serializing lvbo_init.

Comment by Zhenyu Xu [ 26/Dec/10 ]

patch posted in http://review.whamcloud.com/158

Comment by Build Master (Inactive) [ 31/Mar/11 ]

Integrated in reviews-centos5 #615
~~LU-22~~ ldlm_resource::lr_lvb_data is protected by wrong lock

Bobi Jam : e50244c5ebbcd5d0e78f6e03366b2efcbefac4ab
Files :

lustre/include/lustre_dlm.h
lustre/ldlm/ldlm_lockd.c
lustre/obdfilter/filter_lvb.c

Comment by Build Master (Inactive) [ 31/Mar/11 ]

Integrated in reviews-centos5 #616
~~LU-22~~ ldlm_resource::lr_lvb_data is protected by wrong lock

Bobi Jam : 40defeaaaf7510e5b4e78542cf122506c029313b
Files :

lustre/include/lustre_dlm.h
lustre/obdfilter/filter_lvb.c
lustre/ldlm/ldlm_lockd.c

Comment by Build Master (Inactive) [ 31/Mar/11 ]

Integrated in lustre-master-centos5 #178
~~LU-22~~ ldlm_resource::lr_lvb_data is protected by wrong lock

Oleg Drokin : e1491c1b4b6e21352f58b94eee6d125a63566d29
Files :

lustre/ldlm/ldlm_lockd.c
lustre/include/lustre_dlm.h
lustre/obdfilter/filter_lvb.c