linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
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>,
	Guangliang Zhao <lucienchao@gmail.com>,
	Li Wang <li.wang@ubuntykylin.com>,
	zheng.z.yan@intel.com
Subject: Re: [PATCH v2] ceph: fix posix ACL hooks
Date: Mon, 3 Feb 2014 21:19:55 +0000	[thread overview]
Message-ID: <20140203211955.GY10323@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFytvTPdBsv6BwSomUGWXZ_tKP=BxaeJ7FXBft0JNnjBCQ@mail.gmail.com>

On Mon, Feb 03, 2014 at 01:03:32PM -0800, Linus Torvalds wrote:

> 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.

Please, don't do that.  Half of that is pointless (e.g. what you are
doing with vfs_rmdir() - if anything, we could get rid of the first
argument completely, it's always victim->d_parent->d_inode and we are
holding enough locks for that to be stable) and ->permission() is
just plain wrong.

Result *is* a function of inode alone; the problem with 9P is that we
are caching FIDs in the wrong place.

What really happens is that protocol refers to objects by 32bit tokens,
given by server out to client.  Many of those can correspond to the
same file; think of those as file descriptors.  We are associating them
with dentries, but if you have several links to the same file a FID
acquired for either of them will do.

What we ought to do, AFAICS, is moving these guys from dentry to inode;
sure, to *get* one in the first place we need some dentry.  But by the
time ->setxattr() and friends get to see the inode, the caller has already
done things that make sure that some FID of his exists.

IOW, NAK.

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

Thread overview: 37+ 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 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 [this message]
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
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=20140203211955.GY10323@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=ceph-devel@vger.kernel.org \
    --cc=davej@redhat.com \
    --cc=hch@infradead.org \
    --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=torvalds@linux-foundation.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).