From patchwork Wed Dec 16 23:31:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vivek Goyal X-Patchwork-Id: 11978787 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4F6C2C2BBCF for ; Wed, 16 Dec 2020 23:34:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EE82523731 for ; Wed, 16 Dec 2020 23:34:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730918AbgLPXeB (ORCPT ); Wed, 16 Dec 2020 18:34:01 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:22127 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730831AbgLPXdw (ORCPT ); Wed, 16 Dec 2020 18:33:52 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1608161546; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=uobPKuVZDbEZ19fX23VSvt5Q35SONPP7xECvb/u/H1s=; b=clBWycvkp+3Ay7Ialk1Rc4Z0vRAgrjQF+lF0fWfjRy7tC1U5pHASYlF6ffQdQMVmKV6VcQ mdIB6IoarFm8my6K1ISMilvyWsKm7hwQHCPvM2idTK+gMzMY7iwVWK5CeWJk6NHYe6KdBg E9eIFxeb854wIQD2w8wi6/UuwI3XIwg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-301-UVr1GdXPOXW3R1WbGPuZ0w-1; Wed, 16 Dec 2020 18:32:22 -0500 X-MC-Unique: UVr1GdXPOXW3R1WbGPuZ0w-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 208FA800D53; Wed, 16 Dec 2020 23:32:20 +0000 (UTC) Received: from horse.redhat.com (ovpn-112-114.rdu2.redhat.com [10.10.112.114]) by smtp.corp.redhat.com (Postfix) with ESMTP id F10E45D9C0; Wed, 16 Dec 2020 23:32:19 +0000 (UTC) Received: by horse.redhat.com (Postfix, from userid 10451) id 77DF5223D98; Wed, 16 Dec 2020 18:32:19 -0500 (EST) From: Vivek Goyal To: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-unionfs@vger.kernel.org Cc: jlayton@kernel.org, vgoyal@redhat.com, amir73il@gmail.com, sargun@sargun.me, miklos@szeredi.hu, willy@infradead.org, jack@suse.cz, neilb@suse.com, viro@zeniv.linux.org.uk Subject: [PATCH 1/3] vfs: add new f_op->syncfs vector Date: Wed, 16 Dec 2020 18:31:47 -0500 Message-Id: <20201216233149.39025-2-vgoyal@redhat.com> In-Reply-To: <20201216233149.39025-1-vgoyal@redhat.com> References: <20201216233149.39025-1-vgoyal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org Current implementation of __sync_filesystem() ignores the return code from ->sync_fs(). I am not sure why that's the case. There must have been some historical reason for this. Ignoring ->sync_fs() return code is problematic for overlayfs where it can return error if sync_filesystem() on upper super block failed. That error will simply be lost and sycnfs(overlay_fd), will get success (despite the fact it failed). If we modify existing implementation, there is a concern that it will lead to user space visible behavior changes and break things. So instead implement a new file_operations->syncfs() call which will be called in syncfs() syscall path. Return code from this new call will be captured. And all the writeback error detection logic can go in there as well. Only filesystems which implement this call get affected by this change. Others continue to fallback to existing mechanism. To be clear, I mean something like this (draft, untested) patch. You'd also need to add a new ->syncfs op for overlayfs, and that could just do a check_and_advance against the upper layer sb's errseq_t after calling sync_filesystem. Vivek, fixed couple of minor compile errors in original patch. Signed-off-by: Jeff Layton --- fs/sync.c | 29 ++++++++++++++++++++--------- include/linux/fs.h | 1 + 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/fs/sync.c b/fs/sync.c index 1373a610dc78..06caa9758d93 100644 --- a/fs/sync.c +++ b/fs/sync.c @@ -155,27 +155,38 @@ void emergency_sync(void) } } +static int generic_syncfs(struct file *file) +{ + int ret, ret2; + struct super_block *sb = file->f_path.dentry->d_sb; + + down_read(&sb->s_umount); + ret = sync_filesystem(sb); + up_read(&sb->s_umount); + + ret2 = errseq_check_and_advance(&sb->s_wb_err, &file->f_sb_err); + + return ret ? ret : ret2; +} + /* * sync a single super */ SYSCALL_DEFINE1(syncfs, int, fd) { struct fd f = fdget(fd); - struct super_block *sb; - int ret, ret2; + int ret; if (!f.file) return -EBADF; - sb = f.file->f_path.dentry->d_sb; - - down_read(&sb->s_umount); - ret = sync_filesystem(sb); - up_read(&sb->s_umount); - ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err); + if (f.file->f_op->syncfs) + ret = f.file->f_op->syncfs(f.file); + else + ret = generic_syncfs(f.file); fdput(f); - return ret ? ret : ret2; + return ret; } /** diff --git a/include/linux/fs.h b/include/linux/fs.h index 8667d0cdc71e..6710469b7e33 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1859,6 +1859,7 @@ struct file_operations { struct file *file_out, loff_t pos_out, loff_t len, unsigned int remap_flags); int (*fadvise)(struct file *, loff_t, loff_t, int); + int (*syncfs)(struct file *); } __randomize_layout; struct inode_operations { From patchwork Wed Dec 16 23:31:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vivek Goyal X-Patchwork-Id: 11978783 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 81549C2BBCA for ; Wed, 16 Dec 2020 23:34:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 504132388E for ; Wed, 16 Dec 2020 23:34:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730878AbgLPXd4 (ORCPT ); Wed, 16 Dec 2020 18:33:56 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:41306 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730832AbgLPXdw (ORCPT ); Wed, 16 Dec 2020 18:33:52 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1608161546; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/j8AnznhuFvsuzt8LMRV9SccvBVsiOm87kWl69oQMuI=; b=i8Au55S5LJNPUF3gS+SQ7km8d25Y9d3k7GLUG5DOirbXsrpdERn8dvMJf9NI27Q6V/8MZo DIuVFFcbOlmqI1WsNTna8ecSzNR+WrCwofabsa3itmHf/tG7khbaANzJbhKUe1L1EactAJ EjOQIIHnMLDz2NKSHzXC+33rxhF7iEA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-17-BhMMAtBVMx6ndoGkaANqvA-1; Wed, 16 Dec 2020 18:32:22 -0500 X-MC-Unique: BhMMAtBVMx6ndoGkaANqvA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2394EBBEE0; Wed, 16 Dec 2020 23:32:20 +0000 (UTC) Received: from horse.redhat.com (ovpn-112-114.rdu2.redhat.com [10.10.112.114]) by smtp.corp.redhat.com (Postfix) with ESMTP id F11875D9CD; Wed, 16 Dec 2020 23:32:19 +0000 (UTC) Received: by horse.redhat.com (Postfix, from userid 10451) id 7B769223D99; Wed, 16 Dec 2020 18:32:19 -0500 (EST) From: Vivek Goyal To: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-unionfs@vger.kernel.org Cc: jlayton@kernel.org, vgoyal@redhat.com, amir73il@gmail.com, sargun@sargun.me, miklos@szeredi.hu, willy@infradead.org, jack@suse.cz, neilb@suse.com, viro@zeniv.linux.org.uk Subject: [PATCH 2/3] overlayfs: Implement f_op->syncfs() call Date: Wed, 16 Dec 2020 18:31:48 -0500 Message-Id: <20201216233149.39025-3-vgoyal@redhat.com> In-Reply-To: <20201216233149.39025-1-vgoyal@redhat.com> References: <20201216233149.39025-1-vgoyal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org Provide an implementation for ->syncfs(). Now if there is an error returned by sync_filesystem(upper_sb), it will be visible to user space. Currently in ovl_sync_fs() path, this error is ignored by VFS. A later patch also adds logic to detect writeback error. Signed-off-by: Vivek Goyal --- fs/overlayfs/file.c | 1 + fs/overlayfs/overlayfs.h | 3 +++ fs/overlayfs/readdir.c | 1 + fs/overlayfs/super.c | 30 ++++++++++++++++++++++++++++++ 4 files changed, 35 insertions(+) diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index efccb7c1f9bc..affc1ba63202 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -806,6 +806,7 @@ const struct file_operations ovl_file_operations = { .copy_file_range = ovl_copy_file_range, .remap_file_range = ovl_remap_file_range, + .syncfs = ovl_syncfs, }; int __init ovl_aio_request_cache_init(void) diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index f8880aa2ba0e..1efb13800755 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -520,3 +520,6 @@ int ovl_set_origin(struct dentry *dentry, struct dentry *lower, /* export.c */ extern const struct export_operations ovl_export_operations; + +/* super.c */ +int ovl_syncfs(struct file *file); diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index 01620ebae1bd..e89b450c8f8f 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -975,6 +975,7 @@ const struct file_operations ovl_dir_operations = { #ifdef CONFIG_COMPAT .compat_ioctl = ovl_compat_ioctl, #endif + .syncfs = ovl_syncfs, }; int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 290983bcfbb3..b4d92e6fa5ce 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -286,6 +286,36 @@ static int ovl_sync_fs(struct super_block *sb, int wait) return ret; } +int ovl_syncfs(struct file *file) +{ + struct super_block *sb = file->f_path.dentry->d_sb; + struct ovl_fs *ofs = sb->s_fs_info; + struct super_block *upper_sb; + int ret; + + ret = 0; + down_read(&sb->s_umount); + if (sb_rdonly(sb)) + goto out; + + if (!ovl_upper_mnt(ofs)) + goto out; + + if (!ovl_should_sync(ofs)) + goto out; + + upper_sb = ovl_upper_mnt(ofs)->mnt_sb; + + down_read(&upper_sb->s_umount); + ret = sync_filesystem(upper_sb); + up_read(&upper_sb->s_umount); + + +out: + up_read(&sb->s_umount); + return ret; +} + /** * ovl_statfs * @sb: The overlayfs super block From patchwork Wed Dec 16 23:31:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vivek Goyal X-Patchwork-Id: 11978785 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ACE88C2BB9A for ; Wed, 16 Dec 2020 23:34:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 75BA523730 for ; Wed, 16 Dec 2020 23:34:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730868AbgLPXd4 (ORCPT ); Wed, 16 Dec 2020 18:33:56 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:43654 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730830AbgLPXdw (ORCPT ); Wed, 16 Dec 2020 18:33:52 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1608161546; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Iyvu3tczAymPkwWycdPC/BAXXExR62UHIGSUwLXdNqs=; b=GATxJoVUWJWG8NY/0HNJrGYtwdvgP1PJEF/mYjKxrTBr15zQ1SbeexTq50SXG2fkoTRB7x KXN79Z3HTksxGMBrJxeJQZvAWMzz//aJwYtIaH5HV8TX9pzHVyMYVqpdc0UIH0YoTUkW3g mtg/rBen9MrNU5+g8PJuydVhInNuCEo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-501-5DZe-w98PwK_bJ9o0z2SMw-1; Wed, 16 Dec 2020 18:32:22 -0500 X-MC-Unique: 5DZe-w98PwK_bJ9o0z2SMw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2F3C21842142; Wed, 16 Dec 2020 23:32:20 +0000 (UTC) Received: from horse.redhat.com (ovpn-112-114.rdu2.redhat.com [10.10.112.114]) by smtp.corp.redhat.com (Postfix) with ESMTP id 09B795D9D2; Wed, 16 Dec 2020 23:32:20 +0000 (UTC) Received: by horse.redhat.com (Postfix, from userid 10451) id 8098A225FCD; Wed, 16 Dec 2020 18:32:19 -0500 (EST) From: Vivek Goyal To: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-unionfs@vger.kernel.org Cc: jlayton@kernel.org, vgoyal@redhat.com, amir73il@gmail.com, sargun@sargun.me, miklos@szeredi.hu, willy@infradead.org, jack@suse.cz, neilb@suse.com, viro@zeniv.linux.org.uk Subject: [PATCH 3/3] overlayfs: Check writeback errors w.r.t upper in ->syncfs() Date: Wed, 16 Dec 2020 18:31:49 -0500 Message-Id: <20201216233149.39025-4-vgoyal@redhat.com> In-Reply-To: <20201216233149.39025-1-vgoyal@redhat.com> References: <20201216233149.39025-1-vgoyal@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org Check for writeback error on overlay super block w.r.t "struct file" passed in ->syncfs(). As of now real error happens on upper sb. So this patch first propagates error from upper sb to overlay sb and then checks error w.r.t struct file passed in. Jeff, I know you prefer that I should rather file upper file and check error directly on on upper sb w.r.t this real upper file. While I was implementing that I thought what if file is on lower (and has not been copied up yet). In that case shall we not check writeback errors and return back to user space? That does not sound right though because, we are not checking for writeback errors on this file. Rather we are checking for any error on superblock. Upper might have an error and we should report it to user even if file in question is a lower file. And that's why I fell back to this approach. But I am open to change it if there are issues in this method. Signed-off-by: Vivek Goyal --- fs/overlayfs/ovl_entry.h | 2 ++ fs/overlayfs/super.c | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h index 1b5a2094df8e..a08fd719ee7b 100644 --- a/fs/overlayfs/ovl_entry.h +++ b/fs/overlayfs/ovl_entry.h @@ -79,6 +79,8 @@ struct ovl_fs { atomic_long_t last_ino; /* Whiteout dentry cache */ struct dentry *whiteout; + /* Protects multiple sb->s_wb_err update from upper_sb . */ + spinlock_t errseq_lock; }; static inline struct vfsmount *ovl_upper_mnt(struct ovl_fs *ofs) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index b4d92e6fa5ce..e7bc4492205e 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -291,7 +291,7 @@ int ovl_syncfs(struct file *file) struct super_block *sb = file->f_path.dentry->d_sb; struct ovl_fs *ofs = sb->s_fs_info; struct super_block *upper_sb; - int ret; + int ret, ret2; ret = 0; down_read(&sb->s_umount); @@ -310,10 +310,18 @@ int ovl_syncfs(struct file *file) ret = sync_filesystem(upper_sb); up_read(&upper_sb->s_umount); + /* Update overlay sb->s_wb_err */ + if (errseq_check(&upper_sb->s_wb_err, sb->s_wb_err)) { + /* Upper sb has errors since last time */ + spin_lock(&ofs->errseq_lock); + errseq_check_and_advance(&upper_sb->s_wb_err, &sb->s_wb_err); + spin_unlock(&ofs->errseq_lock); + } + ret2 = errseq_check_and_advance(&sb->s_wb_err, &file->f_sb_err); out: up_read(&sb->s_umount); - return ret; + return ret ? ret : ret2; } /** @@ -1903,6 +1911,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) if (!cred) goto out_err; + spin_lock_init(&ofs->errseq_lock); /* Is there a reason anyone would want not to share whiteouts? */ ofs->share_whiteout = true; @@ -1975,7 +1984,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) sb->s_stack_depth = ovl_upper_mnt(ofs)->mnt_sb->s_stack_depth; sb->s_time_gran = ovl_upper_mnt(ofs)->mnt_sb->s_time_gran; - + sb->s_wb_err = errseq_sample(&ovl_upper_mnt(ofs)->mnt_sb->s_wb_err); } oe = ovl_get_lowerstack(sb, splitlower, numlower, ofs, layers); err = PTR_ERR(oe);