On 6/30/21 11:55 AM, J. Bruce Fields wrote:
On Wed, Jun 30, 2021 at 11:49:18AM -0700, dai.ngo@xxxxxxxxxx wrote:
On 6/30/21 11:05 AM, J. Bruce Fields wrote:
On Wed, Jun 30, 2021 at 10:51:27AM -0700, dai.ngo@xxxxxxxxxx wrote:
On 6/28/21 1:23 PM, J. Bruce Fields wrote:
where ->fl_expire_lock is a new lock callback with second
argument "check"
where:
check = 1 means: just check whether this lock could be freed
Why do we need this, is there a use case for it? can we just always try
to expire the lock and return success/fail?
We can't expire the client while holding the flc_lock. And once we drop
that lock we need to restart the loop. Clearly we can't do that every
time.
(So, my code was wrong, it should have been:
if (fl->fl_lops->fl_expire_lock(fl, 1)) {
spin_unlock(&ct->flc_lock);
fl->fl_lops->fl_expire_locks(fl, 0);
goto retry;
}
)
This is what I currently have:
retry:
list_for_each_entry(fl, &ctx->flc_posix, fl_list) {
if (!posix_locks_conflict(request, fl))
continue;
if (fl->fl_lmops && fl->fl_lmops->lm_expire_lock) {
spin_unlock(&ctx->flc_lock);
ret = fl->fl_lmops->lm_expire_lock(fl, 0);
spin_lock(&ctx->flc_lock);
if (ret)
goto retry;
We have to retry regardless of the return value. Once we've dropped
flc_lock, it's not safe to continue trying to iterate through the list.
Yes, thanks!
}
if (conflock)
locks_copy_conflock(conflock, fl);
But the 1 and 0 cases are starting to look pretty different; maybe they
should be two different callbacks.
why the case of 1 (test only) is needed, who would use this call?
We need to avoid dropping the spinlock in the case there are no clients
to expire, otherwise we'll make no forward progress.
I think we can remember the last checked file_lock and skip it:
retry:
list_for_each_entry(fl, &ctx->flc_posix, fl_list) {
if (!posix_locks_conflict(request, fl))
continue;
if (checked_fl != fl && fl->fl_lmops &&
fl->fl_lmops->lm_expire_lock) {
checked_fl = fl;
spin_unlock(&ctx->flc_lock);
fl->fl_lmops->lm_expire_lock(fl);
spin_lock(&ctx->flc_lock);
goto retry;
}
if (conflock)
locks_copy_conflock(conflock, fl);
-Dai
--b.
