All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	Ilya Dryomov <ilya.dryomov@inktank.com>,
	Sage Weil <sage@inktank.com>, Dave Jones <davej@redhat.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	ceph-devel@vger.kernel.org,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Guangliang Zhao <lucienchao@gmail.com>,
	Li Wang <li.wang@ubuntykylin.com>,
	zheng.z.yan@intel.com, Steven Whitehouse <swhiteho@redhat.com>,
	cluster-devel@redhat.com
Subject: Re: [PATCH v2] ceph: fix posix ACL hooks
Date: Mon, 3 Feb 2014 13:23:36 -0800	[thread overview]
Message-ID: <20140203212336.GA31966@infradead.org> (raw)
In-Reply-To: <CA+55aFytvTPdBsv6BwSomUGWXZ_tKP=BxaeJ7FXBft0JNnjBCQ@mail.gmail.com>

On Mon, Feb 03, 2014 at 01:03:32PM -0800, Linus Torvalds wrote:
> Now, to be honest, pushing it down one more level (to
> generic_permission()) will actually start causing some trouble. In
> particular, gfs2_permission() fundamentally does not have a dentry for
> several of the callers.

Looking over the gfs2 code the problem seems to be that it duplicates
permissions checks from the may_{lookup,create,linkat,delete}, most
likely because it needs cluster locking in place for them.  The right
fix seems to be to optionally call the filesystem from those.  That
being said I wonder how ocfs2 or network filesystems get away without
that.

> What do you think? I guess this patch could be split up into two: one
> that does the "vfs_xyz()" helper functions, and another that does the
> inode_permission() change. I tied them together mainly because I
> started with the inode_permission() change, and that required the
> vfs_xyz() change.

The changes look good to me, and yes I think they should be split.
I'll see if I can take this further, but doing something non-hacky
in GFS2 would be the first step here.


WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH v2] ceph: fix posix ACL hooks
Date: Mon, 3 Feb 2014 13:23:36 -0800	[thread overview]
Message-ID: <20140203212336.GA31966@infradead.org> (raw)
In-Reply-To: <CA+55aFytvTPdBsv6BwSomUGWXZ_tKP=BxaeJ7FXBft0JNnjBCQ@mail.gmail.com>

On Mon, Feb 03, 2014 at 01:03:32PM -0800, Linus Torvalds wrote:
> Now, to be honest, pushing it down one more level (to
> generic_permission()) will actually start causing some trouble. In
> particular, gfs2_permission() fundamentally does not have a dentry for
> several of the callers.

Looking over the gfs2 code the problem seems to be that it duplicates
permissions checks from the may_{lookup,create,linkat,delete}, most
likely because it needs cluster locking in place for them.  The right
fix seems to be to optionally call the filesystem from those.  That
being said I wonder how ocfs2 or network filesystems get away without
that.

> What do you think? I guess this patch could be split up into two: one
> that does the "vfs_xyz()" helper functions, and another that does the
> inode_permission() change. I tied them together mainly because I
> started with the inode_permission() change, and that required the
> vfs_xyz() change.

The changes look good to me, and yes I think they should be split.
I'll see if I can take this further, but doing something non-hacky
in GFS2 would be the first step here.



  parent reply	other threads:[~2014-02-03 21:23 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-28 18:40 [GIT PULL] Ceph updates for -rc1 Sage Weil
2014-01-28 21:10 ` Dave Jones
2014-01-28 21:48   ` Linus Torvalds
2014-01-29  6:08     ` Sage Weil
2014-01-29 14:30       ` Sage Weil
2014-01-29 14:30         ` Sage Weil
2014-01-29 16:36         ` Ilya Dryomov
2014-01-29 16:37           ` [PATCH v2] ceph: fix posix ACL hooks Ilya Dryomov
2014-01-29 19:09             ` Linus Torvalds
2014-01-30  7:54               ` Christoph Hellwig
2014-01-30 22:01                 ` Linus Torvalds
2014-01-31  0:14                   ` Sage Weil
2014-02-03 10:29                   ` Christoph Hellwig
2014-02-03 11:13                     ` Al Viro
2014-02-03 21:03                     ` Linus Torvalds
2014-02-03 21:19                       ` Al Viro
2014-02-03 21:24                         ` Christoph Hellwig
2014-02-03 21:31                           ` Al Viro
2014-02-03 21:36                             ` Christoph Hellwig
2014-02-03 21:37                             ` Linus Torvalds
2014-02-03 21:42                               ` Al Viro
2014-02-03 21:31                         ` Linus Torvalds
2014-02-03 21:39                           ` Al Viro
2014-02-03 21:43                             ` Al Viro
2014-02-03 21:44                             ` Linus Torvalds
2014-02-03 22:31                               ` Al Viro
2014-02-06 20:51                                 ` Jeremy Allison
2014-02-03 21:23                       ` Christoph Hellwig [this message]
2014-02-03 21:23                         ` [Cluster-devel] " Christoph Hellwig
2014-02-03 21:59                       ` Al Viro
2014-02-03 22:12                         ` Linus Torvalds
2014-02-03 22:40                           ` Al Viro
2014-02-03 22:55                             ` Linus Torvalds
2014-02-04 11:33                             ` Steven Whitehouse
2014-02-04 15:57                               ` Christoph Hellwig
2014-02-04 16:17                               ` Linus Torvalds
2014-02-03 21:59                       ` Linus Torvalds
2014-01-29 20:43             ` Ilya Dryomov
2014-01-30 10:46         ` [GIT PULL] Ceph updates for -rc1 Christoph Hellwig

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=20140203212336.GA31966@infradead.org \
    --to=hch@infradead.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=cluster-devel@redhat.com \
    --cc=davej@redhat.com \
    --cc=ilya.dryomov@inktank.com \
    --cc=li.wang@ubuntykylin.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucienchao@gmail.com \
    --cc=sage@inktank.com \
    --cc=swhiteho@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=zheng.z.yan@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.