On Mon, 2005-03-14 at 17:14 -0600, Timothy R. Chavez wrote:
The only thing I'm a little concerned about in this patch has to
do with
audit_watch_insert(). I think I need to lock around where I check to see if
the watchlist is empty and then possibly update the inode. Other then that,
I think I have it pretty much all covered. I feel more comfortable with the
locking in patch #6 then I did in patch #5.
- Is there any reason to use the irqsave/irqrestore form of the locks?
When is your code ever called from irq?
- Relying on locking in the caller to protect your own data structures
seems fragile and unmaintainable; I don't think you want to rely on the
caller holding dentry->d_lock for exclusion for your own lists, e.g. the
audit data stack?
- I'm a bit concerned by the whole audit data stack model; sorry to
bring it up now. Is the resulting complexity truly justified (vs. up
front allocations upon audit_inode_alloc, at the cost of wasting that
memory for inodes that never require watches, or an explicit hook
elsewhere at a point where we can perform blocking allocation and
propagate errors)?
--
Stephen Smalley <sds(a)tycho.nsa.gov>
National Security Agency