[PATCH] splice: update mtime and atime on files

[PATCH] splice: update mtime and atime on files

[Miklos Szeredi]

From: Miklos Szeredi <mszeredi@suse.cz>

Splice should update the modification and access times on regular
files just like read and write.  Not updating mtime will confuse
backup tools, etc...

This patch only adds the time updates for regular files.  For pipes
and other special files that splice touches the need for updating the
times is less clear.  Let's discuss and fix that separately.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/splice.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c	2009-07-03 13:12:06.000000000 +0200
+++ linux-2.6/fs/splice.c	2009-07-21 17:58:52.000000000 +0200
@@ -502,8 +502,10 @@ ssize_t generic_file_splice_read(struct
 		len = left;
 
 	ret = __generic_file_splice_read(in, ppos, pipe, len, flags);
-	if (ret > 0)
+	if (ret > 0) {
 		*ppos += ret;
+		file_accessed(in);
+	}
 
 	return ret;
 }
@@ -963,8 +965,10 @@ generic_file_splice_write(struct pipe_in
 
 		mutex_lock_nested(&inode->i_mutex, I_MUTEX_CHILD);
 		ret = file_remove_suid(out);
-		if (!ret)
+		if (!ret) {
+			file_update_time(out);
 			ret = splice_from_pipe_feed(pipe, &sd, pipe_to_file);
+		}
 		mutex_unlock(&inode->i_mutex);
 	} while (ret > 0);
 	splice_from_pipe_end(pipe, &sd);

Re: [PATCH] splice: update mtime and atime on files

[Jens Axboe]

On Fri, Aug 14 2009, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Splice should update the modification and access times on regular
> files just like read and write.  Not updating mtime will confuse
> backup tools, etc...
> 
> This patch only adds the time updates for regular files.  For pipes
> and other special files that splice touches the need for updating the
> times is less clear.  Let's discuss and fix that separately.

Thanks Miklos, I've queued this one up.

-- 
Jens Axboe

Re: [PATCH] splice: update mtime and atime on files

[Willy Tarreau]

On Sat, Aug 15, 2009 at 08:42:41AM +0200, Jens Axboe wrote:
> On Fri, Aug 14 2009, Miklos Szeredi wrote:
> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > Splice should update the modification and access times on regular
> > files just like read and write.  Not updating mtime will confuse
> > backup tools, etc...
> > 
> > This patch only adds the time updates for regular files.  For pipes
> > and other special files that splice touches the need for updating the
> > times is less clear.  Let's discuss and fix that separately.
> 
> Thanks Miklos, I've queued this one up.

wouldn't it make sense to send that to stable too ? The patch is not
intrusive and looks like a fix to me.

Willy

Re: [PATCH] splice: update mtime and atime on files

[Jens Axboe]

On Tue, Aug 18 2009, Willy Tarreau wrote:
> On Sat, Aug 15, 2009 at 08:42:41AM +0200, Jens Axboe wrote:
> > On Fri, Aug 14 2009, Miklos Szeredi wrote:
> > > From: Miklos Szeredi <mszeredi@suse.cz>
> > > 
> > > Splice should update the modification and access times on regular
> > > files just like read and write.  Not updating mtime will confuse
> > > backup tools, etc...
> > > 
> > > This patch only adds the time updates for regular files.  For pipes
> > > and other special files that splice touches the need for updating the
> > > times is less clear.  Let's discuss and fix that separately.
> > 
> > Thanks Miklos, I've queued this one up.
> 
> wouldn't it make sense to send that to stable too ? The patch is not
> intrusive and looks like a fix to me.

It doesn't really meet the stable criteria, as it's not a security issue
nor does it crash the kernel.

-- 
Jens Axboe

Re: [PATCH] splice: update mtime and atime on files

[Willy Tarreau]

On Tue, Aug 18, 2009 at 10:35:48AM +0200, Jens Axboe wrote:
> On Tue, Aug 18 2009, Willy Tarreau wrote:
> > On Sat, Aug 15, 2009 at 08:42:41AM +0200, Jens Axboe wrote:
> > > On Fri, Aug 14 2009, Miklos Szeredi wrote:
> > > > From: Miklos Szeredi <mszeredi@suse.cz>
> > > > 
> > > > Splice should update the modification and access times on regular
> > > > files just like read and write.  Not updating mtime will confuse
> > > > backup tools, etc...
> > > > 
> > > > This patch only adds the time updates for regular files.  For pipes
> > > > and other special files that splice touches the need for updating the
> > > > times is less clear.  Let's discuss and fix that separately.
> > > 
> > > Thanks Miklos, I've queued this one up.
> > 
> > wouldn't it make sense to send that to stable too ? The patch is not
> > intrusive and looks like a fix to me.
> 
> It doesn't really meet the stable criteria, as it's not a security issue
> nor does it crash the kernel.

Oh there are many other things which go to stable, especially bug
fixes and minor improvements. We *could* classify this in the bug
fixes as, I Miklos suggested it, this could have an impact on
modified files which would currently not be backed up for instance.

Regards,
Willy

Re: [PATCH] splice: update mtime and atime on files

[Jens Axboe]

On Tue, Aug 18 2009, Willy Tarreau wrote:
> On Tue, Aug 18, 2009 at 10:35:48AM +0200, Jens Axboe wrote:
> > On Tue, Aug 18 2009, Willy Tarreau wrote:
> > > On Sat, Aug 15, 2009 at 08:42:41AM +0200, Jens Axboe wrote:
> > > > On Fri, Aug 14 2009, Miklos Szeredi wrote:
> > > > > From: Miklos Szeredi <mszeredi@suse.cz>
> > > > > 
> > > > > Splice should update the modification and access times on regular
> > > > > files just like read and write.  Not updating mtime will confuse
> > > > > backup tools, etc...
> > > > > 
> > > > > This patch only adds the time updates for regular files.  For pipes
> > > > > and other special files that splice touches the need for updating the
> > > > > times is less clear.  Let's discuss and fix that separately.
> > > > 
> > > > Thanks Miklos, I've queued this one up.
> > > 
> > > wouldn't it make sense to send that to stable too ? The patch is not
> > > intrusive and looks like a fix to me.
> > 
> > It doesn't really meet the stable criteria, as it's not a security issue
> > nor does it crash the kernel.
> 
> Oh there are many other things which go to stable, especially bug

I'm aware of that, doesn't mean it's what the rules describe or that
it's necessarily a good idea :-)

> fixes and minor improvements. We *could* classify this in the bug
> fixes as, I Miklos suggested it, this could have an impact on
> modified files which would currently not be backed up for instance.

I've never heard anyone complain about this, and I suspect that Miklos
found it from code inspection rather than experiencing an issue with it.
So while it can indeed be classified as a bug (and it is), the impact is
not huge.

That said, I don't have a huge issue with shoving this in -stable. I
just don't think it's a big deal.

-- 
Jens Axboe

Re: [PATCH] splice: update mtime and atime on files

[Miklos Szeredi]

On Tue, 18 Aug 2009, Jens Axboe wrote:
> I've never heard anyone complain about this, and I suspect that Miklos
> found it from code inspection rather than experiencing an issue with it.

Right, I found it by code inspection.

> So while it can indeed be classified as a bug (and it is), the impact is
> not huge.
> 
> That said, I don't have a huge issue with shoving this in -stable. I
> just don't think it's a big deal.

I agree that it's not a huge issue, however it might be worth it to
add it to -stable for the sake of distro kernels which are usually
based on the long term stable branch.  If new apps start to use the
splice interface then we want those to work well on distros as well as
the latest kernel.org kernels.

Thanks,
Miklos

Re: [PATCH] splice: update mtime and atime on files

[Jens Axboe]

On Tue, Aug 18 2009, Miklos Szeredi wrote:
> On Tue, 18 Aug 2009, Jens Axboe wrote:
> > I've never heard anyone complain about this, and I suspect that Miklos
> > found it from code inspection rather than experiencing an issue with it.
> 
> Right, I found it by code inspection.
> 
> > So while it can indeed be classified as a bug (and it is), the impact is
> > not huge.
> > 
> > That said, I don't have a huge issue with shoving this in -stable. I
> > just don't think it's a big deal.
> 
> I agree that it's not a huge issue, however it might be worth it to
> add it to -stable for the sake of distro kernels which are usually
> based on the long term stable branch.  If new apps start to use the
> splice interface then we want those to work well on distros as well as
> the latest kernel.org kernels.

As I said, I don't have a problem with adding it to -stable, even if it
isn't a critical issue. The patch is straight forward and fixes a real
issue, so risk is low (and benefit high).

-- 
Jens Axboe