2014-04-15 12:44:20

by Jeff Layton

Subject: [PATCH] locks: allow __break_lease to sleep even when break_time is 0

A fl->fl_break_time of 0 has a special meaning to the lease break code
that basically means "never break the lease". knfsd uses this to ensure
that leases don't disappear out from under it.

Unfortunately, the code in __break_lease can end up passing this value
to wait_event_interruptible as a timeout, which prevents it from going
to sleep at all. This causes __break_lease to spin in a tight loop and
causes soft lockups.

Fix this by ensuring that we pass a minimum value of 1 as a timeout
instead.

Cc: <[email protected]>
Cc: J. Bruce Fields <[email protected]>
Reported-by: Terry Barnaby <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/locks.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 13fc7a6d380a..b380f5543614 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1391,11 +1391,10 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)

restart:
break_time = flock->fl_break_time;
- if (break_time != 0) {
+ if (break_time != 0)
break_time -= jiffies;
- if (break_time == 0)
- break_time++;
- }
+ if (break_time == 0)
+ break_time++;
locks_insert_block(flock, new_fl);
spin_unlock(&inode->i_lock);
error = wait_event_interruptible_timeout(new_fl->fl_wait,
--
1.9.0

2014-04-18 12:40:13

by J. Bruce Fields

Subject: Re: [PATCH] locks: allow __break_lease to sleep even when break_time is 0

Thanks, applying.--b.

On Tue, Apr 15, 2014 at 08:44:12AM -0400, Jeff Layton wrote:
> A fl->fl_break_time of 0 has a special meaning to the lease break code
> that basically means "never break the lease". knfsd uses this to ensure
> that leases don't disappear out from under it.
>
> Unfortunately, the code in __break_lease can end up passing this value
> to wait_event_interruptible as a timeout, which prevents it from going
> to sleep at all. This causes __break_lease to spin in a tight loop and
> causes soft lockups.
>
> Fix this by ensuring that we pass a minimum value of 1 as a timeout
> instead.
>
> Cc: <[email protected]>
> Cc: J. Bruce Fields <[email protected]>
> Reported-by: Terry Barnaby <[email protected]>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/locks.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/locks.c b/fs/locks.c
> index 13fc7a6d380a..b380f5543614 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -1391,11 +1391,10 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type)
>
> restart:
> break_time = flock->fl_break_time;
> - if (break_time != 0) {
> + if (break_time != 0)
> break_time -= jiffies;
> - if (break_time == 0)
> - break_time++;
> - }
> + if (break_time == 0)
> + break_time++;
> locks_insert_block(flock, new_fl);
> spin_unlock(&inode->i_lock);
> error = wait_event_interruptible_timeout(new_fl->fl_wait,
> --
> 1.9.0
>