From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: [PATCH block/for-linus] writeback: flush inode cgroup wb switches instead of pinning super_block Date: Mon, 29 Feb 2016 20:54:28 +0000 Message-ID: <20160229205428.GB17997@ZenIV.linux.org.uk> References: <20160215210047.GN3965@htj.duckdns.org> <20160216182457.GO3741@mtj.duckdns.org> <20160217205721.GE14140@quack.suse.cz> <20160217210744.GA6479@mtj.duckdns.org> <20160217223009.GN14140@quack.suse.cz> <20160217230231.GC6479@mtj.duckdns.org> <20160229204724.GV3965@htj.duckdns.org> Mime-Version: 1.0 Return-path: Content-Disposition: inline In-Reply-To: <20160229204724.GV3965-piEFEHQLUPpN0TnZuCh8vA@public.gmane.org> Sender: cgroups-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Tejun Heo Cc: Tahsin Erdogan , Jan Kara , Jens Axboe , cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Theodore Ts'o , Nauman Rafique , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jan Kara On Mon, Feb 29, 2016 at 03:47:24PM -0500, Tejun Heo wrote: > If cgroup writeback is in use, inodes can be scheduled for > asynchronous wb switching. Before 5ff8eaac1636 ("writeback: keep > superblock pinned during cgroup writeback association switches"), this > could race with umount leading to super_block being destroyed while > inodes are pinned for wb switching. 5ff8eaac1636 fixed it by bumping > s_active while wb switches are in flight; however, this allowed > in-flight wb switches to make umounts asynchronous when the userland > expected synchronosity - e.g. fsck immediately following umount may > fail because the device is still busy. > > This patch removes the problematic super_block pinning and instead > makes generic_shutdown_super() flush in-flight wb switches. wb > switches are now executed on a dedicated isw_wq so that they can be > flushed and isw_nr_in_flight keeps track of the number of in-flight wb > switches so that flushing can be avoided in most cases. Wait a bloody minute. What's to prevent shrink_dcache_for_umount() from dirtying more inodes, triggering more of the same? > - if (!atomic_inc_not_zero(&inode->i_sb->s_active)) > - goto out_unlock; This would've failed for inodes on superblock in the middle of shutdown; what's to do the same for the new variant? From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750846AbcB2Uyj (ORCPT ); Mon, 29 Feb 2016 15:54:39 -0500 Received: from zeniv.linux.org.uk ([195.92.253.2]:55347 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750709AbcB2Uyi (ORCPT ); Mon, 29 Feb 2016 15:54:38 -0500 Date: Mon, 29 Feb 2016 20:54:28 +0000 From: Al Viro To: Tejun Heo Cc: Tahsin Erdogan , Jan Kara , Jens Axboe , cgroups@vger.kernel.org, "Theodore Ts'o" , Nauman Rafique , linux-kernel@vger.kernel.org, Jan Kara Subject: Re: [PATCH block/for-linus] writeback: flush inode cgroup wb switches instead of pinning super_block Message-ID: <20160229205428.GB17997@ZenIV.linux.org.uk> References: <20160215210047.GN3965@htj.duckdns.org> <20160216182457.GO3741@mtj.duckdns.org> <20160217205721.GE14140@quack.suse.cz> <20160217210744.GA6479@mtj.duckdns.org> <20160217223009.GN14140@quack.suse.cz> <20160217230231.GC6479@mtj.duckdns.org> <20160229204724.GV3965@htj.duckdns.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160229204724.GV3965@htj.duckdns.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Feb 29, 2016 at 03:47:24PM -0500, Tejun Heo wrote: > If cgroup writeback is in use, inodes can be scheduled for > asynchronous wb switching. Before 5ff8eaac1636 ("writeback: keep > superblock pinned during cgroup writeback association switches"), this > could race with umount leading to super_block being destroyed while > inodes are pinned for wb switching. 5ff8eaac1636 fixed it by bumping > s_active while wb switches are in flight; however, this allowed > in-flight wb switches to make umounts asynchronous when the userland > expected synchronosity - e.g. fsck immediately following umount may > fail because the device is still busy. > > This patch removes the problematic super_block pinning and instead > makes generic_shutdown_super() flush in-flight wb switches. wb > switches are now executed on a dedicated isw_wq so that they can be > flushed and isw_nr_in_flight keeps track of the number of in-flight wb > switches so that flushing can be avoided in most cases. Wait a bloody minute. What's to prevent shrink_dcache_for_umount() from dirtying more inodes, triggering more of the same? > - if (!atomic_inc_not_zero(&inode->i_sb->s_active)) > - goto out_unlock; This would've failed for inodes on superblock in the middle of shutdown; what's to do the same for the new variant?