Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S265664AbTGHJz2 (ORCPT ); Tue, 8 Jul 2003 05:55:28 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S265666AbTGHJz2 (ORCPT ); Tue, 8 Jul 2003 05:55:28 -0400 Received: from air-2.osdl.org ([65.172.181.6]:5863 "EHLO mail.osdl.org") by vger.kernel.org with ESMTP id S265664AbTGHJz0 (ORCPT ); Tue, 8 Jul 2003 05:55:26 -0400 Date: Tue, 8 Jul 2003 03:09:31 -0700 From: Andrew Morton To: James Morris Cc: jgarzik@pobox.com, sds@epoch.ncsc.mil, torvalds@osdl.org, viro@parcelfarce.linux.theplanet.co.uk, alan@lxorguk.ukuu.org.uk, hch@infradead.org, greg@kroah.com, chris@wirex.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Add SELinux module to 2.5.74-bk1 Message-Id: <20030708030931.6864972b.akpm@osdl.org> In-Reply-To: References: <20030703175153.GC27556@gtf.org> X-Mailer: Sylpheed version 0.9.0pre1 (GTK+ 1.2.10; i686-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1508 Lines: 41 James Morris wrote: > > +int hashtab_replace(struct hashtab *h, void *key, void *datum, > + void (*destroy)(void *k, void *d, void *args), > + void *args) > +{ > ... > + newnode = kmalloc(sizeof(*newnode), GFP_KERNEL); >From an API perspective, the GFP_KERNEL is a problem. Particularly as this code seems to require that the caller perform the locking? The GFP_KERNEL means that the locking _has_ to be via a sleeping lock rather than a spinlock, and interrupt-time insertions are not possible. Would be better to pass in the gfp_flags. Comparing the complexity (size) of this code with the q-n-d hash tables which are currently used one does wonder how useful it all will be. The additional indirections are not needed with q-n-d hashes. But if it doesn't significantly add to the overall selinux patch then I guess it makes sense. A slab cache for hashtab_nodes would probably save a bit of space. otoh: It would be faster and more space-efficient to require that the clients of ths code embed a hastab_node inside their structures and just pass the address of that hastab_node into here. When they do a lookup they retreive their original struct with container_of. That fixes the GFP_KERNEL thing too. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/