From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753219AbcIEEcl (ORCPT ); Mon, 5 Sep 2016 00:32:41 -0400 Received: from esa1.hgst.iphmx.com ([68.232.141.245]:14197 "EHLO esa1.hgst.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750720AbcIEEcj (ORCPT ); Mon, 5 Sep 2016 00:32:39 -0400 X-IronPort-AV: E=Sophos;i="5.30,284,1470672000"; d="scan'208";a="16675415" From: Naohiro Aota To: "jbacik@fb.com" , "linux-btrfs@vger.kernel.org" CC: "linux-kernel@vger.kernel.org" , "dsterba@suse.com" , "clm@fb.com" Subject: Re: [PATCH] btrfs: let btrfs_delete_unused_bgs() to clean relocated bgs Thread-Topic: [PATCH] btrfs: let btrfs_delete_unused_bgs() to clean relocated bgs Thread-Index: AQHSBO5MgN4eXP6C0k254Tw4zq2+DKBmM+OAgAQfTIA= Date: Mon, 5 Sep 2016 04:32:37 +0000 Message-ID: <1473049955.14093.37.camel@hgst.com> References: <1472802392-10851-1-git-send-email-naohiro.aota@hgst.com> In-Reply-To: Accept-Language: ja-JP, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Naohiro.Aota@hgst.com; x-originating-ip: [199.255.47.6] x-ms-office365-filtering-correlation-id: a497c0cd-7bf0-470c-faa0-08d3d545aa24 x-microsoft-exchange-diagnostics: 1;CY4PR04MB0742;20:NLSpfkO3CZoPz7dDUSxoLSUTv1L5uyU+s6wjKDldi2lWGD5gqpfLZAAO+Z4Yq8CxigI2NMBRbaG7MQKSc9sH0jvq5OsuKxr0UT4MGOVyX+vpTApjtvr4aNaAoWXT0MT2NRafsQ59BsqmEtFxvT13fkLQess0NSwNCwGuEjgAnNBLFntQAxvyiATuxsAmatB3Wbi992tPvKUyuevWECT2PeRibuUQiv93sQTl+W2TaR5GUscC93868oQqWSYOe+FK x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CY4PR04MB0742; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6055026);SRVR:CY4PR04MB0742;BCL:0;PCL:0;RULEID:;SRVR:CY4PR04MB0742; x-forefront-prvs: 005671E15D x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(7916002)(377454003)(189002)(24454002)(199003)(97736004)(10400500002)(2900100001)(92566002)(50986999)(76176999)(11100500001)(86362001)(103116003)(2950100001)(101416001)(551934003)(105586002)(106356001)(2501003)(87936001)(106116001)(99286002)(54356999)(4326007)(8936002)(8676002)(33646002)(81166006)(81156014)(36756003)(2906002)(3280700002)(3660700001)(68736007)(305945005)(3846002)(586003)(102836003)(6116002)(7846002)(122556002)(7736002)(77096005)(66066001)(5002640100001)(5001770100001)(189998001)(5660300001);DIR:OUT;SFP:1102;SCL:1;SRVR:CY4PR04MB0742;H:CY4PR04MB0745.namprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="utf-8" Content-ID: MIME-Version: 1.0 X-OriginatorOrg: hgst.com X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Sep 2016 04:32:37.0629 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86 X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR04MB0742 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u854Wk6w028571 2016-09-02 (金) の 09:35 -0400 に Josef Bacik さんは書きました: > On 09/02/2016 03:46 AM, Naohiro Aota wrote: > > > > Currently, btrfs_relocate_chunk() is removing relocated BG by > > itself. But > > the work can be done by btrfs_delete_unused_bgs() (and it's better > > since it > > trim the BG). Let's dedupe the code. > > > > While btrfs_delete_unused_bgs() is already hitting the relocated > > BG, it > > skip the BG since the BG has "ro" flag set (to keep balancing BG > > intact). > > On the other hand, btrfs cannot drop "ro" flag here to prevent > > additional > > writes. So this patch make use of "removed" flag. > > btrfs_delete_unused_bgs() now detect the flag to distinguish > > whether a > > read-only BG is relocating or not. > > > > This seems racey to me.  We remove the last part of the block group, > it ends up  > on the unused_bgs_list, we process this list, see that removed isn't > set and we  > skip it, then later we set removed, but it's too late.  I think the > right way is  > to actually do a transaction, set ->removed, manually add it to the  > unused_bgs_list if it's not already, then end the transaction.  This > way we are  > guaranteed to have the bg on the list when it is ready to be > removed.  This is  > my analysis after looking at it for 10 seconds after being awake for > like 30  > minutes so if I'm missing something let me know.  Thanks, I don't think a race will happen. Since we are holding delete_unused_bgs_mutex here, btrfs_delte_unused_bgs() checks ->removed flag after we unlock the mutex i.e. we setup the flag properly. For a case btrfs_delete_usused_bgs() checks the BG before we hold delte_unused_bgs_mutex, then that BG is removed by it (if it's empty) and btrfs_relocate_chunk() should never see it. Regards, Naohiro