CEPH-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Luis Henriques <lhenriques@suse.de>
Cc: ceph-devel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-fscrypt@vger.kernel.org
Subject: Re: [RFC PATCH v6 00/20] ceph+fscrypt: context, filename and symlink support
Date: Mon, 19 Apr 2021 12:28:10 -0400
Message-ID: <13750c0b72dccd84e75179d62e9a9038d6f57371.camel@kernel.org> (raw)
In-Reply-To: <871rb6mfch.fsf@suse.de>

On Mon, 2021-04-19 at 17:03 +0100, Luis Henriques wrote:
> Jeff Layton <jlayton@kernel.org> writes:
> 
> > On Mon, 2021-04-19 at 11:30 +0100, Luis Henriques wrote:
> ...
> > Ouch. That looks like a real bug, alright.
> > 
> > Basically when building the path, we occasionally need to fetch the
> > crypto context for parent inodes and such, and that can cause us to
> > recurse back into __ceph_getxattr and try to issue another RPC to the
> > MDS.
> > 
> > I'll have to look and see what we can do. Maybe it's safe to drop the
> > mdsc->mutex while we're building the path? Or maybe this is a good time
> > to re-think a lot of the really onerous locking in this codepath?
> > 
> > I'm open to suggestions here...
> 
> Yeah, I couldn't see a good fix at a first glace.  Dropping the mutex
> while building the path was my initial thought too but it's not easy to
> proof that's a safe thing to do.
> 

Indeed. It's an extremely coarse-grained mutex and not at all clear what
it protects here.

> The other idea I had was to fetch all the needed fscrypt contexts at the
> end, after building the path.  But I didn't found a way for doing that
> because to build the path... we need the contexts.
> 
> It looks like this leaves us with the locking rethinking option.
> 
> /me tries harder to find another way out
> 
> Cheers,

The other option I think is to not store the context in an xattr at all,
and instead make a dedicated field in the inode for it that we can
ensure is always present for encrypted inodes.  For the most part the
crypto context is a static thing. The only exception is when we're first
encrypting an empty dir.

We already have the fscrypt bool in the inodestat, and we're going to
need another field to hold the real size for files. It may be worthwhile
to just reconsider the design at that level. Maybe we just need to carve
out a chunk of fscrypt space in the inode for the client and let it
manage that however it sees fit.
-- 
Jeff Layton <jlayton@kernel.org>


  reply index

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-13 17:50 Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 01/20] vfs: export new_inode_pseudo Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 02/20] fscrypt: export fscrypt_base64_encode and fscrypt_base64_decode Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 03/20] fscrypt: export fscrypt_fname_encrypt and fscrypt_fname_encrypted_size Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 04/20] fscrypt: add fscrypt_context_for_new_inode Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 05/20] ceph: crypto context handling for ceph Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 06/20] ceph: implement -o test_dummy_encryption mount option Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 07/20] ceph: preallocate inode for ops that may create one Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 08/20] ceph: add routine to create fscrypt context prior to RPC Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 09/20] ceph: make ceph_msdc_build_path use ref-walk Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 10/20] ceph: add encrypted fname handling to ceph_mdsc_build_path Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 11/20] ceph: decode alternate_name in lease info Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 12/20] ceph: send altname in MClientRequest Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 13/20] ceph: properly set DCACHE_NOKEY_NAME flag in lookup Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 14/20] ceph: make d_revalidate call fscrypt revalidator for encrypted dentries Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 15/20] ceph: add helpers for converting names for userland presentation Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 16/20] ceph: add fscrypt support to ceph_fill_trace Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 17/20] ceph: add support to readdir for encrypted filenames Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 18/20] ceph: create symlinks with encrypted and base64-encoded targets Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 19/20] ceph: make ceph_get_name decrypt filenames Jeff Layton
2021-04-13 17:50 ` [RFC PATCH v6 20/20] ceph: add fscrypt ioctls Jeff Layton
2021-04-19 10:09   ` Luis Henriques
2021-04-19 12:19     ` Jeff Layton
2021-04-19 19:54       ` Eric Biggers
2021-04-20  9:34         ` Luis Henriques
2021-04-20 11:45         ` Jeff Layton
2021-04-19 10:30 ` [RFC PATCH v6 00/20] ceph+fscrypt: context, filename and symlink support Luis Henriques
2021-04-19 12:23   ` Jeff Layton
2021-04-19 16:03     ` Luis Henriques
2021-04-19 16:28       ` Jeff Layton [this message]
2021-04-20 10:11         ` Luis Henriques
2021-04-20 15:52           ` Jeff Layton

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=13750c0b72dccd84e75179d62e9a9038d6f57371.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=lhenriques@suse.de \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    /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

CEPH-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/ceph-devel/0 ceph-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ceph-devel ceph-devel/ https://lore.kernel.org/ceph-devel \
		ceph-devel@vger.kernel.org
	public-inbox-index ceph-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.ceph-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git