* [PATCH v4] ovl: Check link ability between upperdir and workdir
@ 2017-12-20 2:09 Chengguang Xu
2018-01-19 16:25 ` Miklos Szeredi
0 siblings, 1 reply; 3+ messages in thread
From: Chengguang Xu @ 2017-12-20 2:09 UTC (permalink / raw)
To: miklos, amir73il, vgoyal; +Cc: linux-unionfs, Chengguang Xu
Inspired by encountering unexpected write error with
error code -EXDEV when upperdir and workdir having
different project quotas. The reason of the error is
project quota asks files in it strictly inherit
project id of it’s own, so rename/link operations
between those directories will fail and it may cause
failure during copy-up/index-link operations in overlayfs.
So if upper filesystem supports O_TMPFILE, try to check
link ability between upperdir and workdir and if check
fails, print a proper warning message to indicate potential
possibility of write failure.
The failure of the check does not directly lead to
read-only mounting because in some use cases may only
write to upperdir and do not modify anything in lowerdirs.
Signed-off-by: Chengguang Xu <cgxu519@icloud.com>
---
Changes since v3:
- Add more detail comment about motivation to changelog.
Changes since v2:
- Add comment about motivation.
- Modify warning message.
Changes since v1:
- Check link ablity between upperdir and workdir instead of checking
project ids of upperdir and workdir.
- Print a warning message instead of falling to readonly mode.
fs/overlayfs/super.c | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 76440fe..7d6bd82 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -929,7 +929,7 @@ static int ovl_get_upper(struct ovl_fs *ofs, struct path *upperpath)
static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
{
- struct dentry *temp;
+ struct dentry *uppertemp, *worktemp;
int err;
ofs->workdir = ovl_workdir_create(ofs, OVL_WORKDIR_NAME, false);
@@ -953,12 +953,28 @@ static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
if (!err)
pr_warn("overlayfs: upper fs needs to support d_type.\n");
- /* Check if upper/work fs supports O_TMPFILE */
- temp = ovl_do_tmpfile(ofs->workdir, S_IFREG | 0);
- ofs->tmpfile = !IS_ERR(temp);
- if (ofs->tmpfile)
- dput(temp);
- else
+ /*
+ * Check if upper/work fs supports O_TMPFILE and if support, check
+ * link ability between upperdir and workdir, this is inspired by
+ * encountering unexpected write error when upperdir and workdir
+ * having different project ids.
+ */
+ uppertemp = ovl_do_tmpfile(ofs->upper_mnt->mnt_root, S_IFREG | 0);
+ ofs->tmpfile = !IS_ERR(uppertemp);
+ if (ofs->tmpfile) {
+ worktemp = ovl_lookup_temp(ofs->workdir);
+ if (!IS_ERR(worktemp)) {
+ inode_lock_nested(ofs->workdir->d_inode, I_MUTEX_PARENT);
+ err = ovl_do_link(uppertemp, ofs->workdir->d_inode, worktemp, true);
+ inode_unlock(ofs->workdir->d_inode);
+ if (err)
+ pr_warn("overlayfs: cannot link files between upperdir and workdir, it may cause write error.\n");
+ else
+ ovl_cleanup(ofs->workdir->d_inode, worktemp);
+ dput(worktemp);
+ }
+ dput(uppertemp);
+ } else
pr_warn("overlayfs: upper fs does not support tmpfile.\n");
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v4] ovl: Check link ability between upperdir and workdir
2017-12-20 2:09 [PATCH v4] ovl: Check link ability between upperdir and workdir Chengguang Xu
@ 2018-01-19 16:25 ` Miklos Szeredi
2018-01-20 10:22 ` Chengguang Xu
0 siblings, 1 reply; 3+ messages in thread
From: Miklos Szeredi @ 2018-01-19 16:25 UTC (permalink / raw)
To: Chengguang Xu; +Cc: Amir Goldstein, Vivek Goyal, overlayfs
On Wed, Dec 20, 2017 at 3:09 AM, Chengguang Xu <cgxu519@icloud.com> wrote:
> Inspired by encountering unexpected write error with
> error code -EXDEV when upperdir and workdir having
> different project quotas. The reason of the error is
> project quota asks files in it strictly inherit
> project id of it’s own, so rename/link operations
> between those directories will fail and it may cause
> failure during copy-up/index-link operations in overlayfs.
>
> So if upper filesystem supports O_TMPFILE, try to check
> link ability between upperdir and workdir and if check
> fails, print a proper warning message to indicate potential
> possibility of write failure.
>
> The failure of the check does not directly lead to
> read-only mounting because in some use cases may only
> write to upperdir and do not modify anything in lowerdirs.
>
> Signed-off-by: Chengguang Xu <cgxu519@icloud.com>
> ---
> Changes since v3:
> - Add more detail comment about motivation to changelog.
>
> Changes since v2:
> - Add comment about motivation.
> - Modify warning message.
>
> Changes since v1:
> - Check link ablity between upperdir and workdir instead of checking
> project ids of upperdir and workdir.
> - Print a warning message instead of falling to readonly mode.
>
> fs/overlayfs/super.c | 30 +++++++++++++++++++++++-------
> 1 file changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 76440fe..7d6bd82 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -929,7 +929,7 @@ static int ovl_get_upper(struct ovl_fs *ofs, struct path *upperpath)
>
> static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
> {
> - struct dentry *temp;
> + struct dentry *uppertemp, *worktemp;
> int err;
>
> ofs->workdir = ovl_workdir_create(ofs, OVL_WORKDIR_NAME, false);
> @@ -953,12 +953,28 @@ static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
> if (!err)
> pr_warn("overlayfs: upper fs needs to support d_type.\n");
>
> - /* Check if upper/work fs supports O_TMPFILE */
> - temp = ovl_do_tmpfile(ofs->workdir, S_IFREG | 0);
> - ofs->tmpfile = !IS_ERR(temp);
> - if (ofs->tmpfile)
> - dput(temp);
> - else
> + /*
> + * Check if upper/work fs supports O_TMPFILE and if support, check
> + * link ability between upperdir and workdir, this is inspired by
> + * encountering unexpected write error when upperdir and workdir
> + * having different project ids.
> + */
> + uppertemp = ovl_do_tmpfile(ofs->upper_mnt->mnt_root, S_IFREG | 0);
> + ofs->tmpfile = !IS_ERR(uppertemp);
> + if (ofs->tmpfile) {
> + worktemp = ovl_lookup_temp(ofs->workdir);
Have you tested this? Because there's a warning at the top of
lookup_one_len() that triggers because of no locking of parent.
I can add the locking, it's trivial, but I'd still like you to test
this on your setup, if it actually does what you promise it does.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4] ovl: Check link ability between upperdir and workdir
2018-01-19 16:25 ` Miklos Szeredi
@ 2018-01-20 10:22 ` Chengguang Xu
0 siblings, 0 replies; 3+ messages in thread
From: Chengguang Xu @ 2018-01-20 10:22 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Amir Goldstein, Vivek Goyal, overlayfs
>
> 在 2018年1月20日,上午12:25,Miklos Szeredi <miklos@szeredi.hu> 写道:
>
> On Wed, Dec 20, 2017 at 3:09 AM, Chengguang Xu <cgxu519@icloud.com> wrote:
>> Inspired by encountering unexpected write error with
>> error code -EXDEV when upperdir and workdir having
>> different project quotas. The reason of the error is
>> project quota asks files in it strictly inherit
>> project id of it’s own, so rename/link operations
>> between those directories will fail and it may cause
>> failure during copy-up/index-link operations in overlayfs.
>>
>> So if upper filesystem supports O_TMPFILE, try to check
>> link ability between upperdir and workdir and if check
>> fails, print a proper warning message to indicate potential
>> possibility of write failure.
>>
>> The failure of the check does not directly lead to
>> read-only mounting because in some use cases may only
>> write to upperdir and do not modify anything in lowerdirs.
>>
>> Signed-off-by: Chengguang Xu <cgxu519@icloud.com>
>> ---
>> Changes since v3:
>> - Add more detail comment about motivation to changelog.
>>
>> Changes since v2:
>> - Add comment about motivation.
>> - Modify warning message.
>>
>> Changes since v1:
>> - Check link ablity between upperdir and workdir instead of checking
>> project ids of upperdir and workdir.
>> - Print a warning message instead of falling to readonly mode.
>>
>> fs/overlayfs/super.c | 30 +++++++++++++++++++++++-------
>> 1 file changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
>> index 76440fe..7d6bd82 100644
>> --- a/fs/overlayfs/super.c
>> +++ b/fs/overlayfs/super.c
>> @@ -929,7 +929,7 @@ static int ovl_get_upper(struct ovl_fs *ofs, struct path *upperpath)
>>
>> static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
>> {
>> - struct dentry *temp;
>> + struct dentry *uppertemp, *worktemp;
>> int err;
>>
>> ofs->workdir = ovl_workdir_create(ofs, OVL_WORKDIR_NAME, false);
>> @@ -953,12 +953,28 @@ static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
>> if (!err)
>> pr_warn("overlayfs: upper fs needs to support d_type.\n");
>>
>> - /* Check if upper/work fs supports O_TMPFILE */
>> - temp = ovl_do_tmpfile(ofs->workdir, S_IFREG | 0);
>> - ofs->tmpfile = !IS_ERR(temp);
>> - if (ofs->tmpfile)
>> - dput(temp);
>> - else
>> + /*
>> + * Check if upper/work fs supports O_TMPFILE and if support, check
>> + * link ability between upperdir and workdir, this is inspired by
>> + * encountering unexpected write error when upperdir and workdir
>> + * having different project ids.
>> + */
>> + uppertemp = ovl_do_tmpfile(ofs->upper_mnt->mnt_root, S_IFREG | 0);
>> + ofs->tmpfile = !IS_ERR(uppertemp);
>> + if (ofs->tmpfile) {
>> + worktemp = ovl_lookup_temp(ofs->workdir);
>
> Have you tested this? Because there's a warning at the top of
> lookup_one_len() that triggers because of no locking of parent.
>
> I can add the locking, it's trivial, but I'd still like you to test
> this on your setup, if it actually does what you promise it does.
Hi Miklos,
Sorry that I could not reproduce it on my setup for the patch because
the test vm has already reclaimed, but I can see the warning in my new
test vm, so I guess maybe ‘DEBUG_BUGVERBOSE’ configuration was not
properly set in the previous test setup.
Thanks,
Chengguang.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-01-20 10:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-20 2:09 [PATCH v4] ovl: Check link ability between upperdir and workdir Chengguang Xu
2018-01-19 16:25 ` Miklos Szeredi
2018-01-20 10:22 ` Chengguang Xu
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.