From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Subject: Re: [PATCH] vfs: move ACL cache lookup into generic code
Date: Mon, 25 Jul 2011 13:46:05 +0530
Message-ID: <87r55eg4hm.fsf@skywalker.in.ibm.com>
References: <alpine.LFD.2.02.1107221023170.29055@i5.linux-foundation.org> <alpine.LFD.2.02.1107221038090.29055@i5.linux-foundation.org> <alpine.LFD.2.02.1107221932120.27112@i5.linux-foundation.org> <20110723032944.GA24703@ZenIV.linux.org.uk> <CA+55aFx90c2HaBJ+SA7zEVrnQJkiQRrfK6XUqJvLVA1CsR1Okw@mail.gmail.com> <20110723043120.GB24703@ZenIV.linux.org.uk>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Christoph Hellwig <hch@lst.de>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
To: Al Viro <viro@ZenIV.linux.org.uk>,
	Linus Torvalds <torvalds@linux-foundation.org>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from e9.ny.us.ibm.com ([32.97.182.139]:34941 "EHLO e9.ny.us.ibm.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1750977Ab1GYIQQ (ORCPT <rfc822;linux-fsdevel@vger.kernel.org>);
	Mon, 25 Jul 2011 04:16:16 -0400
Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233])
	by e9.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p6P7hh6e031366
	for <linux-fsdevel@vger.kernel.org>; Mon, 25 Jul 2011 03:43:43 -0400
Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64])
	by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p6P8GAj0122256
	for <linux-fsdevel@vger.kernel.org>; Mon, 25 Jul 2011 04:16:10 -0400
Received: from d01av04.pok.ibm.com (loopback [127.0.0.1])
	by d01av04.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p6P8GADH016738
	for <linux-fsdevel@vger.kernel.org>; Mon, 25 Jul 2011 04:16:10 -0400
In-Reply-To: <20110723043120.GB24703@ZenIV.linux.org.uk>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Sat, 23 Jul 2011 05:31:20 +0100, Al Viro <viro@ZenIV.linux.org.uk> wrote:
> On Fri, Jul 22, 2011 at 08:42:35PM -0700, Linus Torvalds wrote:
> > > ... why is that a problem? ?Locking there is mere ->i_lock and getting
> > > a refcount is atomic_inc(). ?Grabbing a reference might be Not Nice from
> > > the cacheline bouncing POV, but...
> > 
> > I agree in theory, but it's not something we've done before. Some of
> > the posix-acl code is pretty disgusting, I didn't even want to go
> > there.
> > 
> > And the case that tends to really *matter* is the "no acls" case
> > anyway, and that's the case that is guaranteed to have no nasty races
> > or odd issues with having to allocate/de-allocate any acl structures.
> > But yes, this could be looked at.
> 
> Heh...  In addition to ocfs2 leak: 9p leaks nicely if v9fs_acl_mode() is
> called with !S_ISDIR(mode).  In that case acl reference is simply lost.
> So yes, it's worth looking at.

something like the below ? I will also audit rest of the code.

diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index e98f56d..abad8b3 100644
--- a/fs/9p/acl.c
+++ b/fs/9p/acl.c
@@ -212,7 +212,7 @@ int v9fs_acl_mode(struct inode *dir, mode_t *modep,
 		struct posix_acl *clone;
 
 		if (S_ISDIR(mode))
-			*dpacl = acl;
+			*dpacl = posix_acl_clone(acl, GFP_NOFS);
 		clone = posix_acl_clone(acl, GFP_NOFS);
 		retval = -ENOMEM;
 		if (!clone)
@@ -227,7 +227,7 @@ int v9fs_acl_mode(struct inode *dir, mode_t *modep,
 			*pacl = clone;
 	}
 	*modep  = mode;
-	return 0;
+	retval = 0;
 cleanup:
 	posix_acl_release(acl);
 	return retval;