xfs
[Top] [All Lists]

Re: another problem with latest code drops

To: Lachlan McIlroy <lachlan@xxxxxxx>
Subject: Re: another problem with latest code drops
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 17 Oct 2008 12:21:41 +1100
Cc: xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <48F7E7BA.4070209@sgi.com>
Mail-followup-to: Lachlan McIlroy <lachlan@xxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
References: <48F6A19D.9080900@sgi.com> <20081016060247.GF25906@disturbed> <48F6EF7F.4070008@sgi.com> <20081016072019.GH25906@disturbed> <48F6FCB7.6050905@sgi.com> <20081016222904.GA31761@disturbed> <48F7E7BA.4070209@sgi.com>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.18 (2008-05-17)
On Fri, Oct 17, 2008 at 11:17:46AM +1000, Lachlan McIlroy wrote:
> Dave Chinner wrote:
>>> I am seeing a lot of memory used here though:
>>>
>>> 116605669 116605669  26%    0.23K 6859157       17  27436628K 
>>> selinux_inode_security
>>
>> Ah - I don't run selinux. Sounds like a bug that needs reporting
>> to lkml...
>
> I'm sure this is caused by your changes that introduced inode_init_always().
> It re-initialises an existing inode without destroying it first so it calls
> security_inode_alloc() without calling security_inode_free().

I can't think of how. The layers above XFS are symmetric:

        alloc_inode()
          inode_init_always()
            security_inode_alloc()
        .....
        security_inode_free()
        ->destroy_inode()

So the filesystems should never, ever need to know about the
security context attached to the inode. The changes that introduced
inode_init_always() do not change this symmetry - we do:

        xfs_inode_alloc()
          inode_init_always()
            security_inode_alloc()
        .....
        security_inode_free()
        ->destroy_inode()

And we should have this symmetry everywhere.

<thinks for a bit>

Hmmmm - maybe the xfs_iget_cache_miss failure paths where we call
xfs_idestroy() could leak contexts. We should really call xfs_iput()
because we have an initialised linux inode at this point and so
we need to go through destroy_inode(). I'll have a bit more of
a look, but this doesn't seem to account for the huge number of
leaked contexts you reported....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx


<Prev in Thread] Current Thread [Next in Thread>