From mboxrd@z Thu Jan 1 00:00:00 1970 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=-2.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no 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 70B29CA9EA0 for ; Tue, 22 Oct 2019 08:25:26 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 37D0320684 for ; Tue, 22 Oct 2019 08:25:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 37D0320684 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id CBEA96B0003; Tue, 22 Oct 2019 04:25:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C94CA6B0006; Tue, 22 Oct 2019 04:25:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BAB7B6B0007; Tue, 22 Oct 2019 04:25:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0080.hostedemail.com [216.40.44.80]) by kanga.kvack.org (Postfix) with ESMTP id 9990F6B0003 for ; Tue, 22 Oct 2019 04:25:25 -0400 (EDT) Received: from smtpin18.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with SMTP id 36218499606 for ; Tue, 22 Oct 2019 08:25:25 +0000 (UTC) X-FDA: 76070736210.18.ghost62_697b1fb3a2441 X-HE-Tag: ghost62_697b1fb3a2441 X-Filterd-Recvd-Size: 4014 Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by imf17.hostedemail.com (Postfix) with ESMTP for ; Tue, 22 Oct 2019 08:25:24 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 6F466B314; Tue, 22 Oct 2019 08:25:23 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 9B09F1E4812; Tue, 22 Oct 2019 10:15:51 +0200 (CEST) Date: Tue, 22 Oct 2019 10:15:51 +0200 From: Jan Kara To: Roman Gushchin Cc: Jan Kara , "linux-mm@kvack.org" , "linux-fsdevel@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Kernel Team , "tj@kernel.org" , Dennis Zhou Subject: Re: [PATCH v2] cgroup, blkcg: prevent dirty inodes to pin dying memory cgroups Message-ID: <20191022081551.GD2436@quack2.suse.cz> References: <20191010234036.2860655-1-guro@fb.com> <20191015090933.GA21104@quack2.suse.cz> <20191015214041.GA24736@tower.DHCP.thefacebook.com> <20191016091840.GC30337@quack2.suse.cz> <20191021234858.GA16251@castle> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191021234858.GA16251@castle> User-Agent: Mutt/1.10.1 (2018-07-13) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000138, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon 21-10-19 23:49:04, Roman Gushchin wrote: > On Wed, Oct 16, 2019 at 11:18:40AM +0200, Jan Kara wrote: > > On Tue 15-10-19 21:40:45, Roman Gushchin wrote: > > > On Tue, Oct 15, 2019 at 11:09:33AM +0200, Jan Kara wrote: > > > > On Thu 10-10-19 16:40:36, Roman Gushchin wrote: > > > > > > > > > @@ -426,7 +431,7 @@ static void inode_switch_wbs_work_fn(struct work_struct *work) > > > > > if (!list_empty(&inode->i_io_list)) { > > > > > struct inode *pos; > > > > > > > > > > - inode_io_list_del_locked(inode, old_wb); > > > > > + inode_io_list_del_locked(inode, old_wb, false); > > > > > inode->i_wb = new_wb; > > > > > list_for_each_entry(pos, &new_wb->b_dirty, i_io_list) > > > > > if (time_after_eq(inode->dirtied_when, > > > > > > > > This bit looks wrong. Not the change you made as such but the fact that you > > > > can now move inode from b_attached list of old wb to the dirty list of new > > > > wb. > > > > > > Hm, can you, please, elaborate a bit more why it's wrong? > > > The reference to the old_wb will be dropped by the switching code. > > > > My point is that the code in full looks like: > > > > if (!list_empty(&inode->i_io_list)) { > > struct inode *pos; > > > > inode_io_list_del_locked(inode, old_wb); > > inode->i_wb = new_wb; > > list_for_each_entry(pos, &new_wb->b_dirty, i_io_list) > > if (time_after_eq(inode->dirtied_when, > > pos->dirtied_when)) > > break; > > inode_io_list_move_locked(inode, new_wb, pos->i_io_list.prev); > > } else { > > > > So inode is always moved from some io list in old_wb to b_dirty list of > > new_wb. This is fine when it could be only on b_dirty, b_io, b_more_io lists > > of old_wb. But once you add b_attached list to the game, it is not correct > > anymore. You should not add clean inode to b_dirty list of new_wb. > > I see... > > Hm, will checking of i_state for not containing I_DIRTY_ALL bits be enough here? > Alternatively, I can introduce a new bit which will explicitly point at the > inode being on the b_attached list, but I'd prefer not to do it. Yeah, keying of i_state should work. And while we are at it, we could also correctly handle I_DIRTY_TIME case and move inode only to b_dirty_time list. That seems to be (mostly harmless) preexisting issue. Honza -- Jan Kara SUSE Labs, CR