Date: Thu, 9 Oct 2014 09:00:35 -0700 From: Neel Natu <neelnatu@gmail.com> To: dteske@freebsd.org Cc: svn-src-releng@freebsd.org, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, Neel Natu <neel@freebsd.org> Subject: Re: svn commit: r272752 - releng/10.1/sys/kern Message-ID: <CAFgRE9GVREVe6rK%2BMVZF5xaXcFBZEdgoZABwYa_MzBnO24nXbA@mail.gmail.com> In-Reply-To: <2b3301cfe39b$f2ae29d0$d80a7d70$@FreeBSD.org> References: <201410081539.s98FdPQo052864@svn.freebsd.org> <2a6d01cfe37c$ef5cf410$ce16dc30$@FreeBSD.org> <CAFgRE9FnVx1JMSxMu5yqBdaZL0dtSC0n=0BsWnRBjyyCMbe8sA@mail.gmail.com> <2b3301cfe39b$f2ae29d0$d80a7d70$@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Devin, On Thu, Oct 9, 2014 at 1:35 AM, <dteske@freebsd.org> wrote: > > >> -----Original Message----- >> From: owner-src-committers@freebsd.org [mailto:owner-src- >> committers@freebsd.org] On Behalf Of Neel Natu >> Sent: Wednesday, October 8, 2014 10:47 PM >> To: dteske@freebsd.org >> Cc: Neel Natu; src-committers@freebsd.org; svn-src-all@freebsd.org; svn- >> src-releng@freebsd.org >> Subject: Re: svn commit: r272752 - releng/10.1/sys/kern >> >> Hi Devin, >> >> On Wed, Oct 8, 2014 at 9:53 PM, <dteske@freebsd.org> wrote: >> > >> > >> >> -----Original Message----- >> >> From: owner-src-committers@freebsd.org [mailto:owner-src- >> >> committers@freebsd.org] On Behalf Of Neel Natu >> >> Sent: Wednesday, October 8, 2014 8:39 AM >> >> To: src-committers@freebsd.org; svn-src-all@freebsd.org; svn-src- >> >> releng@freebsd.org >> >> Subject: svn commit: r272752 - releng/10.1/sys/kern >> >> >> >> Author: neel >> >> Date: Wed Oct 8 15:39:24 2014 >> >> New Revision: 272752 >> >> URL: https://svnweb.freebsd.org/changeset/base/272752 >> >> >> >> Log: >> >> MFC r272270: >> > >> > I hate to pick nits, but I believe this revision (272752 in releng/10.1) >> > should (I suggest; deferring to re@ for final prognosis) perhaps have >> > _not_ been an MFC from head (r272270; as was performed) but >> > perhaps have instead been MFS from stable/10 (r272726). >> > >> >> The svn command used to do the merge was: >> # svn merge -c 272726 ^/stable/10 releng/10.1 >> >> This is exactly as per the SVN merge guidelines into releng as documented >> here: >> https://www.freebsd.org/doc/en/articles/committers-guide/subversion- >> primer.html >> > > Hi Neel, > > Nowhere in that document does it describe "MFS". > I'm happy you did the merge as it was supposed to be done. > However, your commit message is inaccurate. > I don't agree - the commit message is entirely self-consistent but I see why you might think otherwise. FWIW here is an analysis of all commits into releng/10.1. There are six commits with a commit message of "MFC <head_revision>": https://svnweb.freebsd.org/changeset/base/272684 https://svnweb.freebsd.org/changeset/base/272669 https://svnweb.freebsd.org/changeset/base/272612 https://svnweb.freebsd.org/changeset/base/272611 https://svnweb.freebsd.org/changeset/base/272608 https://svnweb.freebsd.org/changeset/base/272752 There is one commit with a commit message of "MFC <stable_10_revision>": https://svnweb.freebsd.org/changeset/base/272682 There is one commit with a commit message of "MF10 <stable_10_revision>": https://svnweb.freebsd.org/changeset/base/272819 Clearly there are different styles used by developers but the numbers suggest that "MFC <head_revision>" is the preferred one. best Neel >> The commit message used "MFC r272270" because that was the origin of >> the change and has the full details about the patch. >> > > I don't care if the subversion primer doesn't talk about "MFS" versus > "MFC", but it is technically inaccurate to call your commit an "MFC [of] > r272270" because it is in-fact (as you admit) an MFS of r272726 -- which > is indeed indicated by mergeinfo. > > Making someone look up the mergeinfo because the commit message > is a inaccurate doesn't make for an efficient/predictable setup. > -- > Cheers, > Devin > > P.S. If I was really on your case, I'd insist that you had put "MFS10 r#" > but in all reality, "MFS r#" would have been perfectly acceptable where > "MFC r#" is clearly just plain wrong and misleading. > >> best >> Neel >> >> > The nit being that mergeinfo now shows (unnaturally) that things >> > flowed from head -> stable / head -> releng versus >> > head -> stable -> releng as I suggest would have been cleaner for >> > historical analysis. >> > -- >> > Cheers, >> > Devin >> > >> >> >> >> tty_rel_free() can be called more than once for the same tty so make >> sure >> >> that the tty is dequeued from 'tty_list' only the first time. >> >> >> >> Approved by: re (glebius) >> >> >> >> Modified: >> >> releng/10.1/sys/kern/tty.c >> >> Directory Properties: >> >> releng/10.1/ (props changed) >> >> >> >> Modified: releng/10.1/sys/kern/tty.c >> >> >> ========================================================== >> >> ==================== >> >> --- releng/10.1/sys/kern/tty.c Wed Oct 8 15:30:59 2014 (r272751) >> >> +++ releng/10.1/sys/kern/tty.c Wed Oct 8 15:39:24 2014 (r272752) >> >> @@ -1055,13 +1055,13 @@ tty_rel_free(struct tty *tp) >> >> tp->t_dev = NULL; >> >> tty_unlock(tp); >> >> >> >> - sx_xlock(&tty_list_sx); >> >> - TAILQ_REMOVE(&tty_list, tp, t_list); >> >> - tty_list_count--; >> >> - sx_xunlock(&tty_list_sx); >> >> - >> >> - if (dev != NULL) >> >> + if (dev != NULL) { >> >> + sx_xlock(&tty_list_sx); >> >> + TAILQ_REMOVE(&tty_list, tp, t_list); >> >> + tty_list_count--; >> >> + sx_xunlock(&tty_list_sx); >> >> destroy_dev_sched_cb(dev, tty_dealloc, tp); >> >> + } >> >> } >> >> >> >> void >> > >> > > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFgRE9GVREVe6rK%2BMVZF5xaXcFBZEdgoZABwYa_MzBnO24nXbA>