Please don't reply to lustre-devel. Instead, comment in Bugzilla by using the
following link:
https://bugzilla.lustre.org/show_bug.cgi?id=10589
What |Removed |Added
----------------------------------------------------------------------------
Attachment #9277|review?([EMAIL PROTECTED])|review-
Flag| |
(From update of attachment 9277)
> The period of time that a lock is allowed to be alive. When expired, the lock
> is to be cancelled.
> LDLM_MAX_ALIVE
Don't make it static, it should be adjustable through /proc
> int ldlm_resource_get_unused(ldlm_resource *res, list_head *head, __u64 bits)
> This method gathers all the unused locks on the resource @res into the @head.
> Only those inodebits locks are added to @head that match by the policy @bits.
> The further attempts to find or cancel locks inserted into the list @head,
> will not find nor be able to cancel them.
I think it should be applicable to any lock, not just inodebit locks.
You want this same feature for extent locks too, for example (on destroy).
perhaps it should take policy here and gather all locks that intersect
policy-wise.
In fact I see you use this function from OSC too, and bits there just
make zero sense.
Also throughout the HLD you keep talking about MDS, this should work on
both MDS and OST.
> int ll_mdc_blocking_ast(struct ldlm_lock *lock, struct ldlm_lock_desc *desc,
> void *data, int flag)
> The callback that is called when a blocking request is received or a cancel
> request is about to be sent. It changes all the metadata and data guarded
> with the cancelled lock.
It does not change metadata, it uncaches/invalidates.
> void lustre_swab_ldlm_request(struct ldlm_request *rq)
> Convert the ldlm_request structre from the little endian wire format to
> CPU-endian format.
In fact wire format is cpu-endin format for ending cpu. swabbing only happens
if nodes at the ends of the wire are differently-endian, as I understand it.
usecase missing:
extent lock enqueue cancelling old locks from lru beforehand.
logic:
in ldlm_lru_get_redundant:
LDLM_LOCK_GET(lock);
spin_unlock(&ns->ns_unused_lock);
lock_res_and_lock(lock);
And at this point lock might have been removed from lru by other process (and
not necessary for cancel (you check this case)!
you need to verify it is still in lru by e.g. calling ldlm_lock_remove_from_lru
(you need to do it anyway to remove the lock from lru), see example in
ldlm_cancel_lru
in ll_mdc_blocking_ast:
You cannot use ll_have_md_lock, I think. It does match with LDLM_FL_CBPENDING
and therefore will always match (this same lock you are cancelling) and
will return 1 always.
In osc_destroy: we want to set LDLM_FL_DISCARD_DATA in flags of the locks
before cancelling it, so that whatever dirty data we have is not flushed
to server.
Elsewhere in mdc* osc_destroy:
there is no point in searching locks when OBD_CONNECT_EARLY_CANCEL I think?
Or do you think we can save on blocking ast? Then for such a case cancelling
ASTs should be sent to servers (and ldlm_resource_cancel_unused must not
set the cancelling flags), failing to send these ASTs will cause server to send
blocking ASTs for these locks still (that client does not have already),
and then evicting client because it never replies with cancel.
In lustre_swab_ldlm_request:
Why no swabbing for lock handles themselves if there are more than 2?
Locking:
> While we are sending a batched cancel request, a lock may be cancelled on
> server in parallel. Then this lock will not be found by its handle on the
> server when our batched cancel request will be handled. This is also
> considered as a valid race.
This cannot happen. Server won't cancel lock without confirmation from the
client, so either it degenerates into 1st case in locking section or the lock
is already canceled client-side and then we won't find it on client and won't
send the handle to server.
_______________________________________________
Lustre-devel mailing list
[email protected]
https://mail.clusterfs.com/mailman/listinfo/lustre-devel
