From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Subject: Re: [PATCH 1/3] IMA: move read/write counters into struct inode Date: Tue, 19 Oct 2010 08:52:27 -0700 Message-ID: References: <20101019011650.25346.99614.stgit@paris.rdu.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kenel.org, linux-security-module@vger.kernel.org, linux-fsdevel@vger.kernel.org, hch@infradead.org, zohar@us.ibm.com, warthog9@kernel.org, david@fromorbit.com, jmorris@namei.org, kyle@mcmartin.ca, hpa@zytor.com, akpm@linux-foundation.org, mingo@elte.hu, viro@zeniv.linux.org.uk To: Eric Paris Return-path: Received: from smtp1.linux-foundation.org ([140.211.169.13]:39441 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753807Ab0JSPxs convert rfc822-to-8bit (ORCPT ); Tue, 19 Oct 2010 11:53:48 -0400 In-Reply-To: <20101019011650.25346.99614.stgit@paris.rdu.redhat.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, Oct 18, 2010 at 6:16 PM, Eric Paris wrote: > > IMA currently alocated an inode integrity structure for every inode i= n > core. =A0This stucture is about 120 bytes long. =A0Most files however > (especially on a system which doesn't make use of IMA) will never nee= d any > of this space. =A0The problem is that if IMA is enabled we need to kn= ow > information about the number of readers and the number of writers for= every > inode on the box. =A0At the moment we collect that information in the= per > inode iint structure and waste the rest of the space. =A0This patch m= oves those > counters into the struct inode so we can eventually stop allocating a= n IMA > integrity structure except when absolutely needed. Hmm. I don't think this is really acceptable as-is. =46irst off (and most trivially) - the fields are misnamed. Just callin= g them "{open,read,write}count" was fine when it was part of an ima structure, but for all the historical reasons, inode fields are called 'i_xyzzy'. Secondly, we already maintain a write count (called "i_writecount"). Why is the IMA writecount different, and should it be? Thirdly, why is it an "unsigned long"? Are the IMA numbers cumulative or something? How could you ever overflow a 32-bit counter if not? =46inally, why does IMA even care about the read-counts vs open-counts? Why not just open-counts, and consider any non-write to be an open? In short, I think this patch would be _much_ more acceptable if it added just a _single_ 32-bit "i_opencount". And even then I'd ask "what's the difference between i_opencount and our already existing i_count? Linus IOW, at a glance, I think it might be much more acceptable if we only a= dded -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html