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=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 DD171C43603 for ; Thu, 12 Dec 2019 12:11:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BCD86206A5 for ; Thu, 12 Dec 2019 12:11:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729136AbfLLMLF (ORCPT ); Thu, 12 Dec 2019 07:11:05 -0500 Received: from mx2.suse.de ([195.135.220.15]:39316 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1729118AbfLLMLE (ORCPT ); Thu, 12 Dec 2019 07:11:04 -0500 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 D8E85BA38; Thu, 12 Dec 2019 12:11:02 +0000 (UTC) Received: by ds.suse.cz (Postfix, from userid 10065) id 64035DA82A; Thu, 12 Dec 2019 13:11:03 +0100 (CET) Date: Thu, 12 Dec 2019 13:11:03 +0100 From: David Sterba To: Sasha Levin Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Omar Sandoval , Johannes Thumshirn , David Sterba , linux-btrfs@vger.kernel.org Subject: Re: [PATCH AUTOSEL 5.4 299/350] btrfs: don't prematurely free work in end_workqueue_fn() Message-ID: <20191212121103.GR3929@suse.cz> Reply-To: dsterba@suse.cz Mail-Followup-To: dsterba@suse.cz, Sasha Levin , linux-kernel@vger.kernel.org, stable@vger.kernel.org, Omar Sandoval , Johannes Thumshirn , David Sterba , linux-btrfs@vger.kernel.org References: <20191210210735.9077-1-sashal@kernel.org> <20191210210735.9077-260-sashal@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191210210735.9077-260-sashal@kernel.org> User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On Tue, Dec 10, 2019 at 04:06:44PM -0500, Sasha Levin wrote: > From: Omar Sandoval > > [ Upstream commit 9be490f1e15c34193b1aae17da58e14dd9f55a95 ] > > Currently, end_workqueue_fn() frees the end_io_wq entry (which embeds > the work item) and then calls bio_endio(). This is another potential > instance of the bug in "btrfs: don't prematurely free work in > run_ordered_work()". > > In particular, the endio call may depend on other work items. For > example, btrfs_end_dio_bio() can call btrfs_subio_endio_read() -> > __btrfs_correct_data_nocsum() -> dio_read_error() -> > submit_dio_repair_bio(), which submits a bio that is also completed > through a end_workqueue_fn() work item. However, > __btrfs_correct_data_nocsum() waits for the newly submitted bio to > complete, thus it depends on another work item. > > This example currently usually works because we use different workqueue > helper functions for BTRFS_WQ_ENDIO_DATA and BTRFS_WQ_ENDIO_DIO_REPAIR. > However, it may deadlock with stacked filesystems and is fragile > overall. The proper fix is to free the work item at the very end of the > work function, so let's do that. > > Reviewed-by: Johannes Thumshirn > Signed-off-by: Omar Sandoval > Reviewed-by: David Sterba > Signed-off-by: David Sterba > Signed-off-by: Sasha Levin The were more patches in the series, all contain "don't prematurely free work in" and were part of a rework of async work processing. They're fixing a very uncommon usecase, so if there's desire to backport them the whole series needs to go in. In the autosel list, there are only 2 and without the important fix. c495dcd6fbe1 btrfs: don't prematurely free work in run_ordered_work() 9be490f1e15c btrfs: don't prematurely free work in end_workqueue_fn() e732fe95e4ca btrfs: don't prematurely free work in reada_start_machine_worker() 57d4f0b86327 btrfs: don't prematurely free work in scrub_missing_raid56_worker() a0cac0ec961f btrfs: get rid of unique workqueue helper functions - this is only a cleanup that removes code obsoleted by the fixes above, probably out of scope of stable I have intentionally not tagged the patches for stable, the usecase is is specific to one user (FB), the known reproducer is only their workload and the fixes are in their kernel already. So if there's desire to add the patches to stable trees, then it has to be the whole series, but I don't see a strong reason for it.