Commit 3999e49364193f7dbbba66e2be655fe91ba1fced
Committed by
Al Viro
1 parent
48f7418654
Exists in
master
and in
20 other branches
locks: add a new "lm_owner_key" lock operation
Currently, the hashing that the locking code uses to add these values to the blocked_hash is simply calculated using fl_owner field. That's valid in most cases except for server-side lockd, which validates the owner of a lock based on fl_owner and fl_pid. In the case where you have a small number of NFS clients doing a lot of locking between different processes, you could end up with all the blocked requests sitting in a very small number of hash buckets. Add a new lm_owner_key operation to the lock_manager_operations that will generate an unsigned long to use as the key in the hashtable. That function is only implemented for server-side lockd, and simply XORs the fl_owner and fl_pid. Signed-off-by: Jeff Layton <jlayton@redhat.com> Acked-by: J. Bruce Fields <bfields@fieldses.org> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Showing 4 changed files with 34 additions and 7 deletions Side-by-side Diff
Documentation/filesystems/Locking
... | ... | @@ -349,6 +349,7 @@ |
349 | 349 | ----------------------- lock_manager_operations --------------------------- |
350 | 350 | prototypes: |
351 | 351 | int (*lm_compare_owner)(struct file_lock *, struct file_lock *); |
352 | + unsigned long (*lm_owner_key)(struct file_lock *); | |
352 | 353 | void (*lm_notify)(struct file_lock *); /* unblock callback */ |
353 | 354 | int (*lm_grant)(struct file_lock *, struct file_lock *, int); |
354 | 355 | void (*lm_break)(struct file_lock *); /* break_lease callback */ |
355 | 356 | |
... | ... | @@ -358,16 +359,21 @@ |
358 | 359 | |
359 | 360 | inode->i_lock file_lock_lock may block |
360 | 361 | lm_compare_owner: yes[1] maybe no |
362 | +lm_owner_key yes[1] yes no | |
361 | 363 | lm_notify: yes yes no |
362 | 364 | lm_grant: no no no |
363 | 365 | lm_break: yes no no |
364 | 366 | lm_change yes no no |
365 | 367 | |
366 | -[1]: ->lm_compare_owner is generally called with *an* inode->i_lock held. It | |
367 | -may not be the i_lock of the inode for either file_lock being compared! This is | |
368 | -the case with deadlock detection, since the code has to chase down the owners | |
369 | -of locks that may be entirely unrelated to the one on which the lock is being | |
370 | -acquired. When doing a search for deadlocks, the file_lock_lock is also held. | |
368 | +[1]: ->lm_compare_owner and ->lm_owner_key are generally called with | |
369 | +*an* inode->i_lock held. It may not be the i_lock of the inode | |
370 | +associated with either file_lock argument! This is the case with deadlock | |
371 | +detection, since the code has to chase down the owners of locks that may | |
372 | +be entirely unrelated to the one on which the lock is being acquired. | |
373 | +For deadlock detection however, the file_lock_lock is also held. The | |
374 | +fact that these locks are held ensures that the file_locks do not | |
375 | +disappear out from under you while doing the comparison or generating an | |
376 | +owner key. | |
371 | 377 | |
372 | 378 | --------------------------- buffer_head ----------------------------------- |
373 | 379 | prototypes: |
fs/lockd/svclock.c
... | ... | @@ -744,8 +744,20 @@ |
744 | 744 | return fl1->fl_owner == fl2->fl_owner && fl1->fl_pid == fl2->fl_pid; |
745 | 745 | } |
746 | 746 | |
747 | +/* | |
748 | + * Since NLM uses two "keys" for tracking locks, we need to hash them down | |
749 | + * to one for the blocked_hash. Here, we're just xor'ing the host address | |
750 | + * with the pid in order to create a key value for picking a hash bucket. | |
751 | + */ | |
752 | +static unsigned long | |
753 | +nlmsvc_owner_key(struct file_lock *fl) | |
754 | +{ | |
755 | + return (unsigned long)fl->fl_owner ^ (unsigned long)fl->fl_pid; | |
756 | +} | |
757 | + | |
747 | 758 | const struct lock_manager_operations nlmsvc_lock_operations = { |
748 | 759 | .lm_compare_owner = nlmsvc_same_owner, |
760 | + .lm_owner_key = nlmsvc_owner_key, | |
749 | 761 | .lm_notify = nlmsvc_notify_blocked, |
750 | 762 | .lm_grant = nlmsvc_grant_deferred, |
751 | 763 | }; |
fs/locks.c
... | ... | @@ -521,10 +521,18 @@ |
521 | 521 | spin_unlock(&file_lock_lock); |
522 | 522 | } |
523 | 523 | |
524 | +static unsigned long | |
525 | +posix_owner_key(struct file_lock *fl) | |
526 | +{ | |
527 | + if (fl->fl_lmops && fl->fl_lmops->lm_owner_key) | |
528 | + return fl->fl_lmops->lm_owner_key(fl); | |
529 | + return (unsigned long)fl->fl_owner; | |
530 | +} | |
531 | + | |
524 | 532 | static inline void |
525 | 533 | locks_insert_global_blocked(struct file_lock *waiter) |
526 | 534 | { |
527 | - hash_add(blocked_hash, &waiter->fl_link, (unsigned long)waiter->fl_owner); | |
535 | + hash_add(blocked_hash, &waiter->fl_link, posix_owner_key(waiter)); | |
528 | 536 | } |
529 | 537 | |
530 | 538 | static inline void |
... | ... | @@ -757,7 +765,7 @@ |
757 | 765 | { |
758 | 766 | struct file_lock *fl; |
759 | 767 | |
760 | - hash_for_each_possible(blocked_hash, fl, fl_link, (unsigned long)block_fl->fl_owner) { | |
768 | + hash_for_each_possible(blocked_hash, fl, fl_link, posix_owner_key(block_fl)) { | |
761 | 769 | if (posix_same_owner(fl, block_fl)) |
762 | 770 | return fl->fl_next; |
763 | 771 | } |
include/linux/fs.h
... | ... | @@ -908,6 +908,7 @@ |
908 | 908 | |
909 | 909 | struct lock_manager_operations { |
910 | 910 | int (*lm_compare_owner)(struct file_lock *, struct file_lock *); |
911 | + unsigned long (*lm_owner_key)(struct file_lock *); | |
911 | 912 | void (*lm_notify)(struct file_lock *); /* unblock callback */ |
912 | 913 | int (*lm_grant)(struct file_lock *, struct file_lock *, int); |
913 | 914 | void (*lm_break)(struct file_lock *); |