From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS53758 23.128.96.0/24 X-Spam-Status: No, score=-3.7 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id B1D041F8C6 for ; Wed, 15 Sep 2021 04:55:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230214AbhIOE5A (ORCPT ); Wed, 15 Sep 2021 00:57:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41156 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229450AbhIOE47 (ORCPT ); Wed, 15 Sep 2021 00:56:59 -0400 Received: from mail-lf1-x132.google.com (mail-lf1-x132.google.com [IPv6:2a00:1450:4864:20::132]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 31CCEC061574 for ; Tue, 14 Sep 2021 21:55:41 -0700 (PDT) Received: by mail-lf1-x132.google.com with SMTP id p29so3384018lfa.11 for ; Tue, 14 Sep 2021 21:55:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fhqpyELSSL08oWdK5fTm0m3tGqMKMYG5Jn4lDToXwQY=; b=lMEqWYrGCpOOMvvP2yYLAUatajCmAsO0AcllwtBWjsZUlKk2UZ89VuMd5T/nqHydOi GoUz3e7aFPDgzzS21tQZgOBSy4nbzyQiMDk66MMG95xdT59UDDCcjjPxs0a2L0+zOKR1 FGCJd4g0kxMaZlvhg3+SZ9uzFV9fDAF5ZXTEOFxSgLqNS+uBT6LdlFvl/Zcnl8dnkkh4 SvLBBUfj966/CFViu+gehss4bFhnpzC+UyGUZc24BmqeBbZXJaWfra1g/r4VNNc/ULxZ IpkTAlygWmDcYfg8YajglDDM2WnXXgU0GRp+iuegpuJGX7TgjCcpW1N2xnT3J7GeNku2 AZfA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fhqpyELSSL08oWdK5fTm0m3tGqMKMYG5Jn4lDToXwQY=; b=AodNX+2VXHrw1q6tU1ym0aP2hwWKs6Ny1fvklQIesUVYxyK8KeEc+QpAsZwdF0U0/O K51qq+Xff7/VLiR7BNJD9+IMhDAtztEvm7h2/5Ru04TrNFgmHB+wNBQnoeh0BFz51j8e nnEzANG0M22S81mOSsIJ3cH2gf6bxYONUj2AIxH8A5g2vcsX5fbT9KZLZDYsszG87eFS Z1rOoGhn6PHVf4OrHX28D/OKYkHMtzWez+OMIPmUd9JjaapVKOi7dheF8ybcUU8DQJRB AeSUs8os5mR3Pr7pHLRKvr1Y2ucnn29w5dJjxCC/0rqdlbnZdg5DkJc/qAx0RQH7Wo+4 qkyw== X-Gm-Message-State: AOAM531ahwMH2MnY9WMzwbS4HD6OYaSFdf7idEVoLt7Z8n3EWy6Z6kx5 HoGThRKHvMdQguWNdsU2vNq04gV/KkX+iSKXNoc= X-Google-Smtp-Source: ABdhPJx3UWlyfhaLJlBNIZAaGn0JU7H7zNszOfywDf36OGL5kRQBn+jQIxk9MLU/W221GnO4DlcddIZ+BYFmpLSswXc= X-Received: by 2002:a19:700b:: with SMTP id h11mr15557200lfc.180.1631681739519; Tue, 14 Sep 2021 21:55:39 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Neeraj Singh Date: Tue, 14 Sep 2021 21:55:30 -0700 Message-ID: Subject: Re: [PATCH v3 2/6] core.fsyncobjectfiles: batched disk flushes To: Junio C Hamano Cc: Neeraj Singh via GitGitGadget , Git List , Johannes Schindelin , Jeff King , Jeff Hostetler , Christoph Hellwig , =?UTF-8?B?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= , "Randall S. Becker" , "Neeraj K. Singh" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Tue, Sep 14, 2021 at 12:34 PM Junio C Hamano wrote: > > "Neeraj Singh via GitGitGadget" writes: > > > diff --git a/config.c b/config.c > > index cb4a8058bff..9fe3602e1c4 100644 > > --- a/config.c > > +++ b/config.c > > @@ -1509,7 +1509,13 @@ static int git_default_core_config(const char *var, const char *value, void *cb) > > } > > > > if (!strcmp(var, "core.fsyncobjectfiles")) { > > - fsync_object_files = git_config_bool(var, value); > > + if (!value) > > + return config_error_nonbool(var); > > + if (!strcasecmp(value, "batch")) > > + fsync_object_files = FSYNC_OBJECT_FILES_BATCH; > > + else > > + fsync_object_files = git_config_bool(var, value) > > + ? FSYNC_OBJECT_FILES_ON : FSYNC_OBJECT_FILES_OFF; > > return 0; > > The original code used to allow the short-and-sweet valueless true > > [core] > fsyncobjectfiles > > but it no longer does by calling it a nonbool error. This breaks > existing users' repositories that have been happily working, doesn't > it? > > Perhaps > > if (value && !strcmp(value, "batch")) > fsync_object_files = FSYNC_OBJECT_FILES_BATCH; > else if (git_config_bool(var, value)) > fsync_object_files = FSYNC_OBJECT_FILES_ON; > else > fsync_object_files = FSYNC_OBJECT_FILES_OFF; I'll take your suggestion, including the change to case-sensitive. > > +#ifdef __APPLE__ > > + return fcntl(fd, F_FULLFSYNC); > > +#else > > + return fsync(fd); > > +#endif > > +} > > If we are introducing "enum fsync_action", we should have some way > to make it clear that we are covering all the possible values of > "action". > > Switching on action, i.e. > > switch (action) { > case FSYNC_WRITEOUT_ONLY: > ... > break; > case FSYNC_HARDWARE_FLUSH: > ... > break; > default: > BUG("unexpected git_fsync(%d) call", action); > } > > would be one way to do so. > Will do. Thanks for reviewing my changes. I've updated the github PR. I'll wait for a few more days to see if anyone has more feedback before sending out another round of patches.