From: Tao Ma <tm@tao.ma>
To: Theodore Ts'o <tytso@mit.edu>
Cc: linux-ext4@vger.kernel.org
Subject: Re: [PATCH V7 00/23] ext4: Add inline data support
Date: Fri, 07 Dec 2012 09:34:24 +0800 [thread overview]
Message-ID: <50C147A0.4070109@tao.ma> (raw)
In-Reply-To: <20121206173022.GE30273@thunk.org>
Hi Ted,
On 12/07/2012 01:30 AM, Theodore Ts'o wrote:
> 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.
I was thinking of this, but that would make e2fsck hard to tell whether
the inline data is OK or not. Currently, we can make sure it is inline
data if both the flag and the "system.data" are set correctly. Without
this flag, So what do you think we should do in e2fsck? Having said
that, yes, in the kernel side, there are only 3 places using/checking
this flag and it can be removed easily.
>
> 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.
I think ext4_inode->xattr_sem should protect us? When we do xattr
corresponding operation, we just do
down_write/read(&EXT4_I(inode)->xattr_sem), so we should be fine with
this type of operation. Am I missing something here?
Thanks
Tao
>
> Could you take a look at this, please?
>
> Many thanks,
>
> - Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
next prev parent reply other threads:[~2012-12-07 1:34 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
2012-12-07 1:34 ` Tao Ma [this message]
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=50C147A0.4070109@tao.ma \
--to=tm@tao.ma \
--cc=linux-ext4@vger.kernel.org \
--cc=tytso@mit.edu \
/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).