Linux NFS development
 help / color / mirror / Atom feed
* [PATCH] NLM: Revert parts of commit 24e36663
@ 2008-09-24 18:50 Chuck Lever
       [not found] ` <20080924184732.4078.83334.stgit-lQeC5l55kZ7wdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
  0 siblings, 1 reply; 16+ messages in thread
From: Chuck Lever @ 2008-09-24 18:50 UTC (permalink / raw)
  To: trond.myklebust, bfields; +Cc: linux-nfs

statd and some servers always need lockd to listen on UDP, so always
start both lockd listener sockets.

This probably leaks an svc_serv if the TCP listener can't be created,
but it will do for now.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

I currently have this workaround lurking in my git repo.  Is there some kind
of fix for this issue planned for 2.6.28?

This is only a reminder, not a suggested patch.

 fs/lockd/svc.c |   31 +++++++++++++------------------
 1 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index a7b604c..36b6d03 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -213,25 +213,23 @@ lockd(void *vrqstp)
 }
 
 /*
- * Make any sockets that are needed but not present.
- * If nlm_udpport or nlm_tcpport were set as module
- * options, make those sockets unconditionally
+ * Create UDP and TCP listeners for lockd.  Even if we only
+ * have TCP NFS mounts and TCP NFSDs, some services (such
+ * as rpc.statd) still require UDP.
  */
-static int make_socks(struct svc_serv *serv, const unsigned short proto)
+static int make_socks(struct svc_serv *serv)
 {
 	static int warned;
 	struct svc_xprt *xprt;
 	int err = 0;
 
-	if (proto == IPPROTO_UDP || nlm_udpport) {
-		xprt = svc_find_xprt(serv, "udp", 0, 0);
-		if (!xprt)
-			err = svc_create_xprt(serv, "udp", nlm_udpport,
-					      SVC_SOCK_DEFAULTS);
-		else
-			svc_xprt_put(xprt);
-	}
-	if (err >= 0 && (proto == IPPROTO_TCP || nlm_tcpport)) {
+	xprt = svc_find_xprt(serv, "udp", 0, 0);
+	if (!xprt)
+		err = svc_create_xprt(serv, "udp", nlm_udpport,
+				      SVC_SOCK_DEFAULTS);
+	else
+		svc_xprt_put(xprt);
+	if (err >= 0) {
 		xprt = svc_find_xprt(serv, "tcp", 0, 0);
 		if (!xprt)
 			err = svc_create_xprt(serv, "tcp", nlm_tcpport,
@@ -260,11 +258,8 @@ int lockd_up(const unsigned short proto)
 	/*
 	 * Check whether we're already up and running.
 	 */
-	if (nlmsvc_rqst) {
-		if (proto)
-			error = make_socks(nlmsvc_rqst->rq_server, proto);
+	if (nlmsvc_rqst)
 		goto out;
-	}
 
 	/*
 	 * Sanity check: if there's no pid,
@@ -281,7 +276,7 @@ int lockd_up(const unsigned short proto)
 		goto out;
 	}
 
-	if ((error = make_socks(serv, proto)) < 0)
+	if ((error = make_socks(serv)) < 0)
 		goto destroy_and_out;
 
 	/*


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
       [not found] ` <20080924184732.4078.83334.stgit-lQeC5l55kZ7wdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
@ 2008-09-24 18:54   ` Trond Myklebust
  2008-09-26 15:50     ` Talpey, Thomas
  2008-09-25 21:38   ` J. Bruce Fields
  1 sibling, 1 reply; 16+ messages in thread
From: Trond Myklebust @ 2008-09-24 18:54 UTC (permalink / raw)
  To: Chuck Lever; +Cc: bfields, linux-nfs

On Wed, 2008-09-24 at 14:50 -0400, Chuck Lever wrote:
> statd and some servers always need lockd to listen on UDP, so always
> start both lockd listener sockets.
> 
> This probably leaks an svc_serv if the TCP listener can't be created,
> but it will do for now.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

ACK. We need this also in order to deal with servers that only send NLM
callbacks on UDP (yeah, I know that's lame...).

Trond

> ---
> 
> I currently have this workaround lurking in my git repo.  Is there some kind
> of fix for this issue planned for 2.6.28?
> 
> This is only a reminder, not a suggested patch.
> 
>  fs/lockd/svc.c |   31 +++++++++++++------------------
>  1 files changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index a7b604c..36b6d03 100644
> --- a/fs/lockd/svc.c
> +++ b/fs/lockd/svc.c
> @@ -213,25 +213,23 @@ lockd(void *vrqstp)
>  }
>  
>  /*
> - * Make any sockets that are needed but not present.
> - * If nlm_udpport or nlm_tcpport were set as module
> - * options, make those sockets unconditionally
> + * Create UDP and TCP listeners for lockd.  Even if we only
> + * have TCP NFS mounts and TCP NFSDs, some services (such
> + * as rpc.statd) still require UDP.
>   */
> -static int make_socks(struct svc_serv *serv, const unsigned short proto)
> +static int make_socks(struct svc_serv *serv)
>  {
>  	static int warned;
>  	struct svc_xprt *xprt;
>  	int err = 0;
>  
> -	if (proto == IPPROTO_UDP || nlm_udpport) {
> -		xprt = svc_find_xprt(serv, "udp", 0, 0);
> -		if (!xprt)
> -			err = svc_create_xprt(serv, "udp", nlm_udpport,
> -					      SVC_SOCK_DEFAULTS);
> -		else
> -			svc_xprt_put(xprt);
> -	}
> -	if (err >= 0 && (proto == IPPROTO_TCP || nlm_tcpport)) {
> +	xprt = svc_find_xprt(serv, "udp", 0, 0);
> +	if (!xprt)
> +		err = svc_create_xprt(serv, "udp", nlm_udpport,
> +				      SVC_SOCK_DEFAULTS);
> +	else
> +		svc_xprt_put(xprt);
> +	if (err >= 0) {
>  		xprt = svc_find_xprt(serv, "tcp", 0, 0);
>  		if (!xprt)
>  			err = svc_create_xprt(serv, "tcp", nlm_tcpport,
> @@ -260,11 +258,8 @@ int lockd_up(const unsigned short proto)
>  	/*
>  	 * Check whether we're already up and running.
>  	 */
> -	if (nlmsvc_rqst) {
> -		if (proto)
> -			error = make_socks(nlmsvc_rqst->rq_server, proto);
> +	if (nlmsvc_rqst)
>  		goto out;
> -	}
>  
>  	/*
>  	 * Sanity check: if there's no pid,
> @@ -281,7 +276,7 @@ int lockd_up(const unsigned short proto)
>  		goto out;
>  	}
>  
> -	if ((error = make_socks(serv, proto)) < 0)
> +	if ((error = make_socks(serv)) < 0)
>  		goto destroy_and_out;
>  
>  	/*
> 
-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
       [not found] ` <20080924184732.4078.83334.stgit-lQeC5l55kZ7wdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
  2008-09-24 18:54   ` Trond Myklebust
@ 2008-09-25 21:38   ` J. Bruce Fields
  2008-09-26 17:53     ` Chuck Lever
  1 sibling, 1 reply; 16+ messages in thread
From: J. Bruce Fields @ 2008-09-25 21:38 UTC (permalink / raw)
  To: Chuck Lever; +Cc: trond.myklebust, linux-nfs

On Wed, Sep 24, 2008 at 02:50:36PM -0400, Chuck Lever wrote:
> statd and some servers always need lockd to listen on UDP, so always
> start both lockd listener sockets.
> 
> This probably leaks an svc_serv if the TCP listener can't be created,
> but it will do for now.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
> I currently have this workaround lurking in my git repo.  Is there some kind
> of fix for this issue planned for 2.6.28?

Not that I know of.

> This is only a reminder, not a suggested patch.

Any chance you could generate a patch?

--b.

> 
>  fs/lockd/svc.c |   31 +++++++++++++------------------
>  1 files changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index a7b604c..36b6d03 100644
> --- a/fs/lockd/svc.c
> +++ b/fs/lockd/svc.c
> @@ -213,25 +213,23 @@ lockd(void *vrqstp)
>  }
>  
>  /*
> - * Make any sockets that are needed but not present.
> - * If nlm_udpport or nlm_tcpport were set as module
> - * options, make those sockets unconditionally
> + * Create UDP and TCP listeners for lockd.  Even if we only
> + * have TCP NFS mounts and TCP NFSDs, some services (such
> + * as rpc.statd) still require UDP.
>   */
> -static int make_socks(struct svc_serv *serv, const unsigned short proto)
> +static int make_socks(struct svc_serv *serv)
>  {
>  	static int warned;
>  	struct svc_xprt *xprt;
>  	int err = 0;
>  
> -	if (proto == IPPROTO_UDP || nlm_udpport) {
> -		xprt = svc_find_xprt(serv, "udp", 0, 0);
> -		if (!xprt)
> -			err = svc_create_xprt(serv, "udp", nlm_udpport,
> -					      SVC_SOCK_DEFAULTS);
> -		else
> -			svc_xprt_put(xprt);
> -	}
> -	if (err >= 0 && (proto == IPPROTO_TCP || nlm_tcpport)) {
> +	xprt = svc_find_xprt(serv, "udp", 0, 0);
> +	if (!xprt)
> +		err = svc_create_xprt(serv, "udp", nlm_udpport,
> +				      SVC_SOCK_DEFAULTS);
> +	else
> +		svc_xprt_put(xprt);
> +	if (err >= 0) {
>  		xprt = svc_find_xprt(serv, "tcp", 0, 0);
>  		if (!xprt)
>  			err = svc_create_xprt(serv, "tcp", nlm_tcpport,
> @@ -260,11 +258,8 @@ int lockd_up(const unsigned short proto)
>  	/*
>  	 * Check whether we're already up and running.
>  	 */
> -	if (nlmsvc_rqst) {
> -		if (proto)
> -			error = make_socks(nlmsvc_rqst->rq_server, proto);
> +	if (nlmsvc_rqst)
>  		goto out;
> -	}
>  
>  	/*
>  	 * Sanity check: if there's no pid,
> @@ -281,7 +276,7 @@ int lockd_up(const unsigned short proto)
>  		goto out;
>  	}
>  
> -	if ((error = make_socks(serv, proto)) < 0)
> +	if ((error = make_socks(serv)) < 0)
>  		goto destroy_and_out;
>  
>  	/*
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-24 18:54   ` Trond Myklebust
@ 2008-09-26 15:50     ` Talpey, Thomas
  0 siblings, 0 replies; 16+ messages in thread
From: Talpey, Thomas @ 2008-09-26 15:50 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Chuck Lever, bfields, linux-nfs

At 02:54 PM 9/24/2008, Trond Myklebust wrote:
>On Wed, 2008-09-24 at 14:50 -0400, Chuck Lever wrote:
>> statd and some servers always need lockd to listen on UDP, so always
>> start both lockd listener sockets.
>> 
>> This probably leaks an svc_serv if the TCP listener can't be created,
>> but it will do for now.
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>
>ACK. We need this also in order to deal with servers that only send NLM
>callbacks on UDP (yeah, I know that's lame...).

Lame it may be, but performing a single callback over TCP is expensive,
and if it's frequent enough, can lead to socket exhaustion from TIME_WAIT.

Thumbs-up for this patch. The worst thing about the unpatched issue is that
if the server tries UDP callbacks for a TCP mount, then the client never knows
the lock was available. It enters a polling loop and many races can occur.

I posted some patches to that a while back (when we used sourceforge),
they're not a prerequisite to this change, but perhaps relevant for hardening
purposes:

<http://sourceforge.net/mailarchive/forum.php?thread_name=EXNANE01OSQeSOzNCWb00000734%40exnane01.hq.netapp.com&forum_name=nfs>


Tom.

>
>Trond
>
>> ---
>> 
>> I currently have this workaround lurking in my git repo.  Is there some kind
>> of fix for this issue planned for 2.6.28?
>> 
>> This is only a reminder, not a suggested patch.
>> 
>>  fs/lockd/svc.c |   31 +++++++++++++------------------
>>  1 files changed, 13 insertions(+), 18 deletions(-)
>> 
>> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
>> index a7b604c..36b6d03 100644
>> --- a/fs/lockd/svc.c
>> +++ b/fs/lockd/svc.c
>> @@ -213,25 +213,23 @@ lockd(void *vrqstp)
>>  }
>>  
>>  /*
>> - * Make any sockets that are needed but not present.
>> - * If nlm_udpport or nlm_tcpport were set as module
>> - * options, make those sockets unconditionally
>> + * Create UDP and TCP listeners for lockd.  Even if we only
>> + * have TCP NFS mounts and TCP NFSDs, some services (such
>> + * as rpc.statd) still require UDP.
>>   */
>> -static int make_socks(struct svc_serv *serv, const unsigned short proto)
>> +static int make_socks(struct svc_serv *serv)
>>  {
>>  	static int warned;
>>  	struct svc_xprt *xprt;
>>  	int err = 0;
>>  
>> -	if (proto == IPPROTO_UDP || nlm_udpport) {
>> -		xprt = svc_find_xprt(serv, "udp", 0, 0);
>> -		if (!xprt)
>> -			err = svc_create_xprt(serv, "udp", nlm_udpport,
>> -					      SVC_SOCK_DEFAULTS);
>> -		else
>> -			svc_xprt_put(xprt);
>> -	}
>> -	if (err >= 0 && (proto == IPPROTO_TCP || nlm_tcpport)) {
>> +	xprt = svc_find_xprt(serv, "udp", 0, 0);
>> +	if (!xprt)
>> +		err = svc_create_xprt(serv, "udp", nlm_udpport,
>> +				      SVC_SOCK_DEFAULTS);
>> +	else
>> +		svc_xprt_put(xprt);
>> +	if (err >= 0) {
>>  		xprt = svc_find_xprt(serv, "tcp", 0, 0);
>>  		if (!xprt)
>>  			err = svc_create_xprt(serv, "tcp", nlm_tcpport,
>> @@ -260,11 +258,8 @@ int lockd_up(const unsigned short proto)
>>  	/*
>>  	* Check whether we're already up and running.
>>  	*/
>> -	if (nlmsvc_rqst) {
>> -		if (proto)
>> -			error = make_socks(nlmsvc_rqst->rq_server, proto);
>> +	if (nlmsvc_rqst)
>>  		goto out;
>> -	}
>>  
>>  	/*
>>  	* Sanity check: if there's no pid,
>> @@ -281,7 +276,7 @@ int lockd_up(const unsigned short proto)
>>  		goto out;
>>  	}
>>  
>> -	if ((error = make_socks(serv, proto)) < 0)
>> +	if ((error = make_socks(serv)) < 0)
>>  		goto destroy_and_out;
>>  
>>  	/*
>> 
>-- 
>Trond Myklebust
>Linux NFS client maintainer
>
>NetApp
>Trond.Myklebust@netapp.com
>www.netapp.com
>--
>To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-25 21:38   ` J. Bruce Fields
@ 2008-09-26 17:53     ` Chuck Lever
  2008-09-26 18:45       ` J. Bruce Fields
  0 siblings, 1 reply; 16+ messages in thread
From: Chuck Lever @ 2008-09-26 17:53 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: trond.myklebust, linux-nfs

On Sep 25, 2008, at Sep 25, 2008, 5:38 PM, J. Bruce Fields wrote:
> On Wed, Sep 24, 2008 at 02:50:36PM -0400, Chuck Lever wrote:
>> statd and some servers always need lockd to listen on UDP, so always
>> start both lockd listener sockets.
>>
>> This probably leaks an svc_serv if the TCP listener can't be created,
>> but it will do for now.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>
>> I currently have this workaround lurking in my git repo.  Is there  
>> some kind
>> of fix for this issue planned for 2.6.28?
>
> Not that I know of.
>
>> This is only a reminder, not a suggested patch.
>
> Any chance you could generate a patch?

I can, but no guarantee it will be in time for the 2.6.28 merge window.

Would we consider this a regression fix?  If so, we could post it  
after the merge window closes.  It would give a little more time to do  
some testing.

>> fs/lockd/svc.c |   31 +++++++++++++------------------
>> 1 files changed, 13 insertions(+), 18 deletions(-)
>>
>> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
>> index a7b604c..36b6d03 100644
>> --- a/fs/lockd/svc.c
>> +++ b/fs/lockd/svc.c
>> @@ -213,25 +213,23 @@ lockd(void *vrqstp)
>> }
>>
>> /*
>> - * Make any sockets that are needed but not present.
>> - * If nlm_udpport or nlm_tcpport were set as module
>> - * options, make those sockets unconditionally
>> + * Create UDP and TCP listeners for lockd.  Even if we only
>> + * have TCP NFS mounts and TCP NFSDs, some services (such
>> + * as rpc.statd) still require UDP.
>>  */
>> -static int make_socks(struct svc_serv *serv, const unsigned short  
>> proto)
>> +static int make_socks(struct svc_serv *serv)
>> {
>> 	static int warned;
>> 	struct svc_xprt *xprt;
>> 	int err = 0;
>>
>> -	if (proto == IPPROTO_UDP || nlm_udpport) {
>> -		xprt = svc_find_xprt(serv, "udp", 0, 0);
>> -		if (!xprt)
>> -			err = svc_create_xprt(serv, "udp", nlm_udpport,
>> -					      SVC_SOCK_DEFAULTS);
>> -		else
>> -			svc_xprt_put(xprt);
>> -	}
>> -	if (err >= 0 && (proto == IPPROTO_TCP || nlm_tcpport)) {
>> +	xprt = svc_find_xprt(serv, "udp", 0, 0);
>> +	if (!xprt)
>> +		err = svc_create_xprt(serv, "udp", nlm_udpport,
>> +				      SVC_SOCK_DEFAULTS);
>> +	else
>> +		svc_xprt_put(xprt);
>> +	if (err >= 0) {
>> 		xprt = svc_find_xprt(serv, "tcp", 0, 0);
>> 		if (!xprt)
>> 			err = svc_create_xprt(serv, "tcp", nlm_tcpport,
>> @@ -260,11 +258,8 @@ int lockd_up(const unsigned short proto)
>> 	/*
>> 	 * Check whether we're already up and running.
>> 	 */
>> -	if (nlmsvc_rqst) {
>> -		if (proto)
>> -			error = make_socks(nlmsvc_rqst->rq_server, proto);
>> +	if (nlmsvc_rqst)
>> 		goto out;
>> -	}
>>
>> 	/*
>> 	 * Sanity check: if there's no pid,
>> @@ -281,7 +276,7 @@ int lockd_up(const unsigned short proto)
>> 		goto out;
>> 	}
>>
>> -	if ((error = make_socks(serv, proto)) < 0)
>> +	if ((error = make_socks(serv)) < 0)
>> 		goto destroy_and_out;
>>
>> 	/*
>>

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-26 17:53     ` Chuck Lever
@ 2008-09-26 18:45       ` J. Bruce Fields
  2008-09-26 18:49         ` Trond Myklebust
  0 siblings, 1 reply; 16+ messages in thread
From: J. Bruce Fields @ 2008-09-26 18:45 UTC (permalink / raw)
  To: Chuck Lever; +Cc: trond.myklebust, linux-nfs

On Fri, Sep 26, 2008 at 01:53:58PM -0400, Chuck Lever wrote:
> On Sep 25, 2008, at Sep 25, 2008, 5:38 PM, J. Bruce Fields wrote:
>> On Wed, Sep 24, 2008 at 02:50:36PM -0400, Chuck Lever wrote:
>>> statd and some servers always need lockd to listen on UDP, so always
>>> start both lockd listener sockets.
>>>
>>> This probably leaks an svc_serv if the TCP listener can't be created,
>>> but it will do for now.
>>>
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> ---
>>>
>>> I currently have this workaround lurking in my git repo.  Is there  
>>> some kind
>>> of fix for this issue planned for 2.6.28?
>>
>> Not that I know of.
>>
>>> This is only a reminder, not a suggested patch.
>>
>> Any chance you could generate a patch?
>
> I can, but no guarantee it will be in time for the 2.6.28 merge window.
>
> Would we consider this a regression fix?

I suppose so.  (Are there any bug reports?  If so, could we get URL's
for them into the commit message?)

> If so, we could post it after 
> the merge window closes.  It would give a little more time to do some 
> testing.

That sound suspiciously like "since this is a high priority, it will be
accepted later, so we can make it a lower priority."  Help!

But anyway, yes, we might have more time.

--b.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-26 18:45       ` J. Bruce Fields
@ 2008-09-26 18:49         ` Trond Myklebust
  2008-09-26 18:51           ` J. Bruce Fields
  2008-09-26 19:04           ` Chuck Lever
  0 siblings, 2 replies; 16+ messages in thread
From: Trond Myklebust @ 2008-09-26 18:49 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Chuck Lever, linux-nfs

On Fri, 2008-09-26 at 14:45 -0400, J. Bruce Fields wrote:
> > If so, we could post it after 
> > the merge window closes.  It would give a little more time to do some 
> > testing.
> 
> That sound suspiciously like "since this is a high priority, it will be
> accepted later, so we can make it a lower priority."  Help!
> 
> But anyway, yes, we might have more time.

At the Kernel Summit Linus said he wants this kind of fix to go into the
merge window rather than in the -rc series if possible. His position was
that he'd rather revert a bad fix instead of delaying the testing in the
mainline kernel...

Trond

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-26 18:49         ` Trond Myklebust
@ 2008-09-26 18:51           ` J. Bruce Fields
  2008-09-26 19:04           ` Chuck Lever
  1 sibling, 0 replies; 16+ messages in thread
From: J. Bruce Fields @ 2008-09-26 18:51 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Chuck Lever, linux-nfs

On Fri, Sep 26, 2008 at 02:49:35PM -0400, Trond Myklebust wrote:
> On Fri, 2008-09-26 at 14:45 -0400, J. Bruce Fields wrote:
> > > If so, we could post it after 
> > > the merge window closes.  It would give a little more time to do some 
> > > testing.
> > 
> > That sound suspiciously like "since this is a high priority, it will be
> > accepted later, so we can make it a lower priority."  Help!
> > 
> > But anyway, yes, we might have more time.
> 
> At the Kernel Summit Linus said he wants this kind of fix to go into the
> merge window rather than in the -rc series if possible. His position was
> that he'd rather revert a bad fix instead of delaying the testing in the
> mainline kernel...

Makes sense to me.

--b.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-26 18:49         ` Trond Myklebust
  2008-09-26 18:51           ` J. Bruce Fields
@ 2008-09-26 19:04           ` Chuck Lever
  2008-09-26 19:11             ` Trond Myklebust
  1 sibling, 1 reply; 16+ messages in thread
From: Chuck Lever @ 2008-09-26 19:04 UTC (permalink / raw)
  To: Trond Myklebust, J. Bruce Fields; +Cc: Linux NFS Mailing List

On Sep 26, 2008, at Sep 26, 2008, 2:49 PM, Trond Myklebust wrote:
> On Fri, 2008-09-26 at 14:45 -0400, J. Bruce Fields wrote:
>>> If so, we could post it after
>>> the merge window closes.  It would give a little more time to do  
>>> some
>>> testing.
>>
>> That sound suspiciously like "since this is a high priority, it  
>> will be
>> accepted later, so we can make it a lower priority."  Help!

No, more like "I have way too much to do in the next week because I'm  
out two days, and am required to finish an ethics course before the  
30th."  I don't think there's much to do to get this in shape, I just  
don't want to make promises I can't keep.

>> But anyway, yes, we might have more time.
>
> At the Kernel Summit Linus said he wants this kind of fix to go into  
> the
> merge window rather than in the -rc series if possible. His position  
> was
> that he'd rather revert a bad fix instead of delaying the testing in  
> the
> mainline kernel...

So in light of Linus' stated position, it would be helpful for me (and  
maybe others on the list) to have a good feel for when you guys are  
planning to push your merge-window changes to Linus before each window  
opens.

At the very least, since many of us do not follow LKML, would someone  
commit to reflecting the LKML merge announcements to linux-nfs?  Or is  
there a kernel announcements list we can follow (or that we can  
subscribe linux-nfs to)?

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-26 19:04           ` Chuck Lever
@ 2008-09-26 19:11             ` Trond Myklebust
  2008-09-26 19:44               ` J. Bruce Fields
  0 siblings, 1 reply; 16+ messages in thread
From: Trond Myklebust @ 2008-09-26 19:11 UTC (permalink / raw)
  To: Chuck Lever; +Cc: J. Bruce Fields, Linux NFS Mailing List

On Fri, 2008-09-26 at 15:04 -0400, Chuck Lever wrote:
> 
> So in light of Linus' stated position, it would be helpful for me (and  
> maybe others on the list) to have a good feel for when you guys are  
> planning to push your merge-window changes to Linus before each window  
> opens.
> 
> At the very least, since many of us do not follow LKML, would someone  
> commit to reflecting the LKML merge announcements to linux-nfs?  Or is  
> there a kernel announcements list we can follow (or that we can  
> subscribe linux-nfs to)?

There are no announcements. The merge window starts as soon as 2.6.27 is
released, and lasts for 2 weeks. There has been some talk about
shortening it to 1 week, but that hasn't been fully decided yet.

IOW: Assume as soon as you see the 2.6.27 announcement, that we're
preparing the merge.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-26 19:11             ` Trond Myklebust
@ 2008-09-26 19:44               ` J. Bruce Fields
  2008-09-26 19:54                 ` Chuck Lever
  0 siblings, 1 reply; 16+ messages in thread
From: J. Bruce Fields @ 2008-09-26 19:44 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Chuck Lever, Linux NFS Mailing List

On Fri, Sep 26, 2008 at 03:11:30PM -0400, Trond Myklebust wrote:
> On Fri, 2008-09-26 at 15:04 -0400, Chuck Lever wrote:
> > 
> > So in light of Linus' stated position, it would be helpful for me (and  
> > maybe others on the list) to have a good feel for when you guys are  
> > planning to push your merge-window changes to Linus before each window  
> > opens.
> > 
> > At the very least, since many of us do not follow LKML, would someone  
> > commit to reflecting the LKML merge announcements to linux-nfs?  Or is  
> > there a kernel announcements list we can follow (or that we can  
> > subscribe linux-nfs to)?
> 
> There are no announcements. The merge window starts as soon as 2.6.27 is
> released, and lasts for 2 weeks. There has been some talk about
> shortening it to 1 week, but that hasn't been fully decided yet.
> 
> IOW: Assume as soon as you see the 2.6.27 announcement, that we're
> preparing the merge.

Yeah.  Unfortunately I don't know of a simple way to get just the
release announcements.  I guess lwn.net's rss feed would get them with a
little less noise than lkml.

We should probably restart sending those "what's in my tree for 2.6.x"
messages with the tentative shortlog for the pull.  I think I did it a
couple times and then got lazy....

--b.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-26 19:44               ` J. Bruce Fields
@ 2008-09-26 19:54                 ` Chuck Lever
  2008-09-26 19:57                   ` Trond Myklebust
  0 siblings, 1 reply; 16+ messages in thread
From: Chuck Lever @ 2008-09-26 19:54 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Trond Myklebust, Linux NFS Mailing List

On Sep 26, 2008, at Sep 26, 2008, 3:44 PM, J. Bruce Fields wrote:
> On Fri, Sep 26, 2008 at 03:11:30PM -0400, Trond Myklebust wrote:
>> On Fri, 2008-09-26 at 15:04 -0400, Chuck Lever wrote:
>>>
>>> So in light of Linus' stated position, it would be helpful for me  
>>> (and
>>> maybe others on the list) to have a good feel for when you guys are
>>> planning to push your merge-window changes to Linus before each  
>>> window
>>> opens.
>>>
>>> At the very least, since many of us do not follow LKML, would  
>>> someone
>>> commit to reflecting the LKML merge announcements to linux-nfs?   
>>> Or is
>>> there a kernel announcements list we can follow (or that we can
>>> subscribe linux-nfs to)?
>>
>> There are no announcements. The merge window starts as soon as  
>> 2.6.27 is
>> released, and lasts for 2 weeks. There has been some talk about
>> shortening it to 1 week, but that hasn't been fully decided yet.
>>
>> IOW: Assume as soon as you see the 2.6.27 announcement, that we're
>> preparing the merge.
>
> Yeah.  Unfortunately I don't know of a simple way to get just the
> release announcements.  I guess lwn.net's rss feed would get them  
> with a
> little less noise than lkml.
>
> We should probably restart sending those "what's in my tree for 2.6.x"
> messages with the tentative shortlog for the pull.  I think I did it a
> couple times and then got lazy....

The whole system makes it absolutely impossible to plan features and  
bug fixes.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-26 19:54                 ` Chuck Lever
@ 2008-09-26 19:57                   ` Trond Myklebust
  2008-09-26 21:11                     ` Chuck Lever
  0 siblings, 1 reply; 16+ messages in thread
From: Trond Myklebust @ 2008-09-26 19:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: J. Bruce Fields, Linux NFS Mailing List

On Fri, 2008-09-26 at 15:54 -0400, Chuck Lever wrote:
> The whole system makes it absolutely impossible to plan features and  
> bug fixes.

The only thing is makes impossible is planning features for a
_particular_ kernel revision. You shouldn't be doing that anyway...

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-26 19:57                   ` Trond Myklebust
@ 2008-09-26 21:11                     ` Chuck Lever
  2008-09-26 22:22                       ` Trond Myklebust
  2008-09-26 23:30                       ` J. Bruce Fields
  0 siblings, 2 replies; 16+ messages in thread
From: Chuck Lever @ 2008-09-26 21:11 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: J. Bruce Fields, Linux NFS Mailing List

On Sep 26, 2008, at Sep 26, 2008, 3:57 PM, Trond Myklebust wrote:
> On Fri, 2008-09-26 at 15:54 -0400, Chuck Lever wrote:
>> The whole system makes it absolutely impossible to plan features and
>> bug fixes.
>
> The only thing it makes impossible is planning features for a
> _particular_ kernel revision.

I find it impossible to plan for a particular _year_ (or even two).

The problem is _you_, the subsystem maintainer, have a merge window  
too.  I find it challenging to align my patch submissions with two  
sliding merge windows that open and close at arbitrary times without  
any warning whatever.  And now I have to worry about Bruce's merge  
window as well....

Plus, the submission guidelines keep changing.  FreeBSD has an amazing  
guide to kernel development rules and conventions posted on the web.   
I wish Linux had the same.

If I know in advance when you (and Bruce) are taking new features or  
bug fixes for each kernel release, that would be helpful.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-26 21:11                     ` Chuck Lever
@ 2008-09-26 22:22                       ` Trond Myklebust
  2008-09-26 23:30                       ` J. Bruce Fields
  1 sibling, 0 replies; 16+ messages in thread
From: Trond Myklebust @ 2008-09-26 22:22 UTC (permalink / raw)
  To: Chuck Lever; +Cc: J. Bruce Fields, Linux NFS Mailing List

On Fri, 2008-09-26 at 17:11 -0400, Chuck Lever wrote:
> If I know in advance when you (and Bruce) are taking new features or  
> bug fixes for each kernel release, that would be helpful.

That won't happen.

Send it when it is _ready_. That is the way everyone else works when
doing Linux kernel development...

If it needs to go into a particular kernel release, either because it is
a bugfix for a known problem, or because other patches depend upon it,
then make a special case for that.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] NLM: Revert parts of commit 24e36663
  2008-09-26 21:11                     ` Chuck Lever
  2008-09-26 22:22                       ` Trond Myklebust
@ 2008-09-26 23:30                       ` J. Bruce Fields
  1 sibling, 0 replies; 16+ messages in thread
From: J. Bruce Fields @ 2008-09-26 23:30 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Trond Myklebust, Linux NFS Mailing List

On Fri, Sep 26, 2008 at 05:11:25PM -0400, Chuck Lever wrote:
> On Sep 26, 2008, at Sep 26, 2008, 3:57 PM, Trond Myklebust wrote:
>> On Fri, 2008-09-26 at 15:54 -0400, Chuck Lever wrote:
>>> The whole system makes it absolutely impossible to plan features and
>>> bug fixes.
>>
>> The only thing it makes impossible is planning features for a
>> _particular_ kernel revision.
>
> I find it impossible to plan for a particular _year_ (or even two).
>
> The problem is _you_, the subsystem maintainer, have a merge window too.  
> I find it challenging to align my patch submissions with two sliding 
> merge windows that open and close at arbitrary times without any warning 
> whatever.  And now I have to worry about Bruce's merge window as well....

Please just send them in whenever they're ready--I mean to take patches
at any time, and then decide separately whether they should be queued up
for the current version, the next one, and/or the previous (stable)
version.  There are inevitably delays (especially with vacations and
such), but normally I'll try to at least respond with an estimate if
it's going to take longer than a couple days.

> Plus, the submission guidelines keep changing.  FreeBSD has an amazing  
> guide to kernel development rules and conventions posted on the web.  I 
> wish Linux had the same.

Do you have a pointer to it, and/or suggestions how it could be used to
improve existing linux documentation?

There's Documentation/SubmittingPatches and stuff.  My main gripe with
Documentation/ is that people contribute to it, but there's no
maintainer to say "no" or to keep things organized, so it tends to get
large and messy over time.

In any case lkml would be a better forum for this.

> If I know in advance when you (and Bruce) are taking new features or bug 
> fixes for each kernel release, that would be helpful.

I'd like to say the answer is "any time", barring occasional
interruptions.

Or if the problem is that you've got stuff that you're basically done
with, but that you know you *might* get the chance to take one more look
at if you had another week--I think it'd be better if people just sent
stuff in when they think it's good enough, and if we later find a
problem we'll fix it.

--b.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2008-09-26 23:31 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-24 18:50 [PATCH] NLM: Revert parts of commit 24e36663 Chuck Lever
     [not found] ` <20080924184732.4078.83334.stgit-lQeC5l55kZ7wdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
2008-09-24 18:54   ` Trond Myklebust
2008-09-26 15:50     ` Talpey, Thomas
2008-09-25 21:38   ` J. Bruce Fields
2008-09-26 17:53     ` Chuck Lever
2008-09-26 18:45       ` J. Bruce Fields
2008-09-26 18:49         ` Trond Myklebust
2008-09-26 18:51           ` J. Bruce Fields
2008-09-26 19:04           ` Chuck Lever
2008-09-26 19:11             ` Trond Myklebust
2008-09-26 19:44               ` J. Bruce Fields
2008-09-26 19:54                 ` Chuck Lever
2008-09-26 19:57                   ` Trond Myklebust
2008-09-26 21:11                     ` Chuck Lever
2008-09-26 22:22                       ` Trond Myklebust
2008-09-26 23:30                       ` J. Bruce Fields

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox