Return-Path: Received: from mail-io0-f182.google.com ([209.85.223.182]:32926 "EHLO mail-io0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754285AbdDRR2k (ORCPT ); Tue, 18 Apr 2017 13:28:40 -0400 Received: by mail-io0-f182.google.com with SMTP id k87so4830336ioi.0 for ; Tue, 18 Apr 2017 10:28:40 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20170413174515.GA5140@fieldses.org> <59BDF7CA-78BB-42FC-8BC4-95101F5E1EC7@netapp.com> <20170417133604.GA22694@fieldses.org> From: Olga Kornievskaia Date: Tue, 18 Apr 2017 13:28:39 -0400 Message-ID: Subject: Re: [nfsv4] Inter server-side copy performance To: Anna Schumaker Cc: "J. Bruce Fields" , "linux-nfs@vger.kernel.org" , "nfsv4@ietf.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-nfs-owner@vger.kernel.org List-ID: On Mon, Apr 17, 2017 at 11:37 AM, Anna Schumaker wrote: > > > On Mon, Apr 17, 2017 at 11:30 AM Olga Kornievskaia wrote: >> >> On Mon, Apr 17, 2017 at 9:36 AM, J. Bruce Fields >> wrote: >> > On Fri, Apr 14, 2017 at 05:22:13PM -0400, Olga Kornievskaia wrote: >> >> On Fri, Apr 14, 2017 at 4:09 PM, Mora, Jorge >> >> wrote: >> >> > On 4/13/17, 11:45 AM, "J. Bruce Fields" wrote: >> >> >> Are you timing just the copy_file_range() call, or do you include a >> >> >> following sync? >> >> > >> >> > I am timing right before calling copy_file_range() up to doing an >> >> > fsync() and close() of the destination file. >> >> > For the traditional copy is the same, I am timing right before the >> >> > first read on the source file up to the >> >> > fsync() and close() of the destination file. >> >> >> >> Why should do we need a sync after copy_file_range(). kernel >> >> copy_file_range() will send the commits for any unstable copies it >> >> received. >> > >> > Why does it do that? As far as I can tell it's not required by >> > documentation for copy_file_range() or COPY. COPY has a write verifier >> > and a stable_how argument in the reply. Skipping the commits would >> > allow better performance in case a copy requires multiple COPY calls. >> > >> > But, in any case, if copy_file_range() already committed then it >> > probably doesn't make a significant difference to the timing whether you >> > include a following sync and/or close. >> >> Hm. It does make sense. Anna wrote the original code which included >> the COMMIT after copy which I haven't thought about. >> >> Anna, any comments? > > > I think the commit just seemed like a good idea at the time. I'm okay with > changing it if it doesn't make sense. Given how the code is written now it looks like it's not possible to save up commits.... Here's what I can see happening: nfs42_proc_clone() as well as nfs42_proc_copy() will call nfs_sync_inode(dst) "to make sure server(s) have the latest data" prior to initiating the clone/copy. So even if we just queue up (not send) the commit after the executing nfs42_proc_copy, then next call into vfs_copy_file_range() will send out that queued up commit. Is it ok to relax the requirement that requirement, I'm not sure...