Please don't reply to lustre-devel. Instead, comment in Bugzilla by using the 
following link:
https://bugzilla.lustre.org/show_bug.cgi?id=10902

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #9242|review?([EMAIL PROTECTED])|review-
               Flag|                            |


(From update of attachment 9242)
> ldlm_inodebits_compat_queue(struct list_head *queue, struct ldlm_lock *req,
>                             struct list_head *work_list)
> {
>-        struct list_head *tmp;
>+        struct list_head *tmp, *tmp_tail;
>         struct ldlm_lock *lock;
>         ldlm_mode_t req_mode = req->l_req_mode;
>         __u64 req_bits = req->l_policy_data.l_inodebits.bits;
>         int compat = 1;
>+        int granted_list = -1;

Minor comment, probably easiest of all would be to not assign anything to
granted list and have this construction instead here:
if (!list_empty(queue)) {
lock = list_entry(queue->next, ...);
granted_list = (lock->reqmode == lock->grantedmode) ? 1:0;
}
(hm, this will probably generate a warning about possible uninitialised usage
of granted_list, so you
might initially init it to 0, or have an else part that will do RETURN(0)
immediatelly).
This way we can save one comparison for every loop iteration.

Same in plain_compat function.

Also may be there was some sort of miscommunication between us. I did not ask
you to remove ldlm_lock_convert() changes.
What you need to do there is when you remove lock from granted queue, you need
to remember position where it was, and on a branch where the lock is reinserted
to granted queue again (and only there), you need to insert the lock back at
that position (one of your functions allows this). Of course you only need to
do this for plain and inodebits locks only.
And please think up a plan on how this specific codepath can be tested.

_______________________________________________
Lustre-devel mailing list
[email protected]
https://mail.clusterfs.com/mailman/listinfo/lustre-devel

Reply via email to