Following this email are three patches against overlayfs.v17 which allow
it to work against v3.10-rc6 and later. With these applied overlayfs
passes basic testing.
overlayfs -- ovl_path_open should not take path reference
Found this in testing on Ubuntu raring, testing against loopback
mounted files. Without this change we were unable to release the
loopback device for reuse. Looking at it actually we were also leaking
references on the root filesystem, but these are not as obvious.
vfs: export do_splice_direct() to modules -- fix
overlayfs -- follow change to do_splice_direct interface
This pair switch us to the new do_splice_direct interface. Note that
the first one is effectivly a fix for the existing export of this
interface and likely should be merged down into it.
Please consider for your overlayfs branch.
-apw
The commit below moved the prototype for do_splice_direct to
fs/internal.h expose it:
commit 7995bd287134f6c8f80d94bebe7396f05a9bc42b
Author: Al Viro <[email protected]>
Date: Thu Jun 20 18:58:36 2013 +0400
splice: don't pass the address of ->f_pos to methods
[This should be merged down into:
vfs: export do_splice_direct() to modules.]
Signed-off-by: Andy Whitcroft <[email protected]>
---
fs/internal.h | 6 ------
include/linux/fs.h | 6 ++++++
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/internal.h b/fs/internal.h
index 6dd0ffd..6c9ec69 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -127,12 +127,6 @@ extern struct dentry *__d_alloc(struct super_block *, const struct qstr *);
extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
/*
- * splice.c
- */
-extern long do_splice_direct(struct file *in, loff_t *ppos, struct file *out,
- loff_t *opos, size_t len, unsigned int flags);
-
-/*
* pipe.c
*/
extern const struct file_operations pipefifo_fops;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e623f11..baf1175 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2703,4 +2703,10 @@ static inline void inode_has_no_xattr(struct inode *inode)
inode->i_flags |= S_NOSEC;
}
+/*
+ * splice.c
+ */
+extern long do_splice_direct(struct file *in, loff_t *ppos, struct file *out,
+ loff_t *opos, size_t len, unsigned int flags);
+
#endif /* _LINUX_FS_H */
--
1.8.3.1
The commit below changed the interface to do_splice_direct, follow that
change in copy_up:
commit 7995bd287134f6c8f80d94bebe7396f05a9bc42b
Author: Al Viro <[email protected]>
Date: Thu Jun 20 18:58:36 2013 +0400
splice: don't pass the address of ->f_pos to methods
Signed-off-by: Andy Whitcroft <[email protected]>
---
fs/overlayfs/copy_up.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index eef85e0..8e1b09f 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -90,9 +90,10 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
/* FIXME: copy up sparse files efficiently */
while (len) {
- loff_t offset = new_file->f_pos;
size_t this_len = OVL_COPY_UP_CHUNK_SIZE;
long bytes;
+ loff_t pos = old_file->f_pos;
+ loff_t out_pos = new_file->f_pos;
if (len < this_len)
this_len = len;
@@ -102,7 +103,7 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
break;
}
- bytes = do_splice_direct(old_file, &offset, new_file, this_len,
+ bytes = do_splice_direct(old_file, &pos, new_file, &out_pos, this_len,
SPLICE_F_MOVE);
if (bytes <= 0) {
error = bytes;
--
1.8.3.1
Since the commit below dentry_open now takes its own references
as required. We therefore should no longer take path references in
ovl_path_open. Doing so leaves stray mount references to the underlying
devices preventing them being released:
commit 765927b2d508712d320c8934db963bbe14c3fcec
Author: Al Viro <[email protected]>
Date: Tue Jun 26 21:58:53 2012 +0400
switch dentry_open() to struct path, make it grab references itself
BugLink: http://bugs.launchpad.net/bugs/1098378
Signed-off-by: Andy Whitcroft <[email protected]>
---
fs/overlayfs/super.c | 1 -
1 file changed, 1 deletion(-)
Found this in testing on Ubuntu raring, testing against loopback
mounted files. Without this change we were unable to release the
loopback device for reuse. Looking at it actually we were also leaking
references on the root filesystem, but these are not as obvious.
Applies against overlayfs.v17 as rebased to 3.8 and later.
-apw
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 482c26f..9473e79 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -385,7 +385,6 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
struct file *ovl_path_open(struct path *path, int flags)
{
- path_get(path);
return dentry_open(path, flags, current_cred());
}
--
1.8.3.1
Thanks for the patches, Andy. I already pushed a similar set of fixes
to overlayfs.current (.v18) and to .v17 for the dentry_open() refcount
fix.
On Thu, Jun 27, 2013 at 6:26 PM, Andy Whitcroft <[email protected]> wrote:
> The commit below changed the interface to do_splice_direct, follow that
> change in copy_up:
>
> commit 7995bd287134f6c8f80d94bebe7396f05a9bc42b
> Author: Al Viro <[email protected]>
> Date: Thu Jun 20 18:58:36 2013 +0400
>
> splice: don't pass the address of ->f_pos to methods
>
> Signed-off-by: Andy Whitcroft <[email protected]>
> ---
> fs/overlayfs/copy_up.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index eef85e0..8e1b09f 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -90,9 +90,10 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
>
> /* FIXME: copy up sparse files efficiently */
> while (len) {
> - loff_t offset = new_file->f_pos;
> size_t this_len = OVL_COPY_UP_CHUNK_SIZE;
> long bytes;
> + loff_t pos = old_file->f_pos;
> + loff_t out_pos = new_file->f_pos;
>
> if (len < this_len)
> this_len = len;
> @@ -102,7 +103,7 @@ static int ovl_copy_up_data(struct path *old, struct path *new, loff_t len)
> break;
> }
>
> - bytes = do_splice_direct(old_file, &offset, new_file, this_len,
> + bytes = do_splice_direct(old_file, &pos, new_file, &out_pos, this_len,
It is interesting how many people end up fixing this the wrong way ;)
Hint: how will the file offsets advance?
Thanks,
Miklos
On Thu, Jun 27, 2013 at 06:37:45PM +0200, Miklos Szeredi wrote:
> Thanks for the patches, Andy. I already pushed a similar set of fixes
> to overlayfs.current (.v18) and to .v17 for the dentry_open() refcount
> fix.
Bah so you did, that'll teach me for waiting for them to be tested.
> It is interesting how many people end up fixing this the wrong way ;)
>
> Hint: how will the file offsets advance?
Interesting I got bluffed in by the original implementation which
reloaded the source pos each time.
-apw