On 5/25/2013 9:57 AM, Al Viro wrote: > On Fri, May 24, 2013 at 08:21:08PM -0700, Linus Torvalds wrote: >> On Tue, May 21, 2013 at 3:22 PM, Linus Torvalds >> <torva...@linux-foundation.org> wrote: >>> Untested patch attached. It compiles cleanly, looks sane, and most of >>> it is just making the function prototypes look much nicer. I think it >>> works. >> Ok, here's another patch in the "let's make the VFS go faster series". >> This one, sadly, is not a cleanup. >> >> The concept is simple: right now the inode->i_security pointer chasing >> kills us on inode security checking with selinux. So let's move two of >> the fields from the selinux security fields directly into the inode. >> So instead of doing "inode->i_security->{sid,sclass}", we can just do >> "inode->{i_sid,i_sclass}" directly. >> >> It's a very mechanical transform, so it should all be good, but the >> reason I don't much like it is that I think other security models >> might want to do something like this too, and right now it's >> selinux-specific. I could imagine making it just an anonymous union of >> size 64 bits or something, and just making one of the union entries be >> an (anonymous) struct with those two fields. So it's not conceptually >> selinux-specific, but right now it's pretty much a selinux hack. >> >> But it's a selinux-specific hack that really does matter. The >> inode_has_perm() and selinux_inode_permission() functions show up >> pretty high on kernel profiles that do a lot of filename lookup, and >> it's pretty much all just that i_security pointer chasing and extra >> cache miss. >> >> With this, inode->i_security is not very hot any more, and we could >> move the i_security pointer elsewhere in the inode. >> >> Comments? I don't think this is *pretty* (and I do want to repeat that >> it's not even tested yet), but I think it's worth it. We've been very >> good at avoiding extra pointer dereferences in the path lookup, this >> is one of the few remaining ones. > Well... The problem I see here is not even selinux per se - it's that > "LSM stacking" insanity. How would your anon union deal with that? Which > LSM gets to play with it when we have more than one of those turds around?
I don't know that the terms "insanity" and "turds" really do the situation justice, but Al has a firm grasp on the nut of the issue. The LSM stacking I've been working on (v14 due "real soon") would render this change useless, as you'd have to have the multiple instances of the special fields just as you'd need multiple blob pointers. That would have to reintroduce the indirection you're trying to be rid of. I have been working on the assumption that the single blob pointer was all that could ever go into the inode. If that's not true stacking could get considerably easier and could have less performance impact. Now I'll put on my Smack maintainer hat. Performance improvement is always welcome, but I would rather see attention to performance of the LSM architecture than SELinux specific hacks. The LSM blob pointer scheme is there so that you (Linus) don't have to see the dreadful things that we security people are doing. Is it time to get past that level of disassociation? Or, and I really hate asking this, have you fallen into the SELinux camp? > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/