linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Eric Paris <eparis@redhat.com>
Cc: linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, hch@infradead.org,
	zohar@us.ibm.com, warthog9@kernel.org, david@fromorbit.com,
	jmorris@namei.org, kyle@mcmartin.ca, hpa@zytor.com,
	akpm@linux-foundation.org, torvalds@linux-foundation.org,
	mingo@elte.hu
Subject: Re: [PATCH 4/6] IMA: only allocate iint when needed
Date: Wed, 20 Oct 2010 04:53:24 +0100	[thread overview]
Message-ID: <20101020035324.GX19804@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20101019225833.12396.56721.stgit@paris.rdu.redhat.com>

On Tue, Oct 19, 2010 at 06:58:33PM -0400, Eric Paris wrote:
> IMA always allocates an integrity structure to hold information about every
> inode, but only needed this structure to tract the number of readers and
> writers currently accessing a given inode.  Since that information was moved
> into struct inode instead of the integrity struct this patch stops allocating
> the integrity stucture until it is needed.  Thurs greatly reducing memory
> usage.

OK, I'm really confused.  Could you explain what's going on with refcounts
now?  AFAICS, you allocate them in process_measurement() now and they live
until the inode is torn down.  Fine by me.  However
	* why bother with bumping refcount in ima_file_free()?  It's not going
to die until we free the inode, for fsck sake...
	* i_mutex is a damn strange choice for protecting your write counter
	* for that matter, why bother with refcount at all?  What could
hold a reference to that sucker without holding a reference to struct inode?
I don't see anything of that sort, other that delayed freeing that'll hold
the only remaining pointer to iint until the callback where it'll promptly
drop it.

And while we are at it, you are forcing a hash lookup on _every_ _damn_
close(2) of a regular file.  How about marking the inode as ima-infested
so that it wouldn't bother?  For that matter, this "final writer" logics
needs explanation as well...

  reply	other threads:[~2010-10-20  3:53 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-19 22:58 [PATCH 1/6] IMA: move read/write counters into struct inode Eric Paris
2010-10-19 22:58 ` [PATCH 2/6] IMA: drop the inode opencount since it isn't needed for operation Eric Paris
2010-10-19 22:58 ` [PATCH 3/6] IMA: use unsigned int instead of long for counters Eric Paris
2010-10-19 22:58 ` [PATCH 4/6] IMA: only allocate iint when needed Eric Paris
2010-10-20  3:53   ` Al Viro [this message]
2010-10-19 22:58 ` [PATCH 5/6] IMA: use rbtree instead of radix tree for inode information cache Eric Paris
2010-10-19 23:17   ` Dave Chinner
2010-10-20 11:31     ` Peter Zijlstra
2010-10-20 22:05       ` Dave Chinner
2010-10-20 22:22         ` Linus Torvalds
2010-10-20 22:47           ` Trond Myklebust
2010-10-21  0:58             ` Linus Torvalds
2010-10-21  2:17               ` Dave Chinner
2010-10-19 22:58 ` [PATCH 6/6] IMA: use i_writecount rather than a private counter Eric Paris
2010-10-20  0:25 ` [PATCH 1/6] IMA: move read/write counters into struct inode Linus Torvalds
2010-10-23  3:01   ` Eric Paris
2010-10-24  6:52     ` Mimi Zohar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101020035324.GX19804@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=david@fromorbit.com \
    --cc=eparis@redhat.com \
    --cc=hch@infradead.org \
    --cc=hpa@zytor.com \
    --cc=jmorris@namei.org \
    --cc=kyle@mcmartin.ca \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=torvalds@linux-foundation.org \
    --cc=warthog9@kernel.org \
    --cc=zohar@us.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).