From mboxrd@z Thu Jan 1 00:00:00 1970 Subject: Re: [PATCH/RFC] Add node states sysfs class attributeS - V4 From: Lee Schermerhorn In-Reply-To: References: <200708242228.l7OMS5fU017948@imap1.linux-foundation.org> <20070827181405.57a3d8fe.akpm@linux-foundation.org> <20070827201822.2506b888.akpm@linux-foundation.org> <20070827222912.8b364352.akpm@linux-foundation.org> <20070827231214.99e3c33f.akpm@linux-foundation.org> <1188309928.5079.37.camel@localhost> <29495f1d0708281513g406af15an8139df5fae20ad35@mail.gmail.com> <1188398621.5121.13.camel@localhost> <1188487157.5794.40.camel@localhost> Content-Type: text/plain Date: Thu, 30 Aug 2007 14:41:53 -0400 Message-Id: <1188499314.5794.114.camel@localhost> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org Return-Path: To: Christoph Lameter Cc: linux-mm , Nish Aravamudan , Andrew Morton , mel@skynet.ie, y-goto@jp.fujitsu.com, Kamezawa Hiroyuki , Eric Whitney List-ID: On Thu, 2007-08-30 at 11:19 -0700, Christoph Lameter wrote: > On Thu, 30 Aug 2007, Lee Schermerhorn wrote: > > > Try, try again. Maybe closer this time. > > Yes. Thanks for all your work on this. > > > Question: do we need/want to display the normal and high memory masks > > separately for systems with HIGHMEM? If not, I'd suggest changing the > > print_nodes_state() function to take a nodemask_t* instead of a state > > enum and expose a single 'has_memory' attribute that we print using > > something like: > > No leave it separate. OK. > > > > > static ssize_t print_nodes_has_memory(struct sysdev_class *class, > > char *buf) > > { > > nodemask_t has_memory = node_states[N_NORMAL_MEMORY]; > > > > if (N_HIGH_MEMORY - N_NORMAL_MEMORY) > > Uggg. Better do #ifdef CONFIG_HIGHMEM I thought Andrew preferred to allow the compiler to do the dead code elimination after syntax checking, ... Sounds like it's moot in this case, tho', as we're leaving the two masks separate. > > > nodes_or(has_memory, has_memory, node_states[N_HIGH_MEMORY]); > > > > return print_nodes_state(&has_memory, buf); > > > + * node states attributes > > + */ > > + > > +static ssize_t print_nodes_state(enum node_states state, char *buf) > > +{ > > + int n; > > + > > + n = nodelist_scnprintf(buf, PAGE_SIZE, node_states[state]); > > + if (n <= 0) > > + goto done; > > + if (PAGE_SIZE - n > 1) { > > if (n > 0 && PAGE_SIZE > n + 1) OK. I'll fix that after KS. > > ? > > + *(buf + n++) = '\n'; > > + *(buf + n++) = '\0'; > > + } > > > +static ssize_t print_nodes_possible(struct sysdev_class *class, char *buf) > > +{ > > + return print_nodes_state(N_POSSIBLE, buf); > > +} > > + > > +static ssize_t print_nodes_online(struct sysdev_class *class, char *buf) > > +{ > > + return print_nodes_state(N_ONLINE, buf); > > +} > > + > > +static ssize_t print_nodes_has_normal_memory(struct sysdev_class *class, > > + char *buf) > > +{ > > + return print_nodes_state(N_NORMAL_MEMORY, buf); > > +} > > + > > +static ssize_t print_nodes_has_cpu(struct sysdev_class *class, char *buf) > > +{ > > + return print_nodes_state(N_CPU, buf); > > +} > > + > > +static SYSDEV_CLASS_ATTR(possible, 0444, print_nodes_possible, NULL); > > +static SYSDEV_CLASS_ATTR(online, 0444, print_nodes_online, NULL); > > +static SYSDEV_CLASS_ATTR(has_normal_memory, 0444, print_nodes_has_normal_memory, > > I am sure that there is some way to dynamicall allocate these. There may be some way to allocate the files in a group. Not sure for class attribute files. In any case, AFAICS the attributes/files each require a separately defined attribute structure. I'll let this set for a week or so and see if I can find a preferred way. Lee -- 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: email@kvack.org