linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Tao Ma <tm@tao.ma>
Cc: linux-ext4@vger.kernel.org
Subject: Re: [PATCH V7 00/23] ext4: Add inline data support
Date: Thu, 6 Dec 2012 12:30:22 -0500	[thread overview]
Message-ID: <20121206173022.GE30273@thunk.org> (raw)
In-Reply-To: <1351047002-4723-1-git-send-email-tm@tao.ma>

Hi Tao,

I've been going over the patch series, and one of the things that I
started thinking about was whether we really needed the inline_data
flag, strictly speaking.  After all, the information will be in the
extended attribute space in the inode table, so it's relatively cheap
to search the ibody looking for the extended attribute, and then to
cache that information in a state flag.  That might be a useful thing
to do, so I'd welcome your comments about whether you think this would
be a good idea.

But this brought up a bigger issue, which is the one downside of doing
this is the potential locking overhead before fetching the information
out of the extended attribute --- and that's when I realized the
locking is currently *missing*.  At the moment, the VFS layer takes
i_mutex before calling the per-filesystem extended attribute
functions, and that takes care of the locking.  However, the inline
data patches call ext4_xattr_ibody_find() and ext4_xattr_ibody_set()
directly, and they are not taking the i_mutex first.

Hence, we have potential races between the a process setting an
userspace extended attribute, or selinux setting a security.* xattr,
and the new code which tries reading or writing inline data.

Could you take a look at this, please?

Many thanks,

						- Ted

  parent reply	other threads:[~2012-12-06 17:30 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-24  2:50 [PATCH V7 00/23] ext4: Add inline data support Tao Ma
2012-10-24  2:55 ` [PATCH V7 01/23] ext4: Move extra inode read to a new function Tao Ma
2012-10-24  2:55   ` [PATCH V7 02/23] ext4: export inline xattr functions Tao Ma
2012-10-24  2:55   ` [PATCH V7 03/23] ext4: Add the basic function for inline data support Tao Ma
2012-12-03  1:48     ` Theodore Ts'o
2012-12-03  5:23       ` Tao Ma
2012-12-03 16:17       ` Tao Ma
2012-10-24  2:55   ` [PATCH V7 04/23] ext4: Add read support for inline data Tao Ma
2012-10-24  2:55   ` [PATCH V7 05/23] ext4: Add normal write " Tao Ma
2012-10-24  2:55   ` [PATCH V7 06/23] ext4: Add journalled " Tao Ma
2012-10-24  2:55   ` [PATCH V7 07/23] ext4: Add delalloc " Tao Ma
2012-10-24  2:55   ` [PATCH V7 08/23] ext4: Make ext4_init_dot_dotdot for inline dir usage Tao Ma
2012-10-24  2:55   ` [PATCH V7 09/23] ext4: Refactor __ext4_check_dir_entry to accepts start and size Tao Ma
2012-10-24  2:55   ` [PATCH V7 10/23] ext4: Create __ext4_insert_dentry for dir entry insertion Tao Ma
2012-10-24  2:55   ` [PATCH V7 11/23] ext4: let add_dir_entry handle inline data properly Tao Ma
2012-10-24  2:55   ` [PATCH V7 12/23] ext4: Let ext4_readdir handle inline data Tao Ma
2012-10-24  2:55   ` [PATCH V7 13/23] ext4: Create a new function search_dir Tao Ma
2012-10-24  2:55   ` [PATCH V7 14/23] ext4: let ext4_find_entry handle inline data Tao Ma
2012-10-24  2:55   ` [PATCH V7 15/23] ext4: make ext4_delete_entry generic Tao Ma
2012-10-24  2:55   ` [PATCH V7 16/23] ext4: let ext4_delete_entry handle inline data Tao Ma
2012-10-24  2:55   ` [PATCH V7 17/23] ext4: let empty_dir handle inline dir Tao Ma
2012-10-24  2:55   ` [PATCH V7 18/23] ext4: let ext4_rename " Tao Ma
2012-10-24  2:55   ` [PATCH V7 19/23] ext4: Let fiemap work with inline data Tao Ma
2012-10-24  2:55   ` [PATCH V7 20/23] ext4: Evict inline data out if we needs to strore xattr in inode Tao Ma
2012-10-24  2:55   ` [PATCH V7 21/23] ext4: let ext4_truncate handle inline data correctly Tao Ma
2012-10-24  2:55   ` [PATCH V7 22/23] ext4: let fallocate " Tao Ma
2012-12-03  1:37     ` Theodore Ts'o
2012-10-24  2:55   ` [PATCH V7 23/23] ext4: Enable ext4 inline support Tao Ma
2012-11-19  7:41 ` [PATCH V7 00/23] ext4: Add inline data support Tao Ma
2012-11-19 15:29   ` Theodore Ts'o
2012-12-06 17:30 ` Theodore Ts'o [this message]
2012-12-07  1:34   ` Tao Ma
2012-12-10 15:02     ` Theodore Ts'o
2012-12-10 15:17       ` Tao Ma

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=20121206173022.GE30273@thunk.org \
    --to=tytso@mit.edu \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tm@tao.ma \
    /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).