From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758020Ab2EHHve (ORCPT ); Tue, 8 May 2012 03:51:34 -0400 Received: from mail-qc0-f174.google.com ([209.85.216.174]:51178 "EHLO mail-qc0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757265Ab2EHHuf (ORCPT ); Tue, 8 May 2012 03:50:35 -0400 Message-ID: <4FA8D046.7000808@gmail.com> Date: Tue, 08 May 2012 03:50:30 -0400 From: KOSAKI Motohiro User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0) Gecko/20120428 Thunderbird/12.0.1 MIME-Version: 1.0 To: Pekka Enberg CC: KOSAKI Motohiro , Anton Vorontsov , Minchan Kim , Leonid Moiseichuk , John Stultz , linux-mm@kvack.org, linux-kernel@vger.kernel.org, linaro-kernel@lists.linaro.org, patches@linaro.org, kernel-team@android.com Subject: Re: [PATCH 3/3] vmevent: Implement special low-memory attribute References: <20120501132409.GA22894@lizard> <20120501132620.GC24226@lizard> <4FA35A85.4070804@kernel.org> <20120504073810.GA25175@lizard> <20120507121527.GA19526@lizard> <4FA82056.2070706@gmail.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org (5/8/12 3:36 AM), Pekka Enberg wrote: > On Tue, May 8, 2012 at 10:11 AM, KOSAKI Motohiro > wrote: >> Ok, sane. Then I take my time a little and review current vmevent code briefly. >> (I read vmevent/core branch in pekka's tree. please let me know if >> there is newer repositry) > > It's the latest one. > > On Tue, May 8, 2012 at 10:11 AM, KOSAKI Motohiro > wrote: >> 1) sample_period is brain damaged idea. If people ONLY need to >> sampling stastics, they >> only need to read /proc/vmstat periodically. just remove it and >> implement push notification. >> _IF_ someone need unfrequent level trigger, just use >> "usleep(timeout); read(vmevent_fd)" >> on userland code. > > That comes from a real-world requirement. See Leonid's email on the topic: > > https://lkml.org/lkml/2012/5/2/42 I know, many embedded guys prefer such timer interval. I also have an experience similar logic when I was TV box developer. but I must disagree. Someone hope timer housekeeping complexity into kernel. but I haven't seen any justification. >> 2) VMEVENT_ATTR_STATE_ONE_SHOT is misleading name. That is effect as >> edge trigger shot. not only once. > > Would VMEVENT_ATTR_STATE_EDGE_TRIGGER be a better name? maybe. >> 3) vmevent_fd() seems sane interface. but it has name space unaware. >> maybe we discuss how to harmonize name space feature. No hurry. but we have >> to think that issue since at beginning. > > You mean VFS namespaces? Yeah, we need to take care of that. If we keep current vmevent_fd() design, we may need to create new namespace concept likes ipc namespace. current vmevent_fd() is not VFS based. >> 4) Currently, vmstat have per-cpu batch and vmstat updating makes 3 >> second delay at maximum. >> This is fine for usual case because almost userland watcher only >> read /proc/vmstat per second. >> But, for vmevent_fd() case, 3 seconds may be unacceptable delay. At >> worst, 128 batch x 4096 >> x 4k pagesize = 2G bytes inaccurate is there. > > That's pretty awful. Anton, Leonid, comments? > >> 5) __VMEVENT_ATTR_STATE_VALUE_WAS_LT should be removed from userland >> exporting files. >> When exporing kenrel internal, always silly gus used them and made unhappy. > > Agreed. Anton, care to cook up a patch to do that? > >> 6) Also vmevent_event must hide from userland. > > Why? That's part of the ABI. Ahhh, if so, I missed something. as far as I look, vmevent_fd() only depend on vmevent_config. which syscall depend on vmevent_evennt? >> 7) vmevent_config::size must be removed. In 20th century, M$ API >> prefer to use this technique. But >> They dropped the way because a lot of application don't initialize >> size member and they can't use it for keeping upper compitibility. > > It's there to support forward/backward ABI compatibility like perf > does. I'm going to keep it for now but I'm open to dropping it when > the ABI is more mature. perf api is not intended to use from generic applications. then, I don't think it will make abi issue. tool/perf is sane, isn't it? but vmevent_fd() is generic api and we can't trust all userland guy have sane, unfortunately. >> 8) memcg unaware >> 9) numa unaware >> 10) zone unaware > > Yup. > >> And, we may need vm internal change if we really need lowmem >> notification. current kernel don't have such info. _And_ there is one more >> big problem. Currently the kernel maintain memory per >> zone. But almost all userland application aren't aware zone nor node. >> Thus raw notification aren't useful for userland. In the other hands, total >> memory and total free memory is useful? Definitely No! >> Even though total free memory are lots, system may start swap out and >> oom invokation. If we can't oom invocation, this feature has serious raison >> d'etre issue. (i.e. (4), (8), (9) and (19) are not ignorable issue. I think) > > I'm guessing most of the existing solutions get away with > approximations and soft limits because they're mostly used on UMA > embedded machines. > > But yes, we need to do better here. Hm. If you want vmevent makes depend on CONFIG_EMBEDDED, I have no reason to complain this feature. At that world, almost all applications _know_ their system configuration. then I don't think api misuse issue is big matter.