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=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED 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 98D95C64E7B for ; Mon, 30 Nov 2020 15:02:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 494B7206D8 for ; Mon, 30 Nov 2020 15:02:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727892AbgK3PCC (ORCPT ); Mon, 30 Nov 2020 10:02:02 -0500 Received: from rin.romanrm.net ([51.158.148.128]:50060 "EHLO rin.romanrm.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727001AbgK3PCC (ORCPT ); Mon, 30 Nov 2020 10:02:02 -0500 Received: from natsu (unknown [IPv6:fd39::e99e:8f1b:cfc9:ccb8]) by rin.romanrm.net (Postfix) with SMTP id 9852F50A; Mon, 30 Nov 2020 15:01:16 +0000 (UTC) Date: Mon, 30 Nov 2020 20:01:16 +0500 From: Roman Mamedov To: Josef Bacik Cc: linux-btrfs@vger.kernel.org, kernel-team@fb.com Subject: Re: [PATCH] btrfs: do not allow -o compress-force to override per-inode settings Message-ID: <20201130200116.79a710fe@natsu> In-Reply-To: References: <20201130190805.48779810@natsu> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On Mon, 30 Nov 2020 09:50:13 -0500 Josef Bacik wrote: > The thing you're missing is that when we do chattr -c we're setting NOCOMPRESS > on the file. Wow, and does this need a previously set +c to work? Or just -c on an already -c file will change the Btrfs flag under the hood? Seems to be very weird in any case, as from the user perspective there's no way to view the current status of that flag, with the only way to change it being via a side-effect of another operation. > If chattr -c is supposed to just be the removal of +c, then btrfs is doing the > wrong thing by setting NOCOMPRESS. I would agree with that. > I guess the question is what do we want? Do we want to only allow the user to > indicate we want compression, or do we want to allow them to also indicate that > they don't want compression? If we don't want to enable them to disable > compression for a file, then this patch needs to be thrown away, but then we > also need to fix up all the places we set NOCOMPRESS when we clear these flags. The patch also seems to prioritize "no compress if compression ratio is bad" over compress-force, whereas the whole point of compress-force feels to be to compress no matter what, especially no matter what are the possibly imperfect compression ratio estimates. -- With respect, Roman