Hi David,
I'm testing your three patches.
I am not seeing any degradation with your patches.
But I think the patch that I attach to this mail is necessary.
Isn't it?
David Chinner wrote:
> On Wed, Oct 18, 2006 at 12:33:25PM +1000, David Chinner wrote:
>> On Tue, Oct 17, 2006 at 12:02:18PM +1000, David Chinner wrote:
>>> So, here's another patch that doesn't have the performance problems,
>>> but removes the iput/igrab while still (I think) fixing the use
>>> after free problem. Can you try this one out, Takenori? I've
>>> run it through some stress tests and haven't been able to trigger
>>> problems.
>> I just hit the BUG_ON(vp == NULL) that I put in xfs_iunpin()
>> in this patch. The xfs inode had no link to the bhv_vnode, nor
>> did it have either XFS_IRECLAIM* flag set, and hence it triggered
>> the BUG.
>
> And again. The xfs_iget_core change is valid - there's still a
> race in xfs_iunpin (how many of them can we find?):
>
> xfs_iunpin xfs_iget_core
> if(atomic_dec_and_test(pincount))
> if (vp == NULL)
> if(IRECLAIMABLE)
> if(pincount)
> force+restart
> .....
> clear IRECLAIMABLE
>
> spin_lock(i_flags_lock)
> If (IRECLAIMABLE)
> BUG_ON(vp == NULL)
>
>
> So the solution is this:
>
> ---
> fs/xfs/xfs_inode.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> Index: 2.6.x-xfs-new/fs/xfs/xfs_inode.c
> ===================================================================
> --- 2.6.x-xfs-new.orig/fs/xfs/xfs_inode.c 2006-10-18 11:27:04.000000000
> +1000
> +++ 2.6.x-xfs-new/fs/xfs/xfs_inode.c 2006-10-18 16:45:12.658102093 +1000
> @@ -2738,7 +2738,7 @@ xfs_iunpin(
> {
> ASSERT(atomic_read(&ip->i_pincount) > 0);
>
> - if (atomic_dec_and_test(&ip->i_pincount)) {
> + if (atomic_dec_and_lock(&ip->i_pincount, &ip->i_flags_lock)) {
>
> /*
> * If the inode is currently being reclaimed, the link between
> @@ -2757,7 +2757,6 @@ xfs_iunpin(
> * unpinned.
> */
>
> - spin_lock(&ip->i_flags_lock);
> if (!__xfs_iflags_test(ip, XFS_IRECLAIM|XFS_IRECLAIMABLE)) {
> bhv_vnode_t *vp = XFS_ITOV_NULL(ip);
> struct inode *inode = NULL;
>
> I'm running stress tests on this now - it it survives until morning
> I'll send out a new set of patches for testing...
>
> Cheers,
>
> Dave.
--
+=-=+=-=+=-=+=-=+=-=+=-=+=-=+=-=+=-=+=-=+
ＮＥＣ コンピュータソフトウェア事業本部
ＯＳＳ推進センター 技術開発Ｇ
永野 武則 (Takenori Nagano)
TEL:8-23-57270(MyLine) 042-333-5383(外線)
e-mail:t-nagano@xxxxxxxxxxxxx
+=-=+=-=+=-=+=-=+=-=+=-=+=-=+=-=+=-=+=-=+
diff -Naru linux-2.6.19-rc1/fs/xfs.orig/xfs_inode.c
linux-2.6.19-rc1/fs/xfs/xfs_inode.c
--- linux-2.6.19-rc1/fs/xfs.orig/xfs_inode.c 2006-10-19 01:51:43.020000000
+0900
+++ linux-2.6.19-rc1/fs/xfs/xfs_inode.c 2006-10-19 01:53:47.248000000 +0900
@@ -2713,6 +2713,7 @@
XFS_FORCED_SHUTDOWN(ip->i_mount));
xfs_inode_item_destroy(ip);
}
+ xfs_iunpin_wait(ip);
kmem_zone_free(xfs_inode_zone, ip);
}
@@ -2784,7 +2785,7 @@
* be subsequently pinned once someone is waiting for it to be
* unpinned.
*/
-STATIC void
+void
xfs_iunpin_wait(
xfs_inode_t *ip)
{
diff -Naru linux-2.6.19-rc1/fs/xfs.orig/xfs_inode.h
linux-2.6.19-rc1/fs/xfs/xfs_inode.h
--- linux-2.6.19-rc1/fs/xfs.orig/xfs_inode.h 2006-10-19 01:51:42.980000000
+0900
+++ linux-2.6.19-rc1/fs/xfs/xfs_inode.h 2006-10-19 01:52:17.980000000 +0900
@@ -498,6 +498,7 @@
void xfs_iroot_realloc(xfs_inode_t *, int, int);
void xfs_ipin(xfs_inode_t *);
void xfs_iunpin(xfs_inode_t *);
+void xfs_iunpin_wait(xfs_inode_t *);
int xfs_iextents_copy(xfs_inode_t *, xfs_bmbt_rec_t *, int);
int xfs_iflush(xfs_inode_t *, uint);
void xfs_iflush_all(struct xfs_mount *);
|