Fix a 2.6.38 delegation regression

Fix a 2.6.38 delegation regression

[J. Bruce Fields]

When I removed the mylease callback for the 2.6.38 merge window, I
screwed up: I forgot that nfsd could hold multiple leases on the same
file on behalf of multiple clients, and that all those leases would be
associated with the same file descriptor.  This could cause a problem
when removing a lease, as the wrong lease could get removed.

There's no real reason why we should hold multiple leases; we can
instead just keep one lease, and use a count of the number of
delegations to decide when to remove it.

The following patches do that (after a little preperatory cleanup).

We could instead revert the mylease removal.  But I think this is still
a small enough change to justify after the merge window.  So barring
objections I intend to to submit this for 2.6.38 soon (along with a
couple other bugfixes I have queued up.)

--b.

[PATCH 1/6] nfsd4: split up nfsd_break_deleg_cb

[J. Bruce Fields]

We'll be adding some more code here soon.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |   35 +++++++++++++++++------------------
 1 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d98d021..b5bdc33 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2329,23 +2329,8 @@ nfs4_file_downgrade(struct nfs4_file *fp, unsigned int share_access)
 		nfs4_file_put_access(fp, O_RDONLY);
 }
 
-/*
- * Spawn a thread to perform a recall on the delegation represented
- * by the lease (file_lock)
- *
- * Called from break_lease() with lock_flocks() held.
- * Note: we assume break_lease will only call this *once* for any given
- * lease.
- */
-static
-void nfsd_break_deleg_cb(struct file_lock *fl)
+static void nfsd_break_one_deleg(struct nfs4_delegation *dp)
 {
-	struct nfs4_delegation *dp = (struct nfs4_delegation *)fl->fl_owner;
-
-	dprintk("NFSD nfsd_break_deleg_cb: dp %p fl %p\n",dp,fl);
-	if (!dp)
-		return;
-
 	/* We're assuming the state code never drops its reference
 	 * without first removing the lease.  Since we're in this lease
 	 * callback (and since the lease code is serialized by the kernel
@@ -2360,15 +2345,29 @@ void nfsd_break_deleg_cb(struct file_lock *fl)
 	/* only place dl_time is set. protected by lock_flocks*/
 	dp->dl_time = get_seconds();
 
+	nfsd4_cb_recall(dp);
+}
+
+/*
+ * Called from break_lease() with lock_flocks() held.
+ * Note: we assume break_lease will only call this *once* for any given
+ * lease.
+ */
+static
+void nfsd_break_deleg_cb(struct file_lock *fl)
+{
+	struct nfs4_delegation *dp = (struct nfs4_delegation *)fl->fl_owner;
+
+	BUG_ON(!dp);
 	/*
 	 * We don't want the locks code to timeout the lease for us;
 	 * we'll remove it ourself if the delegation isn't returned
-	 * in time.
+	 * in time:
 	 */
 	fl->fl_break_time = 0;
 
+	nfsd_break_one_deleg(dp);
 	dp->dl_file->fi_had_conflict = true;
-	nfsd4_cb_recall(dp);
 }
 
 static
-- 
1.7.1

[PATCH 2/6] nfsd4: add helper function for lease setup

[J. Bruce Fields]

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |   33 +++++++++++++++++++++------------
 1 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b5bdc33..a47aedd 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2640,6 +2640,26 @@ static bool nfsd4_cb_channel_good(struct nfs4_client *clp)
 	return clp->cl_minorversion && clp->cl_cb_state == NFSD4_CB_UNKNOWN;
 }
 
+static struct file_lock *nfs4_alloc_init_lease(struct nfs4_delegation *dp, int flag)
+{
+	struct file_lock *fl;
+
+	fl = locks_alloc_lock();
+	if (!fl)
+		return NULL;
+	locks_init_lock(fl);
+	fl->fl_lmops = &nfsd_lease_mng_ops;
+	fl->fl_flags = FL_LEASE;
+	fl->fl_type = flag == NFS4_OPEN_DELEGATE_READ? F_RDLCK: F_WRLCK;
+	fl->fl_end = OFFSET_MAX;
+	fl->fl_owner = (fl_owner_t)dp;
+	fl->fl_file = dp->dl_vfs_file;
+	BUG_ON(!fl->fl_file);
+	fl->fl_pid = current->tgid;
+	dp->dl_flock = fl;
+	return fl;
+}
+
 /*
  * Attempt to hand out a delegation.
  */
@@ -2685,20 +2705,9 @@ nfs4_open_delegation(struct svc_fh *fh, struct nfsd4_open *open, struct nfs4_sta
 		goto out;
 	}
 	status = -ENOMEM;
-	fl = locks_alloc_lock();
+	fl = nfs4_alloc_init_lease(dp, flag);
 	if (!fl)
 		goto out;
-	locks_init_lock(fl);
-	fl->fl_lmops = &nfsd_lease_mng_ops;
-	fl->fl_flags = FL_LEASE;
-	fl->fl_type = flag == NFS4_OPEN_DELEGATE_READ? F_RDLCK: F_WRLCK;
-	fl->fl_end = OFFSET_MAX;
-	fl->fl_owner =  (fl_owner_t)dp;
-	fl->fl_file = find_readable_file(stp->st_file);
-	BUG_ON(!fl->fl_file);
-	fl->fl_pid = current->tgid;
-	dp->dl_flock = fl;
-
 	/* vfs_setlease checks to see if delegation should be handed out.
 	 * the lock_manager callback fl_change is used
 	 */
-- 
1.7.1

[PATCH 3/6] nfsd4: fix leak on allocation error

[J. Bruce Fields]

Also share some common exit code.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a47aedd..b59b066 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2700,14 +2700,12 @@ nfs4_open_delegation(struct svc_fh *fh, struct nfsd4_open *open, struct nfs4_sta
 	}
 
 	dp = alloc_init_deleg(sop->so_client, stp, fh, flag);
-	if (dp == NULL) {
-		flag = NFS4_OPEN_DELEGATE_NONE;
-		goto out;
-	}
+	if (dp == NULL)
+		goto out_no_deleg;
 	status = -ENOMEM;
 	fl = nfs4_alloc_init_lease(dp, flag);
 	if (!fl)
-		goto out;
+		goto out_free;
 	/* vfs_setlease checks to see if delegation should be handed out.
 	 * the lock_manager callback fl_change is used
 	 */
@@ -2715,9 +2713,7 @@ nfs4_open_delegation(struct svc_fh *fh, struct nfsd4_open *open, struct nfs4_sta
 		dprintk("NFSD: setlease failed [%d], no delegation\n", status);
 		dp->dl_flock = NULL;
 		locks_free_lock(fl);
-		unhash_delegation(dp);
-		flag = NFS4_OPEN_DELEGATE_NONE;
-		goto out;
+		goto out_free;
 	}
 
 	memcpy(&open->op_delegate_stateid, &dp->dl_stateid, sizeof(dp->dl_stateid));
@@ -2730,6 +2726,12 @@ out:
 			&& open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE)
 		dprintk("NFSD: WARNING: refusing delegation reclaim\n");
 	open->op_delegate_type = flag;
+	return;
+out_free:
+	unhash_delegation(dp);
+out_no_deleg:
+	flag = NFS4_OPEN_DELEGATE_NONE;
+	goto out;
 }
 
 /*
-- 
1.7.1

[PATCH 4/6] nfsd4: split lease setting into separate function

[J. Bruce Fields]

Splitting some code into a separate function which we'll be adding some
more to.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |   32 +++++++++++++++++++-------------
 1 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b59b066..77e94f0 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2660,6 +2660,23 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_delegation *dp, int f
 	return fl;
 }
 
+static int nfs4_setlease(struct nfs4_delegation *dp, int flag)
+{
+	struct file_lock *fl;
+	int status;
+
+	fl = nfs4_alloc_init_lease(dp, flag);
+	if (!fl)
+		return -ENOMEM;
+	status = vfs_setlease(dp->dl_vfs_file, fl->fl_type, &fl);
+	if (status) {
+		dp->dl_flock = NULL;
+		locks_free_lock(fl);
+		return -ENOMEM;
+	}
+	return 0;
+}
+
 /*
  * Attempt to hand out a delegation.
  */
@@ -2669,7 +2686,6 @@ nfs4_open_delegation(struct svc_fh *fh, struct nfsd4_open *open, struct nfs4_sta
 	struct nfs4_delegation *dp;
 	struct nfs4_stateowner *sop = stp->st_stateowner;
 	int cb_up;
-	struct file_lock *fl;
 	int status, flag = 0;
 
 	cb_up = nfsd4_cb_channel_good(sop->so_client);
@@ -2702,19 +2718,9 @@ nfs4_open_delegation(struct svc_fh *fh, struct nfsd4_open *open, struct nfs4_sta
 	dp = alloc_init_deleg(sop->so_client, stp, fh, flag);
 	if (dp == NULL)
 		goto out_no_deleg;
-	status = -ENOMEM;
-	fl = nfs4_alloc_init_lease(dp, flag);
-	if (!fl)
-		goto out_free;
-	/* vfs_setlease checks to see if delegation should be handed out.
-	 * the lock_manager callback fl_change is used
-	 */
-	if ((status = vfs_setlease(fl->fl_file, fl->fl_type, &fl))) {
-		dprintk("NFSD: setlease failed [%d], no delegation\n", status);
-		dp->dl_flock = NULL;
-		locks_free_lock(fl);
+	status = nfs4_setlease(dp, flag);
+	if (status)
 		goto out_free;
-	}
 
 	memcpy(&open->op_delegate_stateid, &dp->dl_stateid, sizeof(dp->dl_stateid));
 
-- 
1.7.1

[PATCH 5/6] nfsd4: remove unused deleg dprintk's.

[J. Bruce Fields]

These aren't all that useful, and get in the way of the next steps.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 77e94f0..0375e99 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -958,8 +958,6 @@ expire_client(struct nfs4_client *clp)
 	spin_lock(&recall_lock);
 	while (!list_empty(&clp->cl_delegations)) {
 		dp = list_entry(clp->cl_delegations.next, struct nfs4_delegation, dl_perclnt);
-		dprintk("NFSD: expire client. dp %p, fp %p\n", dp,
-				dp->dl_flock);
 		list_del_init(&dp->dl_perclnt);
 		list_move(&dp->dl_recall_lru, &reaplist);
 	}
@@ -2932,8 +2930,6 @@ nfs4_laundromat(void)
 				test_val = u;
 			break;
 		}
-		dprintk("NFSD: purging unused delegation dp %p, fp %p\n",
-			            dp, dp->dl_flock);
 		list_move(&dp->dl_recall_lru, &reaplist);
 	}
 	spin_unlock(&recall_lock);
-- 
1.7.1

[PATCH 6/6] nfsd4: modify fi_delegations under recall_lock

[J. Bruce Fields]

Modify fi_delegations only under the recall_lock, allowing us to use
that list on lease breaks.

Also some trivial cleanup to simplify later changes.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4state.c |   99 +++++++++++++++++++++++++++++---------------------
 fs/nfsd/state.h     |    5 ++-
 2 files changed, 60 insertions(+), 44 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0375e99..a397de6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -230,9 +230,6 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_stateid *stp, struct svc_f
 	dp->dl_client = clp;
 	get_nfs4_file(fp);
 	dp->dl_file = fp;
-	dp->dl_vfs_file = find_readable_file(fp);
-	get_file(dp->dl_vfs_file);
-	dp->dl_flock = NULL;
 	dp->dl_type = type;
 	dp->dl_stateid.si_boot = boot_time;
 	dp->dl_stateid.si_stateownerid = current_delegid++;
@@ -241,8 +238,6 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_stateid *stp, struct svc_f
 	fh_copy_shallow(&dp->dl_fh, &current_fh->fh_handle);
 	dp->dl_time = 0;
 	atomic_set(&dp->dl_count, 1);
-	list_add(&dp->dl_perfile, &fp->fi_delegations);
-	list_add(&dp->dl_perclnt, &clp->cl_delegations);
 	INIT_WORK(&dp->dl_recall.cb_work, nfsd4_do_callback_rpc);
 	return dp;
 }
@@ -253,36 +248,30 @@ nfs4_put_delegation(struct nfs4_delegation *dp)
 	if (atomic_dec_and_test(&dp->dl_count)) {
 		dprintk("NFSD: freeing dp %p\n",dp);
 		put_nfs4_file(dp->dl_file);
-		fput(dp->dl_vfs_file);
 		kmem_cache_free(deleg_slab, dp);
 		num_delegations--;
 	}
 }
 
-/* Remove the associated file_lock first, then remove the delegation.
- * lease_modify() is called to remove the FS_LEASE file_lock from
- * the i_flock list, eventually calling nfsd's lock_manager
- * fl_release_callback.
- */
-static void
-nfs4_close_delegation(struct nfs4_delegation *dp)
+static void nfs4_put_deleg_lease(struct nfs4_file *fp)
 {
-	dprintk("NFSD: close_delegation dp %p\n",dp);
-	/* XXX: do we even need this check?: */
-	if (dp->dl_flock)
-		vfs_setlease(dp->dl_vfs_file, F_UNLCK, &dp->dl_flock);
+	if (atomic_dec_and_test(&fp->fi_delegees)) {
+		vfs_setlease(fp->fi_deleg_file, F_UNLCK, &fp->fi_lease);
+		fp->fi_lease = NULL;
+		fp->fi_deleg_file = NULL;
+	}
 }
 
 /* Called under the state lock. */
 static void
 unhash_delegation(struct nfs4_delegation *dp)
 {
-	list_del_init(&dp->dl_perfile);
 	list_del_init(&dp->dl_perclnt);
 	spin_lock(&recall_lock);
+	list_del_init(&dp->dl_perfile);
 	list_del_init(&dp->dl_recall_lru);
 	spin_unlock(&recall_lock);
-	nfs4_close_delegation(dp);
+	nfs4_put_deleg_lease(dp->dl_file);
 	nfs4_put_delegation(dp);
 }
 
@@ -2076,6 +2065,7 @@ alloc_init_file(struct inode *ino)
 		fp->fi_inode = igrab(ino);
 		fp->fi_id = current_fileid++;
 		fp->fi_had_conflict = false;
+		fp->fi_lease = NULL;
 		memset(fp->fi_fds, 0, sizeof(fp->fi_fds));
 		memset(fp->fi_access, 0, sizeof(fp->fi_access));
 		spin_lock(&recall_lock);
@@ -2336,9 +2326,7 @@ static void nfsd_break_one_deleg(struct nfs4_delegation *dp)
 	 * it's safe to take a reference: */
 	atomic_inc(&dp->dl_count);
 
-	spin_lock(&recall_lock);
 	list_add_tail(&dp->dl_recall_lru, &del_recall_lru);
-	spin_unlock(&recall_lock);
 
 	/* only place dl_time is set. protected by lock_flocks*/
 	dp->dl_time = get_seconds();
@@ -2346,26 +2334,28 @@ static void nfsd_break_one_deleg(struct nfs4_delegation *dp)
 	nfsd4_cb_recall(dp);
 }
 
-/*
- * Called from break_lease() with lock_flocks() held.
- * Note: we assume break_lease will only call this *once* for any given
- * lease.
- */
+/* Called from break_lease() with lock_flocks() held. */
 static
 void nfsd_break_deleg_cb(struct file_lock *fl)
 {
-	struct nfs4_delegation *dp = (struct nfs4_delegation *)fl->fl_owner;
+	struct nfs4_file *fp = (struct nfs4_file *)fl->fl_owner;
+	struct nfs4_delegation *dp;
 
-	BUG_ON(!dp);
+	BUG_ON(!fp);
+	/* We assume break_lease is only called once per lease: */
+	BUG_ON(fp->fi_had_conflict);
 	/*
 	 * We don't want the locks code to timeout the lease for us;
-	 * we'll remove it ourself if the delegation isn't returned
+	 * we'll remove it ourself if a delegation isn't returned
 	 * in time:
 	 */
 	fl->fl_break_time = 0;
 
-	nfsd_break_one_deleg(dp);
-	dp->dl_file->fi_had_conflict = true;
+	spin_lock(&recall_lock);
+	fp->fi_had_conflict = true;
+	list_for_each_entry(dp, &fp->fi_delegations, dl_perfile)
+		nfsd_break_one_deleg(dp);
+	spin_unlock(&recall_lock);
 }
 
 static
@@ -2456,13 +2446,15 @@ nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
 static struct nfs4_delegation *
 find_delegation_file(struct nfs4_file *fp, stateid_t *stid)
 {
-	struct nfs4_delegation *dp;
+	struct nfs4_delegation *dp = NULL;
 
+	spin_lock(&recall_lock);
 	list_for_each_entry(dp, &fp->fi_delegations, dl_perfile) {
 		if (dp->dl_stateid.si_stateownerid == stid->si_stateownerid)
-			return dp;
+			break;
 	}
-	return NULL;
+	spin_unlock(&recall_lock);
+	return dp;
 }
 
 int share_access_to_flags(u32 share_access)
@@ -2650,28 +2642,51 @@ static struct file_lock *nfs4_alloc_init_lease(struct nfs4_delegation *dp, int f
 	fl->fl_flags = FL_LEASE;
 	fl->fl_type = flag == NFS4_OPEN_DELEGATE_READ? F_RDLCK: F_WRLCK;
 	fl->fl_end = OFFSET_MAX;
-	fl->fl_owner = (fl_owner_t)dp;
-	fl->fl_file = dp->dl_vfs_file;
-	BUG_ON(!fl->fl_file);
+	fl->fl_owner = (fl_owner_t)(dp->dl_file);
 	fl->fl_pid = current->tgid;
-	dp->dl_flock = fl;
 	return fl;
 }
 
 static int nfs4_setlease(struct nfs4_delegation *dp, int flag)
 {
+	struct nfs4_file *fp = dp->dl_file;
 	struct file_lock *fl;
 	int status;
 
 	fl = nfs4_alloc_init_lease(dp, flag);
 	if (!fl)
 		return -ENOMEM;
-	status = vfs_setlease(dp->dl_vfs_file, fl->fl_type, &fl);
+	fl->fl_file = find_readable_file(fp);
+	list_add(&dp->dl_perclnt, &dp->dl_client->cl_delegations);
+	status = vfs_setlease(fl->fl_file, fl->fl_type, &fl);
 	if (status) {
-		dp->dl_flock = NULL;
+		list_del_init(&dp->dl_perclnt);
 		locks_free_lock(fl);
 		return -ENOMEM;
 	}
+	fp->fi_lease = fl;
+	fp->fi_deleg_file = fl->fl_file;
+	get_file(fp->fi_deleg_file);
+	atomic_set(&fp->fi_delegees, 1);
+	list_add(&dp->dl_perfile, &fp->fi_delegations);
+	return 0;
+}
+
+static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
+{
+	struct nfs4_file *fp = dp->dl_file;
+
+	if (!fp->fi_lease)
+		return nfs4_setlease(dp, flag);
+	spin_lock(&recall_lock);
+	if (fp->fi_had_conflict) {
+		spin_unlock(&recall_lock);
+		return -EAGAIN;
+	}
+	atomic_inc(&fp->fi_delegees);
+	list_add(&dp->dl_perfile, &fp->fi_delegations);
+	spin_unlock(&recall_lock);
+	list_add(&dp->dl_perclnt, &dp->dl_client->cl_delegations);
 	return 0;
 }
 
@@ -2716,7 +2731,7 @@ nfs4_open_delegation(struct svc_fh *fh, struct nfsd4_open *open, struct nfs4_sta
 	dp = alloc_init_deleg(sop->so_client, stp, fh, flag);
 	if (dp == NULL)
 		goto out_no_deleg;
-	status = nfs4_setlease(dp, flag);
+	status = nfs4_set_delegation(dp, flag);
 	if (status)
 		goto out_free;
 
@@ -2732,7 +2747,7 @@ out:
 	open->op_delegate_type = flag;
 	return;
 out_free:
-	unhash_delegation(dp);
+	nfs4_put_delegation(dp);
 out_no_deleg:
 	flag = NFS4_OPEN_DELEGATE_NONE;
 	goto out;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 3074656..2d31224 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -83,8 +83,6 @@ struct nfs4_delegation {
 	atomic_t		dl_count;       /* ref count */
 	struct nfs4_client	*dl_client;
 	struct nfs4_file	*dl_file;
-	struct file		*dl_vfs_file;
-	struct file_lock	*dl_flock;
 	u32			dl_type;
 	time_t			dl_time;
 /* For recall: */
@@ -379,6 +377,9 @@ struct nfs4_file {
 	 */
 	atomic_t		fi_readers;
 	atomic_t		fi_writers;
+	struct file		*fi_deleg_file;
+	struct file_lock	*fi_lease;
+	atomic_t		fi_delegees;
 	struct inode		*fi_inode;
 	u32                     fi_id;      /* used with stateowner->so_id 
 					     * for stateid_hashtbl hash */
-- 
1.7.1

Re: [PATCH 1/6] nfsd4: split up nfsd_break_deleg_cb

[Christoph Hellwig]

> +static
> +void nfsd_break_deleg_cb(struct file_lock *fl)

Very weird formatting.  Please make it either

static void nfsd_break_deleg_cb(struct file_lock *fl)

or

static void
nfsd_break_deleg_cb(struct file_lock *fl)

but never just put the static on a line of it's own.

Re: [PATCH 1/6] nfsd4: split up nfsd_break_deleg_cb

[J. Bruce Fields]

On Wed, Feb 09, 2011 at 12:01:18AM -0500, Christoph Hellwig wrote:
> > +static
> > +void nfsd_break_deleg_cb(struct file_lock *fl)
> 
> Very weird formatting.  Please make it either
> 
> static void nfsd_break_deleg_cb(struct file_lock *fl)
> 
> or
> 
> static void
> nfsd_break_deleg_cb(struct file_lock *fl)
> 
> but never just put the static on a line of it's own.

Agreed.  Those two lines were actually untouched by this patch, no
matter what diff thinks--but I'm just as happy to fix it up while we're
there; done.

--b.

Re: [PATCH 1/6] nfsd4: split up nfsd_break_deleg_cb

[Jim Rees]

J. Bruce Fields wrote:

  Agreed.  Those two lines were actually untouched by this patch, no
  matter what diff thinks--but I'm just as happy to fix it up while we're
  there; done.

I agree too, but "indent -linux" likes "static" on a line by itself, and
prefers it over "static void".  This is not the first time I've disagreed
with it, although I use it a lot.

Re: [PATCH 1/6] nfsd4: split up nfsd_break_deleg_cb

[Christoph Hellwig]

On Wed, Feb 09, 2011 at 11:28:38AM -0500, Jim Rees wrote:
> J. Bruce Fields wrote:
> 
>   Agreed.  Those two lines were actually untouched by this patch, no
>   matter what diff thinks--but I'm just as happy to fix it up while we're
>   there; done.
> 
> I agree too, but "indent -linux" likes "static" on a line by itself, and
> prefers it over "static void".  This is not the first time I've disagreed
> with it, although I use it a lot.

The linux styles of most indent-like programs are utter garbage.  If you
want Linux style use the scripts/Lindent script in the source tree.  It
still has a few quirks, like stupid ways to split overly long lines, but
generally does a reasonable job.

Re: [PATCH 1/6] nfsd4: split up nfsd_break_deleg_cb

[Jim Rees]

Christoph Hellwig wrote:

  The linux styles of most indent-like programs are utter garbage.  If you
  want Linux style use the scripts/Lindent script in the source tree.  It
  still has a few quirks, like stupid ways to split overly long lines, but
  generally does a reasonable job.

I'll give that a try, thanks.  Someone should fix "indent -linux", or at
least its man page.