From: Anna Schumaker <Anna.Schumaker@netapp.com> To: "Pádraig Brady" <P@draigBrady.com>, "Neil Brown" <nfbrown@novell.com>, linux-nfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org, zab@zabbo.net, viro@zeniv.linux.org.uk, clm@fb.com, darrick.wong@oracle.com, mtk.manpages@gmail.com, andros@netapp.com, hch@infradead.org Subject: Re: [PATCH v5 8/9] vfs: Add vfs_copy_file_range() support for pagecache copies Date: Tue, 13 Oct 2015 16:25:29 -0400 [thread overview] Message-ID: <561D68B9.8030209@Netapp.com> (raw) In-Reply-To: <5617A1E9.1080509@draigBrady.com> On 10/09/2015 07:15 AM, Pádraig Brady wrote: > On 08/10/15 02:40, Neil Brown wrote: >> Anna Schumaker <Anna.Schumaker@netapp.com> writes: >> >>> @@ -1338,34 +1362,26 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, >>> struct file *file_out, loff_t pos_out, >>> size_t len, unsigned int flags) >>> { >>> - struct inode *inode_in; >>> - struct inode *inode_out; >>> ssize_t ret; >>> >>> - if (flags) >>> + /* Flags should only be used exclusively. */ >>> + if ((flags & COPY_FR_COPY) && (flags & ~COPY_FR_COPY)) >>> + return -EINVAL; >>> + if ((flags & COPY_FR_REFLINK) && (flags & ~COPY_FR_REFLINK)) >>> + return -EINVAL; >>> + if ((flags & COPY_FR_DEDUP) && (flags & ~COPY_FR_DEDUP)) >>> return -EINVAL; >>> >> >> Do you also need: >> >> if (flags & ~(COPY_FR_COPY | COPY_FR_REFLINK | COPY_FR_DEDUP)) >> return -EINVAL; >> >> so that future user-space can test if the kernel supports new flags? > > Seems like a good idea, yes. > > Also that got me thinking about COPY_FR_SPARSE. > What's the current behavior when copying a sparse range? > Is the hole propagated by default (good), or is it expanded? I haven't tried it, but I think the hole would be expanded :(. I'm having splice() handle the pagecache copy part, and (as far as I know) splice() doesn't know anything about sparse files. I might be able to put in some kind of fallocate() / splice() loop to copy the range in multiple pieces. I don't want to add COPY_FR_SPARSE_AUTO, because then the kernel will have to determine how best to interpret "auto". I'm more inclined to add a single COPY_FR_SPARSE flag to enable creating sparse files, and then have the application tell us what to do for any given range. Anna > > Note cp(1) has --sparse={never,auto,always}. Auto is the default, > so it would be good I think if that was the default mode for copy_file_range(). > With other sparse modes, we'd have to avoid copy_file_range() unless > there was control possible with COPY_FR_SPARSE_{AUTO,NONE,ALWAYS}. > Note currently cp --sparse=always will detect runs of zeros and also > avoid speculative preallocation by using fallocate (fd, FALLOC_FL_PUNCH_HOLE, ...) > > thanks, > Pádraig. >
WARNING: multiple messages have this Message-ID (diff)
From: Anna Schumaker <Anna.Schumaker-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org> To: "Pádraig Brady" <P@draigBrady.com>, "Neil Brown" <nfbrown-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-btrfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, zab-ugsP4Wv/S6ZeoWH0uzbU5w@public.gmane.org, viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org, clm-b10kYP2dOMg@public.gmane.org, darrick.wong-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org, mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, andros-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org, hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org Subject: Re: [PATCH v5 8/9] vfs: Add vfs_copy_file_range() support for pagecache copies Date: Tue, 13 Oct 2015 16:25:29 -0400 [thread overview] Message-ID: <561D68B9.8030209@Netapp.com> (raw) In-Reply-To: <5617A1E9.1080509-V8g9lnOeT5ydJdNcDFJN0w@public.gmane.org> On 10/09/2015 07:15 AM, Pádraig Brady wrote: > On 08/10/15 02:40, Neil Brown wrote: >> Anna Schumaker <Anna.Schumaker-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org> writes: >> >>> @@ -1338,34 +1362,26 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, >>> struct file *file_out, loff_t pos_out, >>> size_t len, unsigned int flags) >>> { >>> - struct inode *inode_in; >>> - struct inode *inode_out; >>> ssize_t ret; >>> >>> - if (flags) >>> + /* Flags should only be used exclusively. */ >>> + if ((flags & COPY_FR_COPY) && (flags & ~COPY_FR_COPY)) >>> + return -EINVAL; >>> + if ((flags & COPY_FR_REFLINK) && (flags & ~COPY_FR_REFLINK)) >>> + return -EINVAL; >>> + if ((flags & COPY_FR_DEDUP) && (flags & ~COPY_FR_DEDUP)) >>> return -EINVAL; >>> >> >> Do you also need: >> >> if (flags & ~(COPY_FR_COPY | COPY_FR_REFLINK | COPY_FR_DEDUP)) >> return -EINVAL; >> >> so that future user-space can test if the kernel supports new flags? > > Seems like a good idea, yes. > > Also that got me thinking about COPY_FR_SPARSE. > What's the current behavior when copying a sparse range? > Is the hole propagated by default (good), or is it expanded? I haven't tried it, but I think the hole would be expanded :(. I'm having splice() handle the pagecache copy part, and (as far as I know) splice() doesn't know anything about sparse files. I might be able to put in some kind of fallocate() / splice() loop to copy the range in multiple pieces. I don't want to add COPY_FR_SPARSE_AUTO, because then the kernel will have to determine how best to interpret "auto". I'm more inclined to add a single COPY_FR_SPARSE flag to enable creating sparse files, and then have the application tell us what to do for any given range. Anna > > Note cp(1) has --sparse={never,auto,always}. Auto is the default, > so it would be good I think if that was the default mode for copy_file_range(). > With other sparse modes, we'd have to avoid copy_file_range() unless > there was control possible with COPY_FR_SPARSE_{AUTO,NONE,ALWAYS}. > Note currently cp --sparse=always will detect runs of zeros and also > avoid speculative preallocation by using fallocate (fd, FALLOC_FL_PUNCH_HOLE, ...) > > thanks, > Pádraig. >
next prev parent reply other threads:[~2015-10-13 20:25 UTC|newest] Thread overview: 129+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-09-30 17:26 [PATCH v5 0/9] VFS: In-kernel copy system call Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-09-30 17:26 ` [PATCH v5 1/9] vfs: add copy_file_range syscall and vfs helper Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-09-30 17:26 ` [PATCH v5 2/9] x86: add sys_copy_file_range to syscall tables Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-09-30 17:26 ` [PATCH v5 3/9] btrfs: add .copy_file_range file operation Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-09-30 17:26 ` [PATCH v5 4/9] vfs: Copy should check len after file open mode Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-10-11 14:22 ` Christoph Hellwig 2015-10-11 14:22 ` Christoph Hellwig 2015-09-30 17:26 ` [PATCH v5 5/9] vfs: Copy shouldn't forbid ranges inside the same file Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-10-11 14:22 ` Christoph Hellwig 2015-10-14 17:37 ` Anna Schumaker 2015-10-14 17:37 ` Anna Schumaker 2015-10-14 17:37 ` Anna Schumaker 2015-10-14 18:25 ` Christoph Hellwig 2015-10-14 18:27 ` Anna Schumaker 2015-10-14 18:27 ` Anna Schumaker 2015-10-14 18:27 ` Anna Schumaker 2015-09-30 17:26 ` [PATCH v5 6/9] vfs: Copy should use file_out rather than file_in Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-10-11 14:24 ` Christoph Hellwig 2015-09-30 17:26 ` [PATCH v5 7/9] vfs: Remove copy_file_range mountpoint checks Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-10-11 14:23 ` Christoph Hellwig 2015-10-14 17:41 ` Anna Schumaker 2015-10-14 17:41 ` Anna Schumaker 2015-10-14 18:25 ` Christoph Hellwig 2015-10-14 18:25 ` Christoph Hellwig 2015-09-30 17:26 ` [PATCH v5 8/9] vfs: Add vfs_copy_file_range() support for pagecache copies Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-10-08 1:40 ` Neil Brown 2015-10-09 11:15 ` Pádraig Brady 2015-10-09 11:15 ` Pádraig Brady 2015-10-13 20:25 ` Anna Schumaker [this message] 2015-10-13 20:25 ` Anna Schumaker 2015-10-14 7:41 ` Christoph Hellwig 2015-10-14 7:41 ` Christoph Hellwig 2015-10-13 19:45 ` Anna Schumaker 2015-10-13 19:45 ` Anna Schumaker 2015-10-13 19:45 ` Anna Schumaker 2015-10-11 14:22 ` Christoph Hellwig 2015-10-11 14:22 ` Christoph Hellwig 2015-10-12 23:17 ` Darrick J. Wong 2015-10-12 23:17 ` Darrick J. Wong 2015-10-13 3:36 ` Trond Myklebust 2015-10-13 7:19 ` Darrick J. Wong 2015-10-13 7:19 ` Darrick J. Wong 2015-10-13 7:30 ` Christoph Hellwig 2015-10-13 7:30 ` Christoph Hellwig 2015-10-13 7:27 ` Christoph Hellwig 2015-10-13 7:27 ` Christoph Hellwig 2015-11-10 6:24 ` Darrick J. Wong 2015-10-14 17:59 ` Anna Schumaker 2015-10-14 17:59 ` Anna Schumaker 2015-10-14 17:59 ` Anna Schumaker 2015-10-14 18:08 ` Andy Lutomirski 2015-10-14 18:27 ` Christoph Hellwig 2015-10-14 18:38 ` Andy Lutomirski 2015-10-14 18:38 ` Andy Lutomirski 2015-10-14 18:49 ` Christoph Hellwig 2015-10-14 18:49 ` Christoph Hellwig 2015-10-14 18:53 ` Andy Lutomirski 2015-10-14 19:14 ` Austin S Hemmelgarn 2015-10-14 19:14 ` Austin S Hemmelgarn 2015-10-14 19:39 ` Pádraig Brady 2015-10-14 19:39 ` Pádraig Brady 2015-10-15 5:56 ` Christoph Hellwig 2015-10-15 5:56 ` Christoph Hellwig 2015-10-14 19:08 ` Austin S Hemmelgarn 2015-10-14 19:08 ` Austin S Hemmelgarn 2015-10-15 6:36 ` Christoph Hellwig 2015-10-15 6:36 ` Christoph Hellwig 2015-10-15 12:24 ` Austin S Hemmelgarn 2015-10-15 12:24 ` Austin S Hemmelgarn 2015-10-16 5:38 ` Christoph Hellwig 2015-10-16 5:38 ` Christoph Hellwig 2015-10-16 11:46 ` Austin S Hemmelgarn 2015-10-16 12:02 ` Pádraig Brady 2015-10-16 12:02 ` Pádraig Brady 2015-10-16 12:24 ` Christoph Hellwig 2015-10-16 12:24 ` Christoph Hellwig 2015-10-16 12:46 ` Austin S Hemmelgarn 2015-10-16 12:46 ` Austin S Hemmelgarn 2015-10-16 12:21 ` Christoph Hellwig 2015-10-16 12:21 ` Christoph Hellwig 2015-10-16 12:50 ` Austin S Hemmelgarn 2015-10-16 13:12 ` Christoph Hellwig 2015-10-16 14:11 ` Austin S Hemmelgarn 2015-10-14 18:11 ` Darrick J. Wong 2015-10-14 18:11 ` Darrick J. Wong 2015-10-14 18:26 ` Andy Lutomirski 2015-09-30 17:26 ` [PATCH v5 9/9] btrfs: btrfs_copy_file_range() only supports reflinks Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-10-11 14:29 ` Christoph Hellwig 2015-10-11 14:29 ` Christoph Hellwig 2015-10-12 10:23 ` Pádraig Brady 2015-10-12 10:23 ` Pádraig Brady 2015-10-12 14:34 ` Christoph Hellwig 2015-10-12 23:41 ` Darrick J. Wong 2015-10-12 23:41 ` Darrick J. Wong 2015-10-13 7:29 ` Christoph Hellwig 2015-10-13 7:29 ` Christoph Hellwig 2015-10-14 18:46 ` Darrick J. Wong 2015-10-14 18:46 ` Darrick J. Wong 2015-10-15 6:00 ` Christoph Hellwig 2015-10-16 11:49 ` Chris Mason 2015-10-16 11:49 ` Chris Mason 2015-10-16 11:49 ` Chris Mason 2015-10-16 12:25 ` Christoph Hellwig 2015-10-16 12:25 ` Christoph Hellwig 2015-10-16 13:19 ` Chris Mason 2015-10-16 13:19 ` Chris Mason 2015-10-16 13:19 ` Chris Mason 2015-10-16 21:44 ` Dave Chinner 2015-10-17 13:44 ` Chris Mason 2015-10-17 13:44 ` Chris Mason 2015-10-17 13:44 ` Chris Mason 2015-10-15 8:35 ` Dave Chinner 2015-10-15 8:35 ` Dave Chinner 2015-09-30 17:26 ` [PATCH v5 10/9] copy_file_range.2: New page documenting copy_file_range() Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker 2015-09-30 17:26 ` Anna Schumaker
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=561D68B9.8030209@Netapp.com \ --to=anna.schumaker@netapp.com \ --cc=P@draigBrady.com \ --cc=andros@netapp.com \ --cc=clm@fb.com \ --cc=darrick.wong@oracle.com \ --cc=hch@infradead.org \ --cc=linux-api@vger.kernel.org \ --cc=linux-btrfs@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-nfs@vger.kernel.org \ --cc=mtk.manpages@gmail.com \ --cc=nfbrown@novell.com \ --cc=viro@zeniv.linux.org.uk \ --cc=zab@zabbo.net \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.