From: Julian Anastasov <ja@ssi.bg> To: Andrew Morton <akpm@linux-foundation.org> Cc: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>, davem@davemloft.net, Simon Horman <horms@verge.net.au>, Linux MM <linux-mm@kvack.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max Date: Fri, 15 Feb 2013 23:39:34 +0200 (EET) [thread overview] Message-ID: <alpine.LFD.2.00.1302152304010.1746@ja.ssi.bg> (raw) In-Reply-To: <20130214142159.d0516a5f.akpm@linux-foundation.org> 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>
WARNING: multiple messages have this Message-ID (diff)
From: Julian Anastasov <ja@ssi.bg> To: Andrew Morton <akpm@linux-foundation.org> Cc: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>, davem@davemloft.net, Simon Horman <horms@verge.net.au>, Linux MM <linux-mm@kvack.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v2] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max Date: Fri, 15 Feb 2013 23:39:34 +0200 (EET) [thread overview] Message-ID: <alpine.LFD.2.00.1302152304010.1746@ja.ssi.bg> (raw) In-Reply-To: <20130214142159.d0516a5f.akpm@linux-foundation.org> 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>
next prev parent reply other threads:[~2013-02-15 21:35 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-02-07 3:12 [PATCH] net: fix functions and variables related to netns_ipvs->sysctl_sync_qlen_max Zhang Yanfei 2013-02-07 3:12 ` Zhang Yanfei 2013-02-07 4:15 ` [PATCH v2] " Zhang Yanfei 2013-02-07 4:15 ` Zhang Yanfei 2013-02-07 7:43 ` Simon Horman 2013-02-07 7:43 ` Simon Horman 2013-02-07 8:40 ` Julian Anastasov 2013-02-07 8:40 ` Julian Anastasov 2013-02-07 8:39 ` Zhang Yanfei 2013-02-07 8:39 ` Zhang Yanfei 2013-02-14 22:21 ` Andrew Morton 2013-02-14 22:21 ` Andrew Morton 2013-02-15 21:39 ` Julian Anastasov [this message] 2013-02-15 21:39 ` Julian Anastasov 2013-02-18 4:54 ` Simon Horman 2013-02-18 4:54 ` Simon Horman 2013-02-18 6:13 ` Zhang Yanfei 2013-02-18 6:13 ` Zhang Yanfei 2013-02-18 21:57 ` Julian Anastasov 2013-02-18 21:57 ` Julian Anastasov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=alpine.LFD.2.00.1302152304010.1746@ja.ssi.bg \ --to=ja@ssi.bg \ --cc=akpm@linux-foundation.org \ --cc=davem@davemloft.net \ --cc=horms@verge.net.au \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=zhangyanfei@cn.fujitsu.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.