Re: [PATCH 1/6] IMA: move read/write counters into struct inode

[Linus Torvalds]

On Tue, Oct 19, 2010 at 3:58 PM, Eric Paris <eparis@xxxxxxxxxx> wrote:
>
> This patch does the minimum needed to move the location of the data.  Further
> cleanups, especially the location of counter updates, may still be possible.

Hmm. The end result looks fine (adding four bytes to struct inode in
order to avoid all the complexity seems reasonable), but I do get the
feeling that this should likely be the last in the series, so that the
VFS level files would get minimal changes. IOW, do the cleanups inside
the IMA code first, and then do the switch-over to using counters in
the inode last.

Well, not last, since I think you need to do this before you can do
the "only allocate iint when needed" only after you've moved the
counters. But I think the logical order would be
 - switch to rbtree
 - drop opencount
 - switch counts to 'unsigned int'
 - drop iint->writecount and use i_writecount instead
 - move the remaining readcount to i_readcount
 - only allocate iint when necessary

That way you'd only have _one_ patch that touches <linux/fs.h>, rather
than four, and the remaining patches would all be to security/ima.

But maybe I missed some reason for this particular ordering.

Oh, and btw, due to alignment reasons it looks like the 4-byte
i_readcount would take 8 bytes due to bad structure packing. I don't
know if that is avoidable, but I do think it would make more sense to
put it next to i_writecount instead of in between two pointers. That
still doesn't help (we've got 3 32-bit values next to each other), but
it's at least -closer- to working out.

                         Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/