On Mon, Oct 18, 2010 at 6:16 PM, Eric Paris <eparis@xxxxxxxxxx> wrote:
>
> IMA currently alocated an inode integrity structure for every inode in
> core. This stucture is about 120 bytes long. Most files however
> (especially on a system which doesn't make use of IMA) will never need any
> of this space. The problem is that if IMA is enabled we need to know
> information about the number of readers and the number of writers for every
> inode on the box. At the moment we collect that information in the per
> inode iint structure and waste the rest of the space. This patch moves those
> counters into the struct inode so we can eventually stop allocating an IMA
> integrity structure except when absolutely needed.
Hmm. I don't think this is really acceptable as-is.
First off (and most trivially) - the fields are misnamed. Just calling
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?
Finally, 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 added
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
