Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965620AbcKXNvZ (ORCPT ); Thu, 24 Nov 2016 08:51:25 -0500 Received: from mail-oi0-f65.google.com ([209.85.218.65]:33592 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964995AbcKXNvX (ORCPT ); Thu, 24 Nov 2016 08:51:23 -0500 MIME-Version: 1.0 X-Originating-IP: [217.173.44.24] In-Reply-To: References: <1479984944-1017-1-git-send-email-mszeredi@redhat.com> <1479984944-1017-6-git-send-email-mszeredi@redhat.com> From: Miklos Szeredi Date: Thu, 24 Nov 2016 14:51:21 +0100 Message-ID: Subject: Re: [PATCH 4/7] ovl: add infrastructure for intercepting file ops To: Amir Goldstein Cc: Miklos Szeredi , "linux-unionfs@vger.kernel.org" , linux-fsdevel , linux-kernel Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2229 Lines: 47 On Thu, Nov 24, 2016 at 2:12 PM, Amir Goldstein wrote: > On Thu, Nov 24, 2016 at 2:03 PM, Miklos Szeredi wrote: >> On Thu, Nov 24, 2016 at 12:52 PM, Amir Goldstein wrote: >>> On Thu, Nov 24, 2016 at 12:55 PM, Miklos Szeredi wrote: >> >>>> + /* >>>> + * These should be intercepted, but they are very unlikely to be >>>> + * a problem in practice. Leave them alone for now. >>> >>> It could also be handled in vfs helpers. >>> Since these ops all start with establishing that src and dest are on >>> the same sb, >>> then the cost of copy up of src is the cost of clone_file_range from >>> lower to upper, >>> so it is probably worth to copy up src and leave those fops alone. >>> >>>> + */ >>>> + ofop->fops.copy_file_range = orig->copy_file_range; >>>> + ofop->fops.clone_file_range = orig->clone_file_range; >>>> + ofop->fops.dedupe_file_range = orig->dedupe_file_range; >> >> Not sure I understand. Why should we copy up src? Copy up is the >> problem not the solution. >> > > Maybe the idea is ill conceived, but the reasoning is: > To avoid the corner case of cloning from a stale lower src, > call d_real() in vfs helpers to always copy up src before cloning from it > and pass the correct file onwards. Which correct file? src is still the wrong one after calling d_real. We need to clone-open src, just like we do in ovl_read_iter to get the correct file. But then what's the use of copying it up beforehand? We could move the whole logic into the vfs, but I don't really see the point. I left these ops alone because there is some confusion in there about getting the f_op from the source or the destination file. And while it doesn't matter normally (all regular files have the same f_op, regardless of open flags) it does matter for overlayfs intercept, because overriding fops in the the dest file would mean additional complexity and resources). That could easily be fixed in the vfs: calling src->f_ops->foo works equally well, but I simply didn't want to bother with this. We can return to it later. Thanks, Miklos