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 #9331|review?([EMAIL PROTECTED])|review-
               Flag|                            |


(From update of attachment 9331)
requirements:
> The client cancels all the conflict locks for the current metadata operation 
> on the client side beforehand

I think you shoulddrop word 'metadata' here and everywhere else in this
section. We want to cancel locks beforehand even for non-metadata operations
like destroy (and may be truncate and read/write in the future).

ldlm_resource_cancel_unused is poorly named, there is nothing about unused,
perhaps it is better to rename it to ldlm_resource_cancel_list?

Like I already said, ldlm_resource_get_unused should take policy argument OR
may
 be we need specific versions of that function for different lock types?
hardcoding bits is confusing, I think. We well might want to match certain
subse
ts of extent locks very soon now.
In ldlm_resource_get_unused( you happily use l_bl_ast, but I think we need some
proof there is no parallel user for that filed too (like lru cancel).

Code calling ldlm_lru_get_redundant now does not set instant cancel flag at all
which will result in cancel RPCs to be sent even if server support bundled
cance
ls. This is bad. I think you only need to use ldlm_lru_get_redundant (in
ldlm_cli_enqueue) if server supports bundled cancels. And always set
LDLM_FL_CANCEL_ON_BLOCK flag.
In section Scalability and performance:
Currently there is a case where performance gets worse and number of RPCs grow:
when you bundle cancel (And cancel lock), but before you finally send your rpc,
some parallel thread gets that lock again. That's one RPC more (to get the
lock)

Alternatives:
like we discussed - we might bundle cancels for conflicting PR and PW locks
when getting intersecting PW/PR locks, but this is counterproductive. Better
course of actions here would be to revive our lock-converting code and improve
it to allove for extent-growing (I made a comment in bug 2746 to better explain
that).

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

Reply via email to