From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f65.google.com ([209.85.218.65]:33451 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751014AbcHQMC0 (ORCPT ); Wed, 17 Aug 2016 08:02:26 -0400 Received: by mail-oi0-f65.google.com with SMTP id s207so10193232oie.0 for ; Wed, 17 Aug 2016 05:01:43 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20160816205906.GB7292@ubuntu-hedt> References: <1470086846-19844-1-git-send-email-seth.forshee@canonical.com> <1470086846-19844-3-git-send-email-seth.forshee@canonical.com> <20160816205906.GB7292@ubuntu-hedt> From: Miklos Szeredi Date: Wed, 17 Aug 2016 14:01:41 +0200 Message-ID: Subject: Re: [RFC v3 2/2] fuse: Add posix acl support To: Seth Forshee Cc: fuse-devel , linux-fsdevel@vger.kernel.org, "Eric W. Biederman" , Michael j Theall , =?UTF-8?B?SmVhbi1QaWVycmUgQW5kcsOp?= , Nikolaus Rath Content-Type: text/plain; charset=UTF-8 Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Aug 16, 2016 at 10:59 PM, Seth Forshee wrote: > Hi Miklos, > > On Thu, Aug 04, 2016 at 02:11:20PM +0200, Miklos Szeredi wrote: > > > >> And again. >> >> I'm really wondering if it's simpler to just add an xattr parser to >> libfuse and do these at the filesystem level. That would simplify >> this patchset a lot: >> >> Reduce the scope to just permission checking, which is what we can do >> best and fastest in the kernel. And leave the rest to userspace. >> They don't have performance impact, but trying to push this into the >> kernel is just asking for trouble. > > I've been playing with this over the past couple of days, and I wanted > to get a little more feedback before I proceed. > > Things are pretty simple in the kernel if we just pass through the acl > xattrs, but either the kernel or libfuse will need to work out the > equivalent file mode when posix acls are written. I'm favoring libfuse > for this, since it's very straightforward once you're already parsing > the xattr and then we won't need to add a setattr+setxattr op. What we > will need is to refresh the mode in the kernel from userspace. > > Right now after a successful setxattr we call fuse_invalidate_attr(), > which should take care of that problem. I'm not sure the reasoning > behind doing this still applies though. According to > d331a415aef98717393dda0be69b7947da08eba3 it was added to force a refresh > of ctime, but later in 31f3267b4ba16b12fb9dd3b1953ea0f221cc2ab4 fuse was > changed to prefer ctime as maintained by the kernel, so it looks like > that invalidate (and the one in removexattr, maybe others?) could be > removed. > > If so, we could still keep it when setting posix acl xattrs, which would > be the simplest option. Otherwise we need to get the mode back from > userspace after the setxattr, either via a conditional outarg for > setxattr or by adding a new operation. > > What's your preference for all of this? Lets just keep the invalidate. My philosophy is to not optimize until someone actually complains about performance. And these invalidates are unlikely to be a problem anyway. Thanks, Miklos