[PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[Jeff Layton]

In the days of yore, the file locking code was primarily protected by
the BKL. That changed in commit 72f98e72551fa (locks: turn lock_flocks
into a spinlock), at which point the code was changed to be protected
by a conventional spinlock (mostly due to a push to finally eliminate
the BKL). Since then, the code has been changed to use the i_lock
instead of a global spinlock, but it's still under a spinlock.

With that change, several functions now no longer can block when they
originally could. This is a particular problem with the
fl_release_private operation. In NFSv4, that operation is used to kick
off a RELEASE_LOCKOWNER or FREE_STATEID call, and that requires being
able to do an allocation.

This was reported by Josh Stone here:

    https://bugzilla.redhat.com/show_bug.cgi?id=1089092

My initial stab at fixing this involved moving this to a workqueue, but
Trond pointed out the above change was technically a regression with the
way the spinlocking in the file locking code works, and suggested an
alternate approach to fixing it.

This set focuses on moving most of the locks_release_private calls
outside of the inode->i_lock. There are still a few that are done
under the i_lock in the lease handling code. Cleaning up the use of
the i_lock in the lease code is a larger project which we'll have to
tackle at some point, but there are some other cleanups that will
need to happen first.

Absent any objections, I'll plan to merge these for 3.18.

Jeff Layton (5):
  locks: don't call locks_release_private from locks_copy_lock
  locks: don't reuse file_lock in __posix_lock_file
  locks: defer freeing locks in locks_delete_lock until after i_lock has
    been dropped
  locks: move locks_free_lock calls in do_fcntl_add_lease outside
    spinlock
  locks: update Locking documentation to clarify fl_release_private
    behavior

 Documentation/filesystems/Locking |  6 ++-
 fs/locks.c                        | 80 +++++++++++++++++++++++++--------------
 2 files changed, 57 insertions(+), 29 deletions(-)

-- 
1.9.3

[PATCH 1/5] locks: don't call locks_release_private from locks_copy_lock

[Jeff Layton]

All callers of locks_copy_lock pass in a brand new file_lock struct, so
there's no need to calls locks_release_private on it. Replace that with
a warning that fires in the event that we receive a target lock that
doesn't look like it's properly initialized.

Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
 fs/locks.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/locks.c b/fs/locks.c
index 356667a434c1..2c2d4f5022a7 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -285,7 +285,8 @@ EXPORT_SYMBOL(__locks_copy_lock);
 
 void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
 {
-	locks_release_private(new);
+	/* "new" must be a freshly-initialized lock */
+	WARN_ON_ONCE(new->fl_ops);
 
 	__locks_copy_lock(new, fl);
 	new->fl_file = fl->fl_file;
-- 
1.9.3

[PATCH 2/5] locks: don't reuse file_lock in __posix_lock_file

[Jeff Layton]

Currently in the case where a new file lock completely replaces the old
one, we end up overwriting the existing lock with the new info. This
means that we have to call fl_release_private inside i_lock. Change the
code to instead copy the info to new_fl, insert that lock into the
correct spot and then delete the old lock. In a later patch, we'll defer
the freeing of the old lock until after the i_lock has been dropped.

Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
 fs/locks.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 2c2d4f5022a7..7dd4defb4d8d 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1022,18 +1022,21 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 					locks_delete_lock(before);
 					continue;
 				}
-				/* Replace the old lock with the new one.
-				 * Wake up anybody waiting for the old one,
-				 * as the change in lock type might satisfy
-				 * their needs.
+				/*
+				 * Replace the old lock with new_fl, and
+				 * remove the old one. It's safe to do the
+				 * insert here since we know that we won't be
+				 * using new_fl later, and that the lock is
+				 * just replacing an existing lock.
 				 */
-				locks_wake_up_blocks(fl);
-				fl->fl_start = request->fl_start;
-				fl->fl_end = request->fl_end;
-				fl->fl_type = request->fl_type;
-				locks_release_private(fl);
-				locks_copy_private(fl, request);
-				request = fl;
+				error = -ENOLCK;
+				if (!new_fl)
+					goto out;
+				locks_copy_lock(new_fl, request);
+				request = new_fl;
+				new_fl = NULL;
+				locks_delete_lock(before);
+				locks_insert_lock(before, request);
 				added = true;
 			}
 		}
-- 
1.9.3

[PATCH 3/5] locks: defer freeing locks in locks_delete_lock until after i_lock has been dropped

[Jeff Layton]

In commit 72f98e72551fa (locks: turn lock_flocks into a spinlock), we
moved from using the BKL to a global spinlock. With this change, we lost
the ability to block in the fl_release_private operation.

This is problematic for NFS (and probably some other filesystems as
well). Add a new list_head argument to locks_delete_lock. If that
argument is non-NULL, then queue any locks that we want to free to the
list instead of freeing them.

Then, add a new locks_dispose_list function that will walk such a list
and call locks_free_lock on them after the i_lock has been dropped.

Finally, change all of the callers of locks_delete_lock to pass in a
list_head, except for lease_modify. That function can be called long
after the i_lock has been acquired. Deferring the freeing of a lease
after unlocking it in that function is non-trivial until we overhaul
some of the spinlocking in the lease code.

Currently though, no filesystem that sets fl_release_private supports
leases, so this is not currently a problem. We'll eventually want to
make the same change in the lease code, but it needs a lot more work
before we can reasonably do so.

Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
 fs/locks.c | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 7dd4defb4d8d..4ce087cca501 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -247,6 +247,18 @@ void locks_free_lock(struct file_lock *fl)
 }
 EXPORT_SYMBOL(locks_free_lock);
 
+static void
+locks_dispose_list(struct list_head *dispose)
+{
+	struct file_lock *fl;
+
+	while (!list_empty(dispose)) {
+		fl = list_first_entry(dispose, struct file_lock, fl_block);
+		list_del_init(&fl->fl_block);
+		locks_free_lock(fl);
+	}
+}
+
 void locks_init_lock(struct file_lock *fl)
 {
 	memset(fl, 0, sizeof(struct file_lock));
@@ -651,12 +663,16 @@ static void locks_unlink_lock(struct file_lock **thisfl_p)
  *
  * Must be called with i_lock held!
  */
-static void locks_delete_lock(struct file_lock **thisfl_p)
+static void locks_delete_lock(struct file_lock **thisfl_p,
+			      struct list_head *dispose)
 {
 	struct file_lock *fl = *thisfl_p;
 
 	locks_unlink_lock(thisfl_p);
-	locks_free_lock(fl);
+	if (dispose)
+		list_add(&fl->fl_block, dispose);
+	else
+		locks_free_lock(fl);
 }
 
 /* Determine if lock sys_fl blocks lock caller_fl. Common functionality
@@ -812,6 +828,7 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
 	struct inode * inode = file_inode(filp);
 	int error = 0;
 	int found = 0;
+	LIST_HEAD(dispose);
 
 	if (!(request->fl_flags & FL_ACCESS) && (request->fl_type != F_UNLCK)) {
 		new_fl = locks_alloc_lock();
@@ -834,7 +851,7 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
 		if (request->fl_type == fl->fl_type)
 			goto out;
 		found = 1;
-		locks_delete_lock(before);
+		locks_delete_lock(before, &dispose);
 		break;
 	}
 
@@ -881,6 +898,7 @@ out:
 	spin_unlock(&inode->i_lock);
 	if (new_fl)
 		locks_free_lock(new_fl);
+	locks_dispose_list(&dispose);
 	return error;
 }
 
@@ -894,6 +912,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 	struct file_lock **before;
 	int error;
 	bool added = false;
+	LIST_HEAD(dispose);
 
 	/*
 	 * We may need two file_lock structures for this operation,
@@ -989,7 +1008,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 			else
 				request->fl_end = fl->fl_end;
 			if (added) {
-				locks_delete_lock(before);
+				locks_delete_lock(before, &dispose);
 				continue;
 			}
 			request = fl;
@@ -1019,7 +1038,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 				 * one (This may happen several times).
 				 */
 				if (added) {
-					locks_delete_lock(before);
+					locks_delete_lock(before, &dispose);
 					continue;
 				}
 				/*
@@ -1035,7 +1054,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 				locks_copy_lock(new_fl, request);
 				request = new_fl;
 				new_fl = NULL;
-				locks_delete_lock(before);
+				locks_delete_lock(before, &dispose);
 				locks_insert_lock(before, request);
 				added = true;
 			}
@@ -1097,6 +1116,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
 		locks_free_lock(new_fl);
 	if (new_fl2)
 		locks_free_lock(new_fl2);
+	locks_dispose_list(&dispose);
 	return error;
 }
 
@@ -1272,7 +1292,7 @@ int lease_modify(struct file_lock **before, int arg)
 			printk(KERN_ERR "locks_delete_lock: fasync == %p\n", fl->fl_fasync);
 			fl->fl_fasync = NULL;
 		}
-		locks_delete_lock(before);
+		locks_delete_lock(before, NULL);
 	}
 	return 0;
 }
@@ -2324,6 +2344,7 @@ void locks_remove_file(struct file *filp)
 	struct inode * inode = file_inode(filp);
 	struct file_lock *fl;
 	struct file_lock **before;
+	LIST_HEAD(dispose);
 
 	if (!inode->i_flock)
 		return;
@@ -2369,12 +2390,13 @@ void locks_remove_file(struct file *filp)
 				fl->fl_type, fl->fl_flags,
 				fl->fl_start, fl->fl_end);
 
-			locks_delete_lock(before);
+			locks_delete_lock(before, &dispose);
 			continue;
  		}
 		before = &fl->fl_next;
 	}
 	spin_unlock(&inode->i_lock);
+	locks_dispose_list(&dispose);
 }
 
 /**
-- 
1.9.3

[PATCH 4/5] locks: move locks_free_lock calls in do_fcntl_add_lease outside spinlock

[Jeff Layton]

There's no need to call locks_free_lock here while still holding the
i_lock. Defer that until the lock has been dropped.

Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
 fs/locks.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 4ce087cca501..cb66fb05ad4a 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1761,13 +1761,10 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg)
 	ret = fl;
 	spin_lock(&inode->i_lock);
 	error = __vfs_setlease(filp, arg, &ret);
-	if (error) {
-		spin_unlock(&inode->i_lock);
-		locks_free_lock(fl);
-		goto out_free_fasync;
-	}
-	if (ret != fl)
-		locks_free_lock(fl);
+	if (error)
+		goto out_unlock;
+	if (ret == fl)
+		fl = NULL;
 
 	/*
 	 * fasync_insert_entry() returns the old entry if any.
@@ -1779,9 +1776,10 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg)
 		new = NULL;
 
 	error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
+out_unlock:
 	spin_unlock(&inode->i_lock);
-
-out_free_fasync:
+	if (fl)
+		locks_free_lock(fl);
 	if (new)
 		fasync_free(new);
 	return error;
-- 
1.9.3

[PATCH 5/5] locks: update Locking documentation to clarify fl_release_private behavior

[Jeff Layton]

Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
 Documentation/filesystems/Locking | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index b18dd1779029..f1997e9da61f 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -349,7 +349,11 @@ prototypes:
 locking rules:
 			inode->i_lock	may block
 fl_copy_lock:		yes		no
-fl_release_private:	maybe		no
+fl_release_private:	maybe		maybe[1]
+
+[1]:	->fl_release_private for flock or POSIX locks is currently allowed
+to block. Leases however can still be freed while the i_lock is held and
+so fl_release_private called on a lease should not block.
 
 ----------------------- lock_manager_operations ---------------------------
 prototypes:
-- 
1.9.3

Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[Christoph Hellwig]

Btw, I might be missing something here, but wouldn't it be better
to reference count the file_lock structure and grab a reference to
it where we currently call (__)locks_copy_lock?

Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[Jeff Layton]

On Tue, 12 Aug 2014 08:32:29 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> Btw, I might be missing something here, but wouldn't it be better
> to reference count the file_lock structure and grab a reference to
> it where we currently call (__)locks_copy_lock?
> 

It's not really possible with the way this code works at the moment.
The problem there is that struct file_lock can represent any of:

- a lock request (copied from userland into a kernel structure)
- a lock record (that lives on the i_flock list)
- a conflicting lock record (returned by GETLK-type request)

In many cases we call (__)locks_copy_lock to copy from one "use-type" of
struct file_lock to another, and doing that with refcounts makes that a
bit difficult to manage.

If I were designing this code from scratch, I'd have probably made each
use-type a distinct structure. Maybe we should do that anyway at
some point -- I'm not sure.

Now, all that said...I think we will end up needing to do some sort of
refcounting to fix the leasing code at some point. Currently,
->setlease operations can't block, which is a significant impediment to
adding leases to clustered filesystems and the like. It would be nice
to lift that limitation and that may require making leases be
refcounted (or maybe RCU-managed).

-- 
Jeff Layton <jlayton@primarydata.com>

Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[Jeff Layton]

On Tue, 12 Aug 2014 10:48:08 -0400
Jeff Layton <jlayton@primarydata.com> wrote:

> In the days of yore, the file locking code was primarily protected by
> the BKL. That changed in commit 72f98e72551fa (locks: turn lock_flocks
> into a spinlock), at which point the code was changed to be protected
> by a conventional spinlock (mostly due to a push to finally eliminate
> the BKL). Since then, the code has been changed to use the i_lock
> instead of a global spinlock, but it's still under a spinlock.
> 
> With that change, several functions now no longer can block when they
> originally could. This is a particular problem with the
> fl_release_private operation. In NFSv4, that operation is used to kick
> off a RELEASE_LOCKOWNER or FREE_STATEID call, and that requires being
> able to do an allocation.
> 
> This was reported by Josh Stone here:
> 
>     https://bugzilla.redhat.com/show_bug.cgi?id=1089092
> 
> My initial stab at fixing this involved moving this to a workqueue, but
> Trond pointed out the above change was technically a regression with the
> way the spinlocking in the file locking code works, and suggested an
> alternate approach to fixing it.
> 
> This set focuses on moving most of the locks_release_private calls
> outside of the inode->i_lock. There are still a few that are done
> under the i_lock in the lease handling code. Cleaning up the use of
> the i_lock in the lease code is a larger project which we'll have to
> tackle at some point, but there are some other cleanups that will
> need to happen first.
> 
> Absent any objections, I'll plan to merge these for 3.18.
> 

Erm...make that v3.17...

As Trond points out, the fact that we end up doing sleeping allocations
under spinlock can allow an unprivileged user to crash a NFSv4 client.
So I may see about merging these sooner rather than later after they've
had a little soak time in linux-next.

-- 
Jeff Layton <jlayton@primarydata.com>

Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[Jeff Layton]

On Tue, 12 Aug 2014 10:48:08 -0400
Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org> wrote:

> In the days of yore, the file locking code was primarily protected by
> the BKL. That changed in commit 72f98e72551fa (locks: turn lock_flocks
> into a spinlock), at which point the code was changed to be protected
> by a conventional spinlock (mostly due to a push to finally eliminate
> the BKL). Since then, the code has been changed to use the i_lock
> instead of a global spinlock, but it's still under a spinlock.
> 
> With that change, several functions now no longer can block when they
> originally could. This is a particular problem with the
> fl_release_private operation. In NFSv4, that operation is used to kick
> off a RELEASE_LOCKOWNER or FREE_STATEID call, and that requires being
> able to do an allocation.
> 
> This was reported by Josh Stone here:
> 
>     https://bugzilla.redhat.com/show_bug.cgi?id=1089092
> 
> My initial stab at fixing this involved moving this to a workqueue, but
> Trond pointed out the above change was technically a regression with the
> way the spinlocking in the file locking code works, and suggested an
> alternate approach to fixing it.
> 
> This set focuses on moving most of the locks_release_private calls
> outside of the inode->i_lock. There are still a few that are done
> under the i_lock in the lease handling code. Cleaning up the use of
> the i_lock in the lease code is a larger project which we'll have to
> tackle at some point, but there are some other cleanups that will
> need to happen first.
> 
> Absent any objections, I'll plan to merge these for 3.18.
> 

Erm...make that v3.17...

As Trond points out, the fact that we end up doing sleeping allocations
under spinlock can allow an unprivileged user to crash a NFSv4 client.
So I may see about merging these sooner rather than later after they've
had a little soak time in linux-next.

-- 
Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[J. Bruce Fields]

On Tue, Aug 12, 2014 at 01:43:13PM -0400, Jeff Layton wrote:
> On Tue, 12 Aug 2014 10:48:08 -0400
> Jeff Layton <jlayton@primarydata.com> wrote:
> 
> > In the days of yore, the file locking code was primarily protected by
> > the BKL. That changed in commit 72f98e72551fa (locks: turn lock_flocks
> > into a spinlock), at which point the code was changed to be protected
> > by a conventional spinlock (mostly due to a push to finally eliminate
> > the BKL). Since then, the code has been changed to use the i_lock
> > instead of a global spinlock, but it's still under a spinlock.
> > 
> > With that change, several functions now no longer can block when they
> > originally could. This is a particular problem with the
> > fl_release_private operation. In NFSv4, that operation is used to kick
> > off a RELEASE_LOCKOWNER or FREE_STATEID call, and that requires being
> > able to do an allocation.
> > 
> > This was reported by Josh Stone here:
> > 
> >     https://bugzilla.redhat.com/show_bug.cgi?id=1089092
> > 
> > My initial stab at fixing this involved moving this to a workqueue, but
> > Trond pointed out the above change was technically a regression with the
> > way the spinlocking in the file locking code works, and suggested an
> > alternate approach to fixing it.
> > 
> > This set focuses on moving most of the locks_release_private calls
> > outside of the inode->i_lock. There are still a few that are done
> > under the i_lock in the lease handling code. Cleaning up the use of
> > the i_lock in the lease code is a larger project which we'll have to
> > tackle at some point, but there are some other cleanups that will
> > need to happen first.
> > 
> > Absent any objections, I'll plan to merge these for 3.18.
> > 
> 
> Erm...make that v3.17...
> 
> As Trond points out, the fact that we end up doing sleeping allocations
> under spinlock can allow an unprivileged user to crash a NFSv4 client.
> So I may see about merging these sooner rather than later after they've
> had a little soak time in linux-next.

Looks reasonable to me--ACK on the set.

--b.

Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[J. Bruce Fields]

On Tue, Aug 12, 2014 at 01:43:13PM -0400, Jeff Layton wrote:
> On Tue, 12 Aug 2014 10:48:08 -0400
> Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org> wrote:
> 
> > In the days of yore, the file locking code was primarily protected by
> > the BKL. That changed in commit 72f98e72551fa (locks: turn lock_flocks
> > into a spinlock), at which point the code was changed to be protected
> > by a conventional spinlock (mostly due to a push to finally eliminate
> > the BKL). Since then, the code has been changed to use the i_lock
> > instead of a global spinlock, but it's still under a spinlock.
> > 
> > With that change, several functions now no longer can block when they
> > originally could. This is a particular problem with the
> > fl_release_private operation. In NFSv4, that operation is used to kick
> > off a RELEASE_LOCKOWNER or FREE_STATEID call, and that requires being
> > able to do an allocation.
> > 
> > This was reported by Josh Stone here:
> > 
> >     https://bugzilla.redhat.com/show_bug.cgi?id=1089092
> > 
> > My initial stab at fixing this involved moving this to a workqueue, but
> > Trond pointed out the above change was technically a regression with the
> > way the spinlocking in the file locking code works, and suggested an
> > alternate approach to fixing it.
> > 
> > This set focuses on moving most of the locks_release_private calls
> > outside of the inode->i_lock. There are still a few that are done
> > under the i_lock in the lease handling code. Cleaning up the use of
> > the i_lock in the lease code is a larger project which we'll have to
> > tackle at some point, but there are some other cleanups that will
> > need to happen first.
> > 
> > Absent any objections, I'll plan to merge these for 3.18.
> > 
> 
> Erm...make that v3.17...
> 
> As Trond points out, the fact that we end up doing sleeping allocations
> under spinlock can allow an unprivileged user to crash a NFSv4 client.
> So I may see about merging these sooner rather than later after they've
> had a little soak time in linux-next.

Looks reasonable to me--ACK on the set.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]

Hi Jeff,

On Tue, 12 Aug 2014 10:48:08 -0400 Jeff Layton <jlayton@primarydata.com> wrote:
>
> Absent any objections, I'll plan to merge these for 3.18.

This means that this patch set should *not* be in linux-next until after
(at least) v3.17-rc1 is released ...  This s reinforced by the lack of
Acked-by, Reviewed-by and Tested-by tags ... (the addition of which would presumably require the rebase (or rewrite) of a published git tree.)

/me suspects that many people do not read his daily release notes :-(
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[Jeff Layton]

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

On Wed, 13 Aug 2014 08:28:27 +1000
Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Jeff,
> 
> On Tue, 12 Aug 2014 10:48:08 -0400 Jeff Layton <jlayton@primarydata.com> wrote:
> >
> > Absent any objections, I'll plan to merge these for 3.18.
> 
> This means that this patch set should *not* be in linux-next until after
> (at least) v3.17-rc1 is released ...  This s reinforced by the lack of
> Acked-by, Reviewed-by and Tested-by tags ... (the addition of which would presumably require the rebase (or rewrite) of a published git tree.)
> 

It would, but I sent a later reply that revised that statement.

Trond pointed out that this problem can cause a user-triggerable oops,
so we may have to merge it for 3.17 after all. With that in mind, I
added these to my linux-next branch and will probably send a pull
request before the window closes (assuming that there are no glaring
problems with it).

While we're on the subject, we probably ought to rename my tree in your
"Trees" file from "file-private-locks" to "file-locks" or something.
File private locks (aka OFD locks) got merged in v3.15, but I have been
collecting patches that touch fs/locks.c

> /me suspects that many people do not read his daily release notes :-(

Guilty as charged. I'll try to keep up in the future.

Thanks,
-- 
Jeff Layton <jlayton@primarydata.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[Jeff Layton]

[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]

On Wed, 13 Aug 2014 08:28:27 +1000
Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org> wrote:

> Hi Jeff,
> 
> On Tue, 12 Aug 2014 10:48:08 -0400 Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org> wrote:
> >
> > Absent any objections, I'll plan to merge these for 3.18.
> 
> This means that this patch set should *not* be in linux-next until after
> (at least) v3.17-rc1 is released ...  This s reinforced by the lack of
> Acked-by, Reviewed-by and Tested-by tags ... (the addition of which would presumably require the rebase (or rewrite) of a published git tree.)
> 

It would, but I sent a later reply that revised that statement.

Trond pointed out that this problem can cause a user-triggerable oops,
so we may have to merge it for 3.17 after all. With that in mind, I
added these to my linux-next branch and will probably send a pull
request before the window closes (assuming that there are no glaring
problems with it).

While we're on the subject, we probably ought to rename my tree in your
"Trees" file from "file-private-locks" to "file-locks" or something.
File private locks (aka OFD locks) got merged in v3.15, but I have been
collecting patches that touch fs/locks.c

> /me suspects that many people do not read his daily release notes :-(

Guilty as charged. I'll try to keep up in the future.

Thanks,
-- 
Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

Hi Jeff,

On Tue, 12 Aug 2014 19:47:36 -0400 Jeff Layton <jeff.layton@primarydata.com> wrote:
>
> On Wed, 13 Aug 2014 08:28:27 +1000
> Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > On Tue, 12 Aug 2014 10:48:08 -0400 Jeff Layton <jlayton@primarydata.com> wrote:
> > >
> > > Absent any objections, I'll plan to merge these for 3.18.
> > 
> > This means that this patch set should *not* be in linux-next until after
> > (at least) v3.17-rc1 is released ...  This s reinforced by the lack of
> > Acked-by, Reviewed-by and Tested-by tags ... (the addition of which would
> > presumably require the rebase (or rewrite) of a published git tree.)
> 
> It would, but I sent a later reply that revised that statement.
> 
> Trond pointed out that this problem can cause a user-triggerable oops,
> so we may have to merge it for 3.17 after all. With that in mind, I
> added these to my linux-next branch and will probably send a pull
> request before the window closes (assuming that there are no glaring
> problems with it).

OK, fine.  I have merged it today in any case.

> While we're on the subject, we probably ought to rename my tree in your
> "Trees" file from "file-private-locks" to "file-locks" or something.
> File private locks (aka OFD locks) got merged in v3.15, but I have been
> collecting patches that touch fs/locks.c

OK, I will do that tomorrow.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock

[J. Bruce Fields]

On Wed, Aug 13, 2014 at 08:28:27AM +1000, Stephen Rothwell wrote:
> This s reinforced by the lack of Acked-by, Reviewed-by and Tested-by
> tags ... (the addition of which would presumably require the rebase
> (or rewrite) of a published git tree.)

By the way, I reshuffled my branches recently so the one you pull has
incoming patches that I think are mature enough to be worth testing but
haven't finished review yet.

That was partly Jeff's request, as he wanted his patches to get some
exposure while waiting for review.

It also means I can fold in some minor fixes instead of piling up fixes
for nits found by the kbuild robot.  (But maybe that unfairly denies it
some credit, I don't know.)

Anyway that means that branch is getting rewritten, say, weekly, as
opposed to never (or maybe for once-in-a-year level screwups).

Am I doing it wrong?

--b.