From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-f67.google.com ([209.85.214.67]:36617 "EHLO mail-it0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935190AbeFMOzo (ORCPT ); Wed, 13 Jun 2018 10:55:44 -0400 MIME-Version: 1.0 References: <2b437c6f-3e10-3d83-bdf3-82075d3eaa1a@i-love.sakura.ne.jp> <3cf4b0e3-31b6-8cdc-7c1e-15ba575a7879@i-love.sakura.ne.jp> <20180611091248.2i6nt27h5mxrodm2@quack2.suse.cz> <20180611160131.GQ1351649@devbig577.frc2.facebook.com> <20180611162920.mwapvuqotvhkntt3@quack2.suse.cz> <20180611172053.GR1351649@devbig577.frc2.facebook.com> <20180612155754.x5k2yndh5t6wlmpy@quack2.suse.cz> <20180613144606.nvbcyg2rdjpxhf7s@quack2.suse.cz> In-Reply-To: <20180613144606.nvbcyg2rdjpxhf7s@quack2.suse.cz> From: Linus Torvalds Date: Wed, 13 Jun 2018 07:55:32 -0700 Message-ID: Subject: Re: [PATCH] bdi: Fix another oops in wb_workfn() To: Jan Kara Cc: Tetsuo Handa , Tejun Heo , Dmitry Vyukov , Jens Axboe , syzbot+4a7438e774b21ddd8eca@syzkaller.appspotmail.com, syzkaller-bugs@googlegroups.com, linux-fsdevel , Linux Kernel Mailing List , Al Viro , Dave Chinner , linux-block Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, Jun 13, 2018 at 7:46 AM Jan Kara wrote: > > On Wed 13-06-18 19:43:47, Tetsuo Handa wrote: > > Can't we utilize RCU grace period (like shown below) ? > > Honestly, the variant 1 looks too ugly to me. However variant 2 looks > mostly OK. The versions that don't have that conditional locking look fine to me, yes. > Also I'd avoid the addition argument to wb_writeback() and split the function instead. The > patch resulting from your and mine ideas is attached. Thoughts? Is there a reason for this model: + if (cgwb_start_shutdown(wb)) + __wb_shutdown(wb); when there is just one call site of this? Why not just make the function void, and make it do that __wb_shutdown() itself in the true case? IOW, just make it be + cgwb_shutdown(wb); instead? That's what "wb_shutdown()" does - it does the "wb_start_shutdown()" test internally, and does __wb_shutdown() all inside itself, instead of expecting the caller to do it. I dunno. Linus Linus