Date: Thu, 25 Jun 2020 09:09:10 +0200 From: "Kristof Provost" <kp@FreeBSD.org> To: "Emanuel Haupt" <ehaupt@FreeBSD.org> Cc: "Ed Maste" <emaste@freebsd.org>, freebsd-git@freebsd.org Subject: Re: svn primer translation to git Message-ID: <A5948BAC-C861-45A5-8AD4-F4A9D3A44EF3@FreeBSD.org> In-Reply-To: <20200625082455.9f1343f6f353f4be24385bc6@FreeBSD.org> References: <CAPyFy2Dy4cLQpgUsk_ushXsQFvRPk2S8SEfgWF=0xibGRGJvKw@mail.gmail.com> <20200625082455.9f1343f6f353f4be24385bc6@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 25 Jun 2020, at 8:24, Emanuel Haupt wrote: > Ed Maste <emaste@freebsd.org> wrote: >> We currently have a FreeBSD Subversion primer in the committer's >> guide: >> https://www.freebsd.org/doc/en_US.ISO8859-1/articles/committers-guide/= subversion-primer.html >> >> I've started a translation of this into a Git primer, at: >> https://hackmd.io/ML5TSl8mQ5-27B5eqDf7YA?view >> >> I'm particularly interested in feedback on how much git background / >> theory of operation we want to include here (vs referring to external >> documentation). > > Thank you for writing the document. > > Maybe we could address the convention [1] that with git commit = > messages > usually begin with a subject, followed by a blank line and then the > commit body. > > This schema produces beautiful commit logs on web-based DevOps = > lifecycle > tools such as github, gitlab, gitea. > That=E2=80=99s a good point. Most of our commit messages already follow this form, so it shouldn=E2=80= =99t = be a painful adjustment. I=E2=80=99ve had a look at the last few commits, and only the MFCs stand = out = as needing a little attention in that area. To grab a random example: MFC r362183 netmap: vtnet: fix races in vtnet_netmap_reg() The nm_register callback needs to call nm_set_native_flags() or nm_clear_native_flags() once the device has been stopped. However, in the current implementation this is not true, as the device is stopped by vtnet_init_locked(). This causes race conditions where the driver crashes as soon as it dequeues netmap buffers assuming they are mbufs (or the other way around). To fix the issue, we extend vtnet_init_locked() with a second argument that, if not zero, will set/clear the netmap flags. This results in a huge simplification of the nm_register callback itself. Also, use netmap_reset() to check if a ring is going to be re-initialized in netmap mode. The original commit already followed the =E2=80=98headline summary, expan= ded = body=E2=80=99 rule. This MFC would also work (often there=E2=80=99s no bl= ank line = between the =E2=80=98MFC=E2=80=99 and the next line), but I do wonder if = `MFC = r362183: netmap: vtnet: fix races in vtnet_netmap_reg()` wouldn=E2=80=99t= be = better. It=E2=80=99s a minor point, but we may as well document a choice now. I h= ave = my preferences, but most of all I=E2=80=99d prefer uniformity. Best regards, Kristof From owner-freebsd-git@freebsd.org Thu Jun 25 09:13:34 2020 Return-Path: <owner-freebsd-git@freebsd.org> Delivered-To: freebsd-git@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id C7E3B356041 for <freebsd-git@mailman.nyi.freebsd.org>; Thu, 25 Jun 2020 09:13:34 +0000 (UTC) (envelope-from mpp302@gmail.com) Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49svSs2Nvjz3ZF1; Thu, 25 Jun 2020 09:13:33 +0000 (UTC) (envelope-from mpp302@gmail.com) Received: by mail-ej1-f47.google.com with SMTP id p20so5166971ejd.13; Thu, 25 Jun 2020 02:13:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=nyllCAIS5Hs1/L0pjS0+UBe49lgzo7b/cz0et6CmnbQ=; b=tnlxG7xFDSqNI4kWhRC6YTlyZ7JxDWtIePvNmEsg64QDXqTVMYijC8EC2JlBF/fpm3 /STFV59UqLjxPUxrojIezKR0W4Gb59UlEecBPnLTFBVzPZZIUolmCKovLDAL6ZhjaCGA QFZa7phZgUDrg+eq5dNQGsrTY5d9SZYBsGv3BzH1rvj067iJR2zPfEZru86tPvjjaOQN L8vuXTGMR6/7HnGoXxfsWXA/B14xb2Cvm29eU6CS63tB/hU17MJKal36wIdJEOBn4JOv 3FEmgnlCHqc1Q6oRBvbu+OhS/ERYgfiwQ5MIEAsTPSAAWvE3Nc0uuYQsn7LMR55KCkEp 1/CQ== X-Gm-Message-State: AOAM531A5oMdi0+1ifHcHJyPq/XlnW/lbI8KAlhuFowXNpOM5ZcPMCwL nvYuiYHZWO4enzH4ZrJKYt57jH0Ocg4= X-Google-Smtp-Source: ABdhPJyuyvnRQI5ZzP6ZaGIURePf+rx2arjXUFtUrpJ15lP3KSE5CXBKbQ13qFZ/jrvlVfiJcA85gw== X-Received: by 2002:a17:906:af62:: with SMTP id os2mr28350729ejb.345.1593076411583; Thu, 25 Jun 2020 02:13:31 -0700 (PDT) Received: from ?IPv6:2a02:8109:98c0:1bc0:5e5f:67ff:fef4:ffd8? ([2a02:8109:98c0:1bc0:5e5f:67ff:fef4:ffd8]) by smtp.gmail.com with ESMTPSA id v11sm6686224eja.113.2020.06.25.02.13.30 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 25 Jun 2020 02:13:30 -0700 (PDT) Subject: Re: svn primer translation to git To: Kristof Provost <kp@FreeBSD.org>, Emanuel Haupt <ehaupt@FreeBSD.org> Cc: freebsd-git@freebsd.org References: <CAPyFy2Dy4cLQpgUsk_ushXsQFvRPk2S8SEfgWF=0xibGRGJvKw@mail.gmail.com> <20200625082455.9f1343f6f353f4be24385bc6@FreeBSD.org> <A5948BAC-C861-45A5-8AD4-F4A9D3A44EF3@FreeBSD.org> From: Mateusz Piotrowski <0mp@FreeBSD.org> Message-ID: <ed4f1b03-371b-54c4-28ac-003aeaf7f61b@FreeBSD.org> Date: Thu, 25 Jun 2020 11:13:30 +0200 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: <A5948BAC-C861-45A5-8AD4-F4A9D3A44EF3@FreeBSD.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 49svSs2Nvjz3ZF1 X-Spamd-Bar: - Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of mpp302@gmail.com designates 209.85.218.47 as permitted sender) smtp.mailfrom=mpp302@gmail.com X-Spamd-Result: default: False [-1.90 / 15.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; RCVD_TLS_ALL(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; R_SPF_ALLOW(-0.20)[+ip4:209.85.128.0/17:c]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[FreeBSD.org]; NEURAL_HAM_LONG(-0.87)[-0.866]; TO_DN_SOME(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; RCVD_COUNT_THREE(0.00)[3]; NEURAL_HAM_MEDIUM(-0.70)[-0.697]; NEURAL_HAM_SHORT(-0.33)[-0.335]; RCVD_IN_DNSWL_NONE(0.00)[209.85.218.47:from]; FORGED_SENDER(0.30)[0mp@FreeBSD.org,mpp302@gmail.com]; RWL_MAILSPIKE_POSSIBLE(0.00)[209.85.218.47:from]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; FROM_NEQ_ENVFROM(0.00)[0mp@FreeBSD.org,mpp302@gmail.com]; FREEMAIL_ENVFROM(0.00)[gmail.com] X-BeenThere: freebsd-git@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: Discussion of git use in the FreeBSD project <freebsd-git.freebsd.org> List-Unsubscribe: <https://lists.freebsd.org/mailman/options/freebsd-git>, <mailto:freebsd-git-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/freebsd-git/>; List-Post: <mailto:freebsd-git@freebsd.org> List-Help: <mailto:freebsd-git-request@freebsd.org?subject=help> List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/freebsd-git>, <mailto:freebsd-git-request@freebsd.org?subject=subscribe> X-List-Received-Date: Thu, 25 Jun 2020 09:13:34 -0000 On 6/25/20 9:09 AM, Kristof Provost wrote: > On 25 Jun 2020, at 8:24, Emanuel Haupt wrote: >> Ed Maste <emaste@freebsd.org> wrote: >>> We currently have a FreeBSD Subversion primer in the committer's >>> guide: >>> https://www.freebsd.org/doc/en_US.ISO8859-1/articles/committers-guide/subversion-primer.html >>> >>> >>> I've started a translation of this into a Git primer, at: >>> https://hackmd.io/ML5TSl8mQ5-27B5eqDf7YA?view >>> >>> I'm particularly interested in feedback on how much git background / >>> theory of operation we want to include here (vs referring to external >>> documentation). >> [...] > > I’ve had a look at the last few commits, and only the MFCs stand out as > needing a little attention in that area. > To grab a random example: > > MFC r362183 > > netmap: vtnet: fix races in vtnet_netmap_reg() > > The nm_register callback needs to call nm_set_native_flags() > or nm_clear_native_flags() once the device has been stopped. [...] > > The original commit already followed the ‘headline summary, expanded > body’ rule. This MFC would also work (often there’s no blank line > between the ‘MFC’ and the next line), but I do wonder if `MFC r362183: > netmap: vtnet: fix races in vtnet_netmap_reg()` wouldn’t be better. +1. And then if you're MFC'ing multiple commits at once, you'd just add a little title capturing the change as a whole?
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?A5948BAC-C861-45A5-8AD4-F4A9D3A44EF3>