[PATCH] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[PATCH] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Zhang Yanfei]

Since the type of netns_ipvs->sysctl_sync_qlen_max has been changed to
unsigned long, type of its related proc var sync_qlen_max should be changed
to unsigned long, too. Also the return type of function sysctl_sync_qlen_max().

Cc: David Miller <davem@davemloft.net>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Simon Horman <horms@verge.net.au>
Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 include/net/ip_vs.h            |    4 ++--
 net/netfilter/ipvs/ip_vs_ctl.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 68c69d5..ba3bd85 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1052,7 +1052,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
 	return ACCESS_ONCE(ipvs->sysctl_sync_ports);
 }
 
-static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
+static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
 {
 	return ipvs->sysctl_sync_qlen_max;
 }
@@ -1099,7 +1099,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
 	return 1;
 }
 
-static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
+static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
 {
 	return IPVS_SYNC_QLEN_MAX;
 }
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index ec664cb..d79a530 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1747,9 +1747,9 @@ static struct ctl_table vs_vars[] = {
 	},
 	{
 		.procname	= "sync_qlen_max",
-		.maxlen		= sizeof(int),
+		.maxlen		= sizeof(unsigned long),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_doulongvec_minmax,
 	},
 	{
 		.procname	= "sync_sock_size",
-- 
1.7.1

[PATCH] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Zhang Yanfei]

Since the type of netns_ipvs->sysctl_sync_qlen_max has been changed to
unsigned long, type of its related proc var sync_qlen_max should be changed
to unsigned long, too. Also the return type of function sysctl_sync_qlen_max().

Cc: David Miller <davem@davemloft.net>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Simon Horman <horms@verge.net.au>
Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 include/net/ip_vs.h            |    4 ++--
 net/netfilter/ipvs/ip_vs_ctl.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 68c69d5..ba3bd85 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1052,7 +1052,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
 	return ACCESS_ONCE(ipvs->sysctl_sync_ports);
 }
 
-static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
+static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
 {
 	return ipvs->sysctl_sync_qlen_max;
 }
@@ -1099,7 +1099,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
 	return 1;
 }
 
-static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
+static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
 {
 	return IPVS_SYNC_QLEN_MAX;
 }
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index ec664cb..d79a530 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1747,9 +1747,9 @@ static struct ctl_table vs_vars[] = {
 	},
 	{
 		.procname	= "sync_qlen_max",
-		.maxlen		= sizeof(int),
+		.maxlen		= sizeof(unsigned long),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_doulongvec_minmax,
 	},
 	{
 		.procname	= "sync_sock_size",
-- 
1.7.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Zhang Yanfei]

Since the type of netns_ipvs->sysctl_sync_qlen_max has been changed to
unsigned long, type of its related proc var sync_qlen_max should be changed
to unsigned long, too. Also the return type of function sysctl_sync_qlen_max().

Besides, the type of ipvs_master_sync_state->sync_queue_len should also be
changed to unsigned long.

Changelog from V1:
- change type of ipvs_master_sync_state->sync_queue_len to unsigned long
  as Simon addressed.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Miller <davem@davemloft.net>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Simon Horman <horms@verge.net.au>
Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 include/net/ip_vs.h            |    6 +++---
 net/netfilter/ipvs/ip_vs_ctl.c |    4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 68c69d5..1d56f92 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -874,7 +874,7 @@ struct ip_vs_app {
 struct ipvs_master_sync_state {
 	struct list_head	sync_queue;
 	struct ip_vs_sync_buff	*sync_buff;
-	int			sync_queue_len;
+	unsigned long		sync_queue_len;
 	unsigned int		sync_queue_delay;
 	struct task_struct	*master_thread;
 	struct delayed_work	master_wakeup_work;
@@ -1052,7 +1052,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
 	return ACCESS_ONCE(ipvs->sysctl_sync_ports);
 }
 
-static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
+static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
 {
 	return ipvs->sysctl_sync_qlen_max;
 }
@@ -1099,7 +1099,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
 	return 1;
 }
 
-static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
+static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
 {
 	return IPVS_SYNC_QLEN_MAX;
 }
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index ec664cb..d79a530 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1747,9 +1747,9 @@ static struct ctl_table vs_vars[] = {
 	},
 	{
 		.procname	= "sync_qlen_max",
-		.maxlen		= sizeof(int),
+		.maxlen		= sizeof(unsigned long),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_doulongvec_minmax,
 	},
 	{
 		.procname	= "sync_sock_size",
-- 
1.7.1

[PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Zhang Yanfei]

Since the type of netns_ipvs->sysctl_sync_qlen_max has been changed to
unsigned long, type of its related proc var sync_qlen_max should be changed
to unsigned long, too. Also the return type of function sysctl_sync_qlen_max().

Besides, the type of ipvs_master_sync_state->sync_queue_len should also be
changed to unsigned long.

Changelog from V1:
- change type of ipvs_master_sync_state->sync_queue_len to unsigned long
  as Simon addressed.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Miller <davem@davemloft.net>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Simon Horman <horms@verge.net.au>
Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
---
 include/net/ip_vs.h            |    6 +++---
 net/netfilter/ipvs/ip_vs_ctl.c |    4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 68c69d5..1d56f92 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -874,7 +874,7 @@ struct ip_vs_app {
 struct ipvs_master_sync_state {
 	struct list_head	sync_queue;
 	struct ip_vs_sync_buff	*sync_buff;
-	int			sync_queue_len;
+	unsigned long		sync_queue_len;
 	unsigned int		sync_queue_delay;
 	struct task_struct	*master_thread;
 	struct delayed_work	master_wakeup_work;
@@ -1052,7 +1052,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
 	return ACCESS_ONCE(ipvs->sysctl_sync_ports);
 }
 
-static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
+static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
 {
 	return ipvs->sysctl_sync_qlen_max;
 }
@@ -1099,7 +1099,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
 	return 1;
 }
 
-static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
+static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
 {
 	return IPVS_SYNC_QLEN_MAX;
 }
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index ec664cb..d79a530 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1747,9 +1747,9 @@ static struct ctl_table vs_vars[] = {
 	},
 	{
 		.procname	= "sync_qlen_max",
-		.maxlen		= sizeof(int),
+		.maxlen		= sizeof(unsigned long),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_doulongvec_minmax,
 	},
 	{
 		.procname	= "sync_sock_size",
-- 
1.7.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Simon Horman]

On Thu, Feb 07, 2013 at 12:15:18PM +0800, Zhang Yanfei wrote:
> Since the type of netns_ipvs->sysctl_sync_qlen_max has been changed to
> unsigned long, type of its related proc var sync_qlen_max should be changed
> to unsigned long, too. Also the return type of function sysctl_sync_qlen_max().
> 
> Besides, the type of ipvs_master_sync_state->sync_queue_len should also be
> changed to unsigned long.
> 
> Changelog from V1:
> - change type of ipvs_master_sync_state->sync_queue_len to unsigned long
>   as Simon addressed.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Miller <davem@davemloft.net>
> Cc: Julian Anastasov <ja@ssi.bg>
> Cc: Simon Horman <horms@verge.net.au>
> Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>

Acked-by: Simon Horman <horms@verge.net.au>

> ---
>  include/net/ip_vs.h            |    6 +++---
>  net/netfilter/ipvs/ip_vs_ctl.c |    4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 68c69d5..1d56f92 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -874,7 +874,7 @@ struct ip_vs_app {
>  struct ipvs_master_sync_state {
>  	struct list_head	sync_queue;
>  	struct ip_vs_sync_buff	*sync_buff;
> -	int			sync_queue_len;
> +	unsigned long		sync_queue_len;
>  	unsigned int		sync_queue_delay;
>  	struct task_struct	*master_thread;
>  	struct delayed_work	master_wakeup_work;
> @@ -1052,7 +1052,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>  	return ACCESS_ONCE(ipvs->sysctl_sync_ports);
>  }
>  
> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>  {
>  	return ipvs->sysctl_sync_qlen_max;
>  }
> @@ -1099,7 +1099,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>  	return 1;
>  }
>  
> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>  {
>  	return IPVS_SYNC_QLEN_MAX;
>  }
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index ec664cb..d79a530 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1747,9 +1747,9 @@ static struct ctl_table vs_vars[] = {
>  	},
>  	{
>  		.procname	= "sync_qlen_max",
> -		.maxlen		= sizeof(int),
> +		.maxlen		= sizeof(unsigned long),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> +		.proc_handler	= proc_doulongvec_minmax,
>  	},
>  	{
>  		.procname	= "sync_sock_size",
> -- 
> 1.7.1
> 

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Simon Horman]

On Thu, Feb 07, 2013 at 12:15:18PM +0800, Zhang Yanfei wrote:
> Since the type of netns_ipvs->sysctl_sync_qlen_max has been changed to
> unsigned long, type of its related proc var sync_qlen_max should be changed
> to unsigned long, too. Also the return type of function sysctl_sync_qlen_max().
> 
> Besides, the type of ipvs_master_sync_state->sync_queue_len should also be
> changed to unsigned long.
> 
> Changelog from V1:
> - change type of ipvs_master_sync_state->sync_queue_len to unsigned long
>   as Simon addressed.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Miller <davem@davemloft.net>
> Cc: Julian Anastasov <ja@ssi.bg>
> Cc: Simon Horman <horms@verge.net.au>
> Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>

Acked-by: Simon Horman <horms@verge.net.au>

> ---
>  include/net/ip_vs.h            |    6 +++---
>  net/netfilter/ipvs/ip_vs_ctl.c |    4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 68c69d5..1d56f92 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -874,7 +874,7 @@ struct ip_vs_app {
>  struct ipvs_master_sync_state {
>  	struct list_head	sync_queue;
>  	struct ip_vs_sync_buff	*sync_buff;
> -	int			sync_queue_len;
> +	unsigned long		sync_queue_len;
>  	unsigned int		sync_queue_delay;
>  	struct task_struct	*master_thread;
>  	struct delayed_work	master_wakeup_work;
> @@ -1052,7 +1052,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>  	return ACCESS_ONCE(ipvs->sysctl_sync_ports);
>  }
>  
> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>  {
>  	return ipvs->sysctl_sync_qlen_max;
>  }
> @@ -1099,7 +1099,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>  	return 1;
>  }
>  
> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>  {
>  	return IPVS_SYNC_QLEN_MAX;
>  }
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index ec664cb..d79a530 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1747,9 +1747,9 @@ static struct ctl_table vs_vars[] = {
>  	},
>  	{
>  		.procname	= "sync_qlen_max",
> -		.maxlen		= sizeof(int),
> +		.maxlen		= sizeof(unsigned long),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> +		.proc_handler	= proc_doulongvec_minmax,
>  	},
>  	{
>  		.procname	= "sync_sock_size",
> -- 
> 1.7.1
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Zhang Yanfei]

于 2013年02月07日 16:40, Julian Anastasov 写道:
> 
> 	Hello,
> 
> On Thu, 7 Feb 2013, Zhang Yanfei wrote:
> 
>> Since the type of netns_ipvs->sysctl_sync_qlen_max has been changed to
>> unsigned long, type of its related proc var sync_qlen_max should be changed
>> to unsigned long, too. Also the return type of function sysctl_sync_qlen_max().
>>
>> Besides, the type of ipvs_master_sync_state->sync_queue_len should also be
>> changed to unsigned long.
> 
> 	v2 looks fine. Thanks! Regarding your question
> see below...
> 
>> Changelog from V1:
>> - change type of ipvs_master_sync_state->sync_queue_len to unsigned long
>>   as Simon addressed.
>>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: David Miller <davem@davemloft.net>
>> Cc: Julian Anastasov <ja@ssi.bg>
>> Cc: Simon Horman <horms@verge.net.au>
>> Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
>> ---
>>  include/net/ip_vs.h            |    6 +++---
>>  net/netfilter/ipvs/ip_vs_ctl.c |    4 ++--
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
>> index 68c69d5..1d56f92 100644
>> --- a/include/net/ip_vs.h
>> +++ b/include/net/ip_vs.h
>> @@ -874,7 +874,7 @@ struct ip_vs_app {
>>  struct ipvs_master_sync_state {
>>  	struct list_head	sync_queue;
>>  	struct ip_vs_sync_buff	*sync_buff;
>> -	int			sync_queue_len;
>> +	unsigned long		sync_queue_len;
>>  	unsigned int		sync_queue_delay;
>>  	struct task_struct	*master_thread;
>>  	struct delayed_work	master_wakeup_work;
>> @@ -1052,7 +1052,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>>  	return ACCESS_ONCE(ipvs->sysctl_sync_ports);
>>  }
>>  
>> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>>  {
>>  	return ipvs->sysctl_sync_qlen_max;
>>  }
>> @@ -1099,7 +1099,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>>  	return 1;
>>  }
>>  
>> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>>  {
>>  	return IPVS_SYNC_QLEN_MAX;
>>  }
>> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
>> index ec664cb..d79a530 100644
>> --- a/net/netfilter/ipvs/ip_vs_ctl.c
>> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
>> @@ -1747,9 +1747,9 @@ static struct ctl_table vs_vars[] = {
>>  	},
>>  	{
>>  		.procname	= "sync_qlen_max",
>> -		.maxlen		= sizeof(int),
>> +		.maxlen		= sizeof(unsigned long),
>>  		.mode		= 0644,
>> -		.proc_handler	= proc_dointvec,
>> +		.proc_handler	= proc_doulongvec_minmax,
>>  	},
>>  	{
>>  		.procname	= "sync_sock_size",
>> -- 
>> 1.7.1
> 
> 
>> Another question about the sysctl_sync_qlen_max:
>> This variable is assigned as:
>>
>> ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
>>
>> The function nr_free_buffer_pages actually means: counts of pages
>> which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
>>
>> is it ok to be called here? Some people misused this function because
>> the function name was misleading them. I am sorry I am totally not
>> familiar with the ipvs code, so I am just asking you about
>> this.
> 
> 	Using nr_free_buffer_pages should be fine here.
> We are using it as rough estimation for the number of sync
> buffers we can use in NORMAL zones. We are using dev->mtu
> for such buffers, so it can take a PAGE_SIZE for a buffer.
> We are not interested in HIGHMEM size. high watermarks
> should have negliable effect. I'm even not sure whether
> we need to clamp it for systems with TBs of memory.
> 

I see. Thanks for your review and your explanation!

Zhang

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Zhang Yanfei]

于 2013年02月07日 16:40, Julian Anastasov 写道:
> 
> 	Hello,
> 
> On Thu, 7 Feb 2013, Zhang Yanfei wrote:
> 
>> Since the type of netns_ipvs->sysctl_sync_qlen_max has been changed to
>> unsigned long, type of its related proc var sync_qlen_max should be changed
>> to unsigned long, too. Also the return type of function sysctl_sync_qlen_max().
>>
>> Besides, the type of ipvs_master_sync_state->sync_queue_len should also be
>> changed to unsigned long.
> 
> 	v2 looks fine. Thanks! Regarding your question
> see below...
> 
>> Changelog from V1:
>> - change type of ipvs_master_sync_state->sync_queue_len to unsigned long
>>   as Simon addressed.
>>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: David Miller <davem@davemloft.net>
>> Cc: Julian Anastasov <ja@ssi.bg>
>> Cc: Simon Horman <horms@verge.net.au>
>> Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
>> ---
>>  include/net/ip_vs.h            |    6 +++---
>>  net/netfilter/ipvs/ip_vs_ctl.c |    4 ++--
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
>> index 68c69d5..1d56f92 100644
>> --- a/include/net/ip_vs.h
>> +++ b/include/net/ip_vs.h
>> @@ -874,7 +874,7 @@ struct ip_vs_app {
>>  struct ipvs_master_sync_state {
>>  	struct list_head	sync_queue;
>>  	struct ip_vs_sync_buff	*sync_buff;
>> -	int			sync_queue_len;
>> +	unsigned long		sync_queue_len;
>>  	unsigned int		sync_queue_delay;
>>  	struct task_struct	*master_thread;
>>  	struct delayed_work	master_wakeup_work;
>> @@ -1052,7 +1052,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>>  	return ACCESS_ONCE(ipvs->sysctl_sync_ports);
>>  }
>>  
>> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>>  {
>>  	return ipvs->sysctl_sync_qlen_max;
>>  }
>> @@ -1099,7 +1099,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>>  	return 1;
>>  }
>>  
>> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>>  {
>>  	return IPVS_SYNC_QLEN_MAX;
>>  }
>> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
>> index ec664cb..d79a530 100644
>> --- a/net/netfilter/ipvs/ip_vs_ctl.c
>> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
>> @@ -1747,9 +1747,9 @@ static struct ctl_table vs_vars[] = {
>>  	},
>>  	{
>>  		.procname	= "sync_qlen_max",
>> -		.maxlen		= sizeof(int),
>> +		.maxlen		= sizeof(unsigned long),
>>  		.mode		= 0644,
>> -		.proc_handler	= proc_dointvec,
>> +		.proc_handler	= proc_doulongvec_minmax,
>>  	},
>>  	{
>>  		.procname	= "sync_sock_size",
>> -- 
>> 1.7.1
> 
> 
>> Another question about the sysctl_sync_qlen_max:
>> This variable is assigned as:
>>
>> ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
>>
>> The function nr_free_buffer_pages actually means: counts of pages
>> which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
>>
>> is it ok to be called here? Some people misused this function because
>> the function name was misleading them. I am sorry I am totally not
>> familiar with the ipvs code, so I am just asking you about
>> this.
> 
> 	Using nr_free_buffer_pages should be fine here.
> We are using it as rough estimation for the number of sync
> buffers we can use in NORMAL zones. We are using dev->mtu
> for such buffers, so it can take a PAGE_SIZE for a buffer.
> We are not interested in HIGHMEM size. high watermarks
> should have negliable effect. I'm even not sure whether
> we need to clamp it for systems with TBs of memory.
> 

I see. Thanks for your review and your explanation!

Zhang

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Julian Anastasov]

	Hello,

On Thu, 7 Feb 2013, Zhang Yanfei wrote:

> Since the type of netns_ipvs->sysctl_sync_qlen_max has been changed to
> unsigned long, type of its related proc var sync_qlen_max should be changed
> to unsigned long, too. Also the return type of function sysctl_sync_qlen_max().
> 
> Besides, the type of ipvs_master_sync_state->sync_queue_len should also be
> changed to unsigned long.

	v2 looks fine. Thanks! Regarding your question
see below...

> Changelog from V1:
> - change type of ipvs_master_sync_state->sync_queue_len to unsigned long
>   as Simon addressed.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Miller <davem@davemloft.net>
> Cc: Julian Anastasov <ja@ssi.bg>
> Cc: Simon Horman <horms@verge.net.au>
> Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
> ---
>  include/net/ip_vs.h            |    6 +++---
>  net/netfilter/ipvs/ip_vs_ctl.c |    4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 68c69d5..1d56f92 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -874,7 +874,7 @@ struct ip_vs_app {
>  struct ipvs_master_sync_state {
>  	struct list_head	sync_queue;
>  	struct ip_vs_sync_buff	*sync_buff;
> -	int			sync_queue_len;
> +	unsigned long		sync_queue_len;
>  	unsigned int		sync_queue_delay;
>  	struct task_struct	*master_thread;
>  	struct delayed_work	master_wakeup_work;
> @@ -1052,7 +1052,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>  	return ACCESS_ONCE(ipvs->sysctl_sync_ports);
>  }
>  
> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>  {
>  	return ipvs->sysctl_sync_qlen_max;
>  }
> @@ -1099,7 +1099,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>  	return 1;
>  }
>  
> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>  {
>  	return IPVS_SYNC_QLEN_MAX;
>  }
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index ec664cb..d79a530 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1747,9 +1747,9 @@ static struct ctl_table vs_vars[] = {
>  	},
>  	{
>  		.procname	= "sync_qlen_max",
> -		.maxlen		= sizeof(int),
> +		.maxlen		= sizeof(unsigned long),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> +		.proc_handler	= proc_doulongvec_minmax,
>  	},
>  	{
>  		.procname	= "sync_sock_size",
> -- 
> 1.7.1


> Another question about the sysctl_sync_qlen_max:
> This variable is assigned as:
> 
> ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
> 
> The function nr_free_buffer_pages actually means: counts of pages
> which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
> 
> is it ok to be called here? Some people misused this function because
> the function name was misleading them. I am sorry I am totally not
> familiar with the ipvs code, so I am just asking you about
> this.

	Using nr_free_buffer_pages should be fine here.
We are using it as rough estimation for the number of sync
buffers we can use in NORMAL zones. We are using dev->mtu
for such buffers, so it can take a PAGE_SIZE for a buffer.
We are not interested in HIGHMEM size. high watermarks
should have negliable effect. I'm even not sure whether
we need to clamp it for systems with TBs of memory.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Julian Anastasov]

	Hello,

On Thu, 7 Feb 2013, Zhang Yanfei wrote:

> Since the type of netns_ipvs->sysctl_sync_qlen_max has been changed to
> unsigned long, type of its related proc var sync_qlen_max should be changed
> to unsigned long, too. Also the return type of function sysctl_sync_qlen_max().
> 
> Besides, the type of ipvs_master_sync_state->sync_queue_len should also be
> changed to unsigned long.

	v2 looks fine. Thanks! Regarding your question
see below...

> Changelog from V1:
> - change type of ipvs_master_sync_state->sync_queue_len to unsigned long
>   as Simon addressed.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Miller <davem@davemloft.net>
> Cc: Julian Anastasov <ja@ssi.bg>
> Cc: Simon Horman <horms@verge.net.au>
> Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
> ---
>  include/net/ip_vs.h            |    6 +++---
>  net/netfilter/ipvs/ip_vs_ctl.c |    4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 68c69d5..1d56f92 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -874,7 +874,7 @@ struct ip_vs_app {
>  struct ipvs_master_sync_state {
>  	struct list_head	sync_queue;
>  	struct ip_vs_sync_buff	*sync_buff;
> -	int			sync_queue_len;
> +	unsigned long		sync_queue_len;
>  	unsigned int		sync_queue_delay;
>  	struct task_struct	*master_thread;
>  	struct delayed_work	master_wakeup_work;
> @@ -1052,7 +1052,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>  	return ACCESS_ONCE(ipvs->sysctl_sync_ports);
>  }
>  
> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>  {
>  	return ipvs->sysctl_sync_qlen_max;
>  }
> @@ -1099,7 +1099,7 @@ static inline int sysctl_sync_ports(struct netns_ipvs *ipvs)
>  	return 1;
>  }
>  
> -static inline int sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
> +static inline unsigned long sysctl_sync_qlen_max(struct netns_ipvs *ipvs)
>  {
>  	return IPVS_SYNC_QLEN_MAX;
>  }
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index ec664cb..d79a530 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1747,9 +1747,9 @@ static struct ctl_table vs_vars[] = {
>  	},
>  	{
>  		.procname	= "sync_qlen_max",
> -		.maxlen		= sizeof(int),
> +		.maxlen		= sizeof(unsigned long),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> +		.proc_handler	= proc_doulongvec_minmax,
>  	},
>  	{
>  		.procname	= "sync_sock_size",
> -- 
> 1.7.1


> Another question about the sysctl_sync_qlen_max:
> This variable is assigned as:
> 
> ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
> 
> The function nr_free_buffer_pages actually means: counts of pages
> which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
> 
> is it ok to be called here? Some people misused this function because
> the function name was misleading them. I am sorry I am totally not
> familiar with the ipvs code, so I am just asking you about
> this.

	Using nr_free_buffer_pages should be fine here.
We are using it as rough estimation for the number of sync
buffers we can use in NORMAL zones. We are using dev->mtu
for such buffers, so it can take a PAGE_SIZE for a buffer.
We are not interested in HIGHMEM size. high watermarks
should have negliable effect. I'm even not sure whether
we need to clamp it for systems with TBs of memory.

Regards

--
Julian Anastasov <ja@ssi.bg>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Andrew Morton]

On Thu, 7 Feb 2013 10:40:26 +0200 (EET)
Julian Anastasov <ja@ssi.bg> wrote:

> > Another question about the sysctl_sync_qlen_max:
> > This variable is assigned as:
> > 
> > ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
> > 
> > The function nr_free_buffer_pages actually means: counts of pages
> > which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
> > 
> > is it ok to be called here? Some people misused this function because
> > the function name was misleading them. I am sorry I am totally not
> > familiar with the ipvs code, so I am just asking you about
> > this.
> 
> 	Using nr_free_buffer_pages should be fine here.
> We are using it as rough estimation for the number of sync
> buffers we can use in NORMAL zones. We are using dev->mtu
> for such buffers, so it can take a PAGE_SIZE for a buffer.
> We are not interested in HIGHMEM size. high watermarks
> should have negliable effect. I'm even not sure whether
> we need to clamp it for systems with TBs of memory.

Using nr_free_buffer_pages() is good-enough-for-now.  There are
questions around the name of this thing and its exact functionality and
whether callers are using it appropriately.  But if anything is changed
there, it will be as part of kernel-wide sweep.

One thing to bear in mind is memory hot[un]plug.  Anything which was
sized using nr_free_buffer_pages() (or similar) may become
inappropriately sized if memory is added or removed.  So any site which
uses nr_free_buffer_pages() really should be associated with a hotplug
handler and a great pile of code to resize the structure at runtime. 
It's pretty ugly stuff :(  I suspect it usually Just Doesn't Matter.

Redarding this patch:
net-change-type-of-netns_ipvs-sysctl_sync_qlen_max.patch and
net-fix-functions-and-variables-related-to-netns_ipvs-sysctl_sync_qlen_max.patch
are joined at the hip and should be redone as a single patch with a
suitable changelog, please.  And with a cc:netdev@vger.kernel.org.

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Andrew Morton]

On Thu, 7 Feb 2013 10:40:26 +0200 (EET)
Julian Anastasov <ja@ssi.bg> wrote:

> > Another question about the sysctl_sync_qlen_max:
> > This variable is assigned as:
> > 
> > ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
> > 
> > The function nr_free_buffer_pages actually means: counts of pages
> > which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
> > 
> > is it ok to be called here? Some people misused this function because
> > the function name was misleading them. I am sorry I am totally not
> > familiar with the ipvs code, so I am just asking you about
> > this.
> 
> 	Using nr_free_buffer_pages should be fine here.
> We are using it as rough estimation for the number of sync
> buffers we can use in NORMAL zones. We are using dev->mtu
> for such buffers, so it can take a PAGE_SIZE for a buffer.
> We are not interested in HIGHMEM size. high watermarks
> should have negliable effect. I'm even not sure whether
> we need to clamp it for systems with TBs of memory.

Using nr_free_buffer_pages() is good-enough-for-now.  There are
questions around the name of this thing and its exact functionality and
whether callers are using it appropriately.  But if anything is changed
there, it will be as part of kernel-wide sweep.

One thing to bear in mind is memory hot[un]plug.  Anything which was
sized using nr_free_buffer_pages() (or similar) may become
inappropriately sized if memory is added or removed.  So any site which
uses nr_free_buffer_pages() really should be associated with a hotplug
handler and a great pile of code to resize the structure at runtime. 
It's pretty ugly stuff :(  I suspect it usually Just Doesn't Matter.

Redarding this patch:
net-change-type-of-netns_ipvs-sysctl_sync_qlen_max.patch and
net-fix-functions-and-variables-related-to-netns_ipvs-sysctl_sync_qlen_max.patch
are joined at the hip and should be redone as a single patch with a
suitable changelog, please.  And with a cc:netdev@vger.kernel.org.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Julian Anastasov]

	Hello,

On Thu, 14 Feb 2013, Andrew Morton wrote:

> On Thu, 7 Feb 2013 10:40:26 +0200 (EET)
> Julian Anastasov <ja@ssi.bg> wrote:
> 
> > > Another question about the sysctl_sync_qlen_max:
> > > This variable is assigned as:
> > > 
> > > ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
> > > 
> > > The function nr_free_buffer_pages actually means: counts of pages
> > > which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
> > > 
> > > is it ok to be called here? Some people misused this function because
> > > the function name was misleading them. I am sorry I am totally not
> > > familiar with the ipvs code, so I am just asking you about
> > > this.
> > 
> > 	Using nr_free_buffer_pages should be fine here.
> > We are using it as rough estimation for the number of sync
> > buffers we can use in NORMAL zones. We are using dev->mtu
> > for such buffers, so it can take a PAGE_SIZE for a buffer.
> > We are not interested in HIGHMEM size. high watermarks
> > should have negliable effect. I'm even not sure whether
> > we need to clamp it for systems with TBs of memory.
> 
> Using nr_free_buffer_pages() is good-enough-for-now.  There are
> questions around the name of this thing and its exact functionality and
> whether callers are using it appropriately.  But if anything is changed
> there, it will be as part of kernel-wide sweep.
> 
> One thing to bear in mind is memory hot[un]plug.  Anything which was
> sized using nr_free_buffer_pages() (or similar) may become
> inappropriately sized if memory is added or removed.  So any site which
> uses nr_free_buffer_pages() really should be associated with a hotplug
> handler and a great pile of code to resize the structure at runtime. 
> It's pretty ugly stuff :(  I suspect it usually Just Doesn't Matter.

	I'll try to think on this hotplug problem
and also on the si_meminfo usage in net/netfilter/ipvs/ip_vs_ctl.c
which is quite wrong for systems with HIGHMEM, may be
we need there a free+reclaimable+file function for
NORMAL zone.

> Redarding this patch:
> net-change-type-of-netns_ipvs-sysctl_sync_qlen_max.patch and
> net-fix-functions-and-variables-related-to-netns_ipvs-sysctl_sync_qlen_max.patch
> are joined at the hip and should be redone as a single patch with a
> suitable changelog, please.  And with a cc:netdev@vger.kernel.org.

	Agreed, Zhang Yanfei and Simon? I'm just not sure,
may be this combined patch should hit only the
ipvs->nf->net trees? Or may be net-next, if we don't have
time for 3.8.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Julian Anastasov]

	Hello,

On Thu, 14 Feb 2013, Andrew Morton wrote:

> On Thu, 7 Feb 2013 10:40:26 +0200 (EET)
> Julian Anastasov <ja@ssi.bg> wrote:
> 
> > > Another question about the sysctl_sync_qlen_max:
> > > This variable is assigned as:
> > > 
> > > ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
> > > 
> > > The function nr_free_buffer_pages actually means: counts of pages
> > > which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
> > > 
> > > is it ok to be called here? Some people misused this function because
> > > the function name was misleading them. I am sorry I am totally not
> > > familiar with the ipvs code, so I am just asking you about
> > > this.
> > 
> > 	Using nr_free_buffer_pages should be fine here.
> > We are using it as rough estimation for the number of sync
> > buffers we can use in NORMAL zones. We are using dev->mtu
> > for such buffers, so it can take a PAGE_SIZE for a buffer.
> > We are not interested in HIGHMEM size. high watermarks
> > should have negliable effect. I'm even not sure whether
> > we need to clamp it for systems with TBs of memory.
> 
> Using nr_free_buffer_pages() is good-enough-for-now.  There are
> questions around the name of this thing and its exact functionality and
> whether callers are using it appropriately.  But if anything is changed
> there, it will be as part of kernel-wide sweep.
> 
> One thing to bear in mind is memory hot[un]plug.  Anything which was
> sized using nr_free_buffer_pages() (or similar) may become
> inappropriately sized if memory is added or removed.  So any site which
> uses nr_free_buffer_pages() really should be associated with a hotplug
> handler and a great pile of code to resize the structure at runtime. 
> It's pretty ugly stuff :(  I suspect it usually Just Doesn't Matter.

	I'll try to think on this hotplug problem
and also on the si_meminfo usage in net/netfilter/ipvs/ip_vs_ctl.c
which is quite wrong for systems with HIGHMEM, may be
we need there a free+reclaimable+file function for
NORMAL zone.

> Redarding this patch:
> net-change-type-of-netns_ipvs-sysctl_sync_qlen_max.patch and
> net-fix-functions-and-variables-related-to-netns_ipvs-sysctl_sync_qlen_max.patch
> are joined at the hip and should be redone as a single patch with a
> suitable changelog, please.  And with a cc:netdev@vger.kernel.org.

	Agreed, Zhang Yanfei and Simon? I'm just not sure,
may be this combined patch should hit only the
ipvs->nf->net trees? Or may be net-next, if we don't have
time for 3.8.

Regards

--
Julian Anastasov <ja@ssi.bg>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Simon Horman]

On Fri, Feb 15, 2013 at 11:39:34PM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Thu, 14 Feb 2013, Andrew Morton wrote:
> 
> > On Thu, 7 Feb 2013 10:40:26 +0200 (EET)
> > Julian Anastasov <ja@ssi.bg> wrote:
> > 
> > > > Another question about the sysctl_sync_qlen_max:
> > > > This variable is assigned as:
> > > > 
> > > > ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
> > > > 
> > > > The function nr_free_buffer_pages actually means: counts of pages
> > > > which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
> > > > 
> > > > is it ok to be called here? Some people misused this function because
> > > > the function name was misleading them. I am sorry I am totally not
> > > > familiar with the ipvs code, so I am just asking you about
> > > > this.
> > > 
> > > 	Using nr_free_buffer_pages should be fine here.
> > > We are using it as rough estimation for the number of sync
> > > buffers we can use in NORMAL zones. We are using dev->mtu
> > > for such buffers, so it can take a PAGE_SIZE for a buffer.
> > > We are not interested in HIGHMEM size. high watermarks
> > > should have negliable effect. I'm even not sure whether
> > > we need to clamp it for systems with TBs of memory.
> > 
> > Using nr_free_buffer_pages() is good-enough-for-now.  There are
> > questions around the name of this thing and its exact functionality and
> > whether callers are using it appropriately.  But if anything is changed
> > there, it will be as part of kernel-wide sweep.
> > 
> > One thing to bear in mind is memory hot[un]plug.  Anything which was
> > sized using nr_free_buffer_pages() (or similar) may become
> > inappropriately sized if memory is added or removed.  So any site which
> > uses nr_free_buffer_pages() really should be associated with a hotplug
> > handler and a great pile of code to resize the structure at runtime. 
> > It's pretty ugly stuff :(  I suspect it usually Just Doesn't Matter.
> 
> 	I'll try to think on this hotplug problem
> and also on the si_meminfo usage in net/netfilter/ipvs/ip_vs_ctl.c
> which is quite wrong for systems with HIGHMEM, may be
> we need there a free+reclaimable+file function for
> NORMAL zone.
> 
> > Redarding this patch:
> > net-change-type-of-netns_ipvs-sysctl_sync_qlen_max.patch and
> > net-fix-functions-and-variables-related-to-netns_ipvs-sysctl_sync_qlen_max.patch
> > are joined at the hip and should be redone as a single patch with a
> > suitable changelog, please.  And with a cc:netdev@vger.kernel.org.
> 
> 	Agreed, Zhang Yanfei and Simon? I'm just not sure,
> may be this combined patch should hit only the
> ipvs->nf->net trees? Or may be net-next, if we don't have
> time for 3.8.

ipvs->nf->net seems to be the most logical approach unless
Andrew wishes to take it.

I assume that at this point its too late for v3.8 and I'm not
entirely clear that the changes is needed there anyway.

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Simon Horman]

On Fri, Feb 15, 2013 at 11:39:34PM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Thu, 14 Feb 2013, Andrew Morton wrote:
> 
> > On Thu, 7 Feb 2013 10:40:26 +0200 (EET)
> > Julian Anastasov <ja@ssi.bg> wrote:
> > 
> > > > Another question about the sysctl_sync_qlen_max:
> > > > This variable is assigned as:
> > > > 
> > > > ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
> > > > 
> > > > The function nr_free_buffer_pages actually means: counts of pages
> > > > which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
> > > > 
> > > > is it ok to be called here? Some people misused this function because
> > > > the function name was misleading them. I am sorry I am totally not
> > > > familiar with the ipvs code, so I am just asking you about
> > > > this.
> > > 
> > > 	Using nr_free_buffer_pages should be fine here.
> > > We are using it as rough estimation for the number of sync
> > > buffers we can use in NORMAL zones. We are using dev->mtu
> > > for such buffers, so it can take a PAGE_SIZE for a buffer.
> > > We are not interested in HIGHMEM size. high watermarks
> > > should have negliable effect. I'm even not sure whether
> > > we need to clamp it for systems with TBs of memory.
> > 
> > Using nr_free_buffer_pages() is good-enough-for-now.  There are
> > questions around the name of this thing and its exact functionality and
> > whether callers are using it appropriately.  But if anything is changed
> > there, it will be as part of kernel-wide sweep.
> > 
> > One thing to bear in mind is memory hot[un]plug.  Anything which was
> > sized using nr_free_buffer_pages() (or similar) may become
> > inappropriately sized if memory is added or removed.  So any site which
> > uses nr_free_buffer_pages() really should be associated with a hotplug
> > handler and a great pile of code to resize the structure at runtime. 
> > It's pretty ugly stuff :(  I suspect it usually Just Doesn't Matter.
> 
> 	I'll try to think on this hotplug problem
> and also on the si_meminfo usage in net/netfilter/ipvs/ip_vs_ctl.c
> which is quite wrong for systems with HIGHMEM, may be
> we need there a free+reclaimable+file function for
> NORMAL zone.
> 
> > Redarding this patch:
> > net-change-type-of-netns_ipvs-sysctl_sync_qlen_max.patch and
> > net-fix-functions-and-variables-related-to-netns_ipvs-sysctl_sync_qlen_max.patch
> > are joined at the hip and should be redone as a single patch with a
> > suitable changelog, please.  And with a cc:netdev@vger.kernel.org.
> 
> 	Agreed, Zhang Yanfei and Simon? I'm just not sure,
> may be this combined patch should hit only the
> ipvs->nf->net trees? Or may be net-next, if we don't have
> time for 3.8.

ipvs->nf->net seems to be the most logical approach unless
Andrew wishes to take it.

I assume that at this point its too late for v3.8 and I'm not
entirely clear that the changes is needed there anyway.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Zhang Yanfei]

于 2013年02月16日 05:39, Julian Anastasov 写道:
> 
> 	Hello,
> 
> On Thu, 14 Feb 2013, Andrew Morton wrote:
> 
>> On Thu, 7 Feb 2013 10:40:26 +0200 (EET)
>> Julian Anastasov <ja@ssi.bg> wrote:
>>
>>>> Another question about the sysctl_sync_qlen_max:
>>>> This variable is assigned as:
>>>>
>>>> ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
>>>>
>>>> The function nr_free_buffer_pages actually means: counts of pages
>>>> which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
>>>>
>>>> is it ok to be called here? Some people misused this function because
>>>> the function name was misleading them. I am sorry I am totally not
>>>> familiar with the ipvs code, so I am just asking you about
>>>> this.
>>>
>>> 	Using nr_free_buffer_pages should be fine here.
>>> We are using it as rough estimation for the number of sync
>>> buffers we can use in NORMAL zones. We are using dev->mtu
>>> for such buffers, so it can take a PAGE_SIZE for a buffer.
>>> We are not interested in HIGHMEM size. high watermarks
>>> should have negliable effect. I'm even not sure whether
>>> we need to clamp it for systems with TBs of memory.
>>
>> Using nr_free_buffer_pages() is good-enough-for-now.  There are
>> questions around the name of this thing and its exact functionality and
>> whether callers are using it appropriately.  But if anything is changed
>> there, it will be as part of kernel-wide sweep.
>>
>> One thing to bear in mind is memory hot[un]plug.  Anything which was
>> sized using nr_free_buffer_pages() (or similar) may become
>> inappropriately sized if memory is added or removed.  So any site which
>> uses nr_free_buffer_pages() really should be associated with a hotplug
>> handler and a great pile of code to resize the structure at runtime. 
>> It's pretty ugly stuff :(  I suspect it usually Just Doesn't Matter.
> 
> 	I'll try to think on this hotplug problem
> and also on the si_meminfo usage in net/netfilter/ipvs/ip_vs_ctl.c
> which is quite wrong for systems with HIGHMEM, may be
> we need there a free+reclaimable+file function for
> NORMAL zone.
> 
>> Redarding this patch:
>> net-change-type-of-netns_ipvs-sysctl_sync_qlen_max.patch and
>> net-fix-functions-and-variables-related-to-netns_ipvs-sysctl_sync_qlen_max.patch
>> are joined at the hip and should be redone as a single patch with a
>> suitable changelog, please.  And with a cc:netdev@vger.kernel.org.
> 
> 	Agreed, Zhang Yanfei and Simon? I'm just not sure,
> may be this combined patch should hit only the
> ipvs->nf->net trees? Or may be net-next, if we don't have
> time for 3.8.
> 

Should I resend the combined patch?

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Zhang Yanfei]

于 2013年02月16日 05:39, Julian Anastasov 写道:
> 
> 	Hello,
> 
> On Thu, 14 Feb 2013, Andrew Morton wrote:
> 
>> On Thu, 7 Feb 2013 10:40:26 +0200 (EET)
>> Julian Anastasov <ja@ssi.bg> wrote:
>>
>>>> Another question about the sysctl_sync_qlen_max:
>>>> This variable is assigned as:
>>>>
>>>> ipvs->sysctl_sync_qlen_max = nr_free_buffer_pages() / 32;
>>>>
>>>> The function nr_free_buffer_pages actually means: counts of pages
>>>> which are beyond high watermark within ZONE_DMA and ZONE_NORMAL.
>>>>
>>>> is it ok to be called here? Some people misused this function because
>>>> the function name was misleading them. I am sorry I am totally not
>>>> familiar with the ipvs code, so I am just asking you about
>>>> this.
>>>
>>> 	Using nr_free_buffer_pages should be fine here.
>>> We are using it as rough estimation for the number of sync
>>> buffers we can use in NORMAL zones. We are using dev->mtu
>>> for such buffers, so it can take a PAGE_SIZE for a buffer.
>>> We are not interested in HIGHMEM size. high watermarks
>>> should have negliable effect. I'm even not sure whether
>>> we need to clamp it for systems with TBs of memory.
>>
>> Using nr_free_buffer_pages() is good-enough-for-now.  There are
>> questions around the name of this thing and its exact functionality and
>> whether callers are using it appropriately.  But if anything is changed
>> there, it will be as part of kernel-wide sweep.
>>
>> One thing to bear in mind is memory hot[un]plug.  Anything which was
>> sized using nr_free_buffer_pages() (or similar) may become
>> inappropriately sized if memory is added or removed.  So any site which
>> uses nr_free_buffer_pages() really should be associated with a hotplug
>> handler and a great pile of code to resize the structure at runtime. 
>> It's pretty ugly stuff :(  I suspect it usually Just Doesn't Matter.
> 
> 	I'll try to think on this hotplug problem
> and also on the si_meminfo usage in net/netfilter/ipvs/ip_vs_ctl.c
> which is quite wrong for systems with HIGHMEM, may be
> we need there a free+reclaimable+file function for
> NORMAL zone.
> 
>> Redarding this patch:
>> net-change-type-of-netns_ipvs-sysctl_sync_qlen_max.patch and
>> net-fix-functions-and-variables-related-to-netns_ipvs-sysctl_sync_qlen_max.patch
>> are joined at the hip and should be redone as a single patch with a
>> suitable changelog, please.  And with a cc:netdev@vger.kernel.org.
> 
> 	Agreed, Zhang Yanfei and Simon? I'm just not sure,
> may be this combined patch should hit only the
> ipvs->nf->net trees? Or may be net-next, if we don't have
> time for 3.8.
> 

Should I resend the combined patch?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Julian Anastasov]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 888 bytes --]


	Hello,

On Mon, 18 Feb 2013, Zhang Yanfei wrote:

> 于 2013年02月16日 05:39, Julian Anastasov 写道:
> > 
> > 	Hello,
> > 
> > On Thu, 14 Feb 2013, Andrew Morton wrote:
> > 
> >> Redarding this patch:
> >> net-change-type-of-netns_ipvs-sysctl_sync_qlen_max.patch and
> >> net-fix-functions-and-variables-related-to-netns_ipvs-sysctl_sync_qlen_max.patch
> >> are joined at the hip and should be redone as a single patch with a
> >> suitable changelog, please.  And with a cc:netdev@vger.kernel.org.
> > 
> > 	Agreed, Zhang Yanfei and Simon? I'm just not sure,
> > may be this combined patch should hit only the
> > ipvs->nf->net trees? Or may be net-next, if we don't have
> > time for 3.8.
> > 
> 
> Should I resend the combined patch?

	Yes, please! Just add CC: netdev@vger.kernel.org.
Simon will take the patch for the networking trees.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max

[Julian Anastasov]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 892 bytes --]


	Hello,

On Mon, 18 Feb 2013, Zhang Yanfei wrote:

> ao? 2013a1'02ae??16ae?JPY 05:39, Julian Anastasov a??e??:
> > 
> > 	Hello,
> > 
> > On Thu, 14 Feb 2013, Andrew Morton wrote:
> > 
> >> Redarding this patch:
> >> net-change-type-of-netns_ipvs-sysctl_sync_qlen_max.patch and
> >> net-fix-functions-and-variables-related-to-netns_ipvs-sysctl_sync_qlen_max.patch
> >> are joined at the hip and should be redone as a single patch with a
> >> suitable changelog, please.  And with a cc:netdev@vger.kernel.org.
> > 
> > 	Agreed, Zhang Yanfei and Simon? I'm just not sure,
> > may be this combined patch should hit only the
> > ipvs->nf->net trees? Or may be net-next, if we don't have
> > time for 3.8.
> > 
> 
> Should I resend the combined patch?

	Yes, please! Just add CC: netdev@vger.kernel.org.
Simon will take the patch for the networking trees.

Regards

--
Julian Anastasov <ja@ssi.bg>