From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755063Ab3BGIlc (ORCPT ); Thu, 7 Feb 2013 03:41:32 -0500 Received: from cn.fujitsu.com ([222.73.24.84]:56092 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1754639Ab3BGIlY convert rfc822-to-8bit (ORCPT ); Thu, 7 Feb 2013 03:41:24 -0500 X-IronPort-AV: E=Sophos;i="4.84,621,1355068800"; d="scan'208";a="6700123" Message-ID: <51136851.1030702@cn.fujitsu.com> Date: Thu, 07 Feb 2013 16:39:45 +0800 From: Zhang Yanfei User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.8) Gecko/20121012 Thunderbird/10.0.8 MIME-Version: 1.0 To: Julian Anastasov CC: Andrew Morton , davem@davemloft.net, Simon Horman , Linux MM , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max References: <51131B88.6040809@cn.fujitsu.com> <51132A56.60906@cn.fujitsu.com> In-Reply-To: X-MIMETrack: Itemize by SMTP Server on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/02/07 16:40:00, Serialize by Router on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/02/07 16:40:01 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 于 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 >> Cc: David Miller >> Cc: Julian Anastasov >> Cc: Simon Horman >> Signed-off-by: Zhang Yanfei >> --- >> 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