[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [RFC][PATCH] (#6 U1) the latest incarnation



On Wed, 2005-03-23 at 14:22 -0600, Timothy R. Chavez wrote:
> This is the latest patch. Quite a bit has changed and a lot more of
> the complexity has been stripped away. I've added back the hooks for
> d_instantiate() and d_splice_alias() for good form and rethought the
> positioning of the d_move() hooks.

Rationale?  At least for me, the development process for this patch is
too opaque. I see changes from version to version (particularly with
respect to hooks and hook placement) with no explanation of the
reasoning, which makes it hard to assess the real impact.  I think we
need a clear stated justification as to what each hook is achieving for
your higher level goals.  Note btw that it is looking like we actually
need another security hook in the fs code to deal with the file creation
issue, so d_instantiate/d_splice_alias might not be sufficient for you
either.

As I mentioned earlier, I think you need a very clearly stated
description of your high level goals and requirements to include in your
submission on linux-fsdevel and lkml, because they will need to
understand those goals in order to assess whether your implementation is
sound and to tell you the right way to implement that desired
functionality if they think your implementation isn't sound.  You want
to be careful about not confusing design/implementation with
goals/requirements.

>  Also, I've done quite a bit with the locking. Now, [admittedly] I'm a
> novice, so the reader-writer locking stradegy I've used is probably
> not the best for performance -- especially since I've hooked
> __d_lookup() and will hit a write_lock() when I enter
> audit_attach_watch() (formly called audit_watch()).

Did you test with a SMP kernel?  Locked up immediately for me on boot,
right after displaying the Mount-cache hash table entries stats
(mnt_init).

> To save space in the inode struct, I've kept the i_audit as a pointer,
> rather then statically allocating memory for it. Now, all inode's get
> i_audit data allocated to them upon creation, and freed upon deletion.
> Is it reasonable to keep it this way?

Right, this is consistent with the security field, and lets people who
don't care about audit at all completely avoid the overhead by disabling
the compile-time option. 

-- 
Stephen Smalley <sds tycho nsa gov>
National Security Agency


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]