Re: [PATCH v10 1/6] generic/631: add test for detached mount propagation

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Wed, Mar 24, 2021 at 10:04:39AM +0200, Amir Goldstein wrote:
> On Wed, Mar 24, 2021 at 9:50 AM Christian Brauner
> <christian.brauner@xxxxxxxxxx> wrote:
> >
> > On Wed, Mar 24, 2021 at 09:40:50AM +0200, Amir Goldstein wrote:
> > > On Mon, Mar 22, 2021 at 3:46 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> > > >
> > > > From: Christian Brauner <christian.brauner@xxxxxxxxxx>
> > > >
> > > > Regression test to verify that creating a series of detached mounts,
> > > > attaching them to the filesystem, and unmounting them does not trigger an
> > > > integer overflow in ns->mounts causing the kernel to block any new mounts in
> > > > count_mounts() and returning ENOSPC because it falsely assumes that the
> > > > maximum number of mounts in the mount namespace has been reached, i.e. it
> > > > thinks it can't fit the new mounts into the mount namespace anymore.
> > > >
> > > > The test is written in a way that it will leave the host's mount
> > > > namespace intact so we are sure to never make the host's mount namespace
> > > > unuseable!
> > > >
> > > > Link: https://git.kernel.org/torvalds/c/ee2e3f50629f17b0752b55b2566c15ce8dafb557
> > > > Cc: Eryu Guan <guan@xxxxxxx>
> > > > Cc: David Howells <dhowells@xxxxxxxxxx>
> > > > Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> > > > Signed-off-by: Christian Brauner <christian.brauner@xxxxxxxxxx>
> > > > ---
> > > > /* v1 - v8 */
> > > > patch not present
> > > >
> > > > /* v9 */
> > > > - Christian Brauner <christian.brauner@xxxxxxxxxx>:
> > > >   - Rebased on current master.
> > > >
> > > > /* v10 */
> > > > - Eryu Guan <guan@xxxxxxx>:
> > > >   - Add missing checks whether test is supported.
> > > >   - Move status=$? assignment up.
> > > > ---
> > >
> > > Technical nit: why did you add this extra --- line?
> > > It causes all the patch changelog to appear in the commit message.
> > > I don't think that was your intention? and I don't think it adds valuable
> > > into to git log.
> >
> > I've not done that in any of the other patches or on any other patches I
> > ever wrote so I think this was just a copy-paste error when I updated
> > the changelog.
> >
> 
> Please note this glitch happened in all the patches in the series...

Oh wait, actually looking at this again I think don't understand just
yet. This is pretty common and when you apply the series git am will cut
after the first
---

And thinking about it so far I've never heard anyone complain about
this. But likely I just misunderstand you and I really borked something.

Fwiw, this format seems documented at
https://www.kernel.org/doc/html/latest/process/submitting-patches.html

"The --- marker line serves the essential purpose of marking for patch
 handling tools where the changelog message ends.

 One good use for the additional comments after the --- marker is for a
 diffstat, to show what files have changed, and the number of inserted
 and deleted lines per file. A diffstat is especially useful on bigger
 patches. Other comments relevant only to the moment or the maintainer,
 not suitable for the permanent changelog, should also go here. A good
 example of such comments might be patch changelogs which describe what
 has changed between the v1 and v2 version of the patch.

 If you are going to include a diffstat after the --- marker, please use
 diffstat options -p 1 -w 70 so that filenames are listed from the top
 of the kernel source tree and don’t use too much horizontal space
 (easily fit in 80 columns, maybe with some indentation). (git generates
 appropriate diffstats by default.)"

And also at
https://www.ozlabs.org/~akpm/stuff/tpp.txt

"g) Non-changelog text:

    Sometimes you want to include text in the email which isn't designed to
    go into the changelog in the git repository.  Things like "this is already
    in Fred's tree" or "this is an updated version of last Friday's patch" or
    whatever.
 
    You should place such text below the "^---" separator so that it is
    auto-removed when being placed into the revision control system."

And there are quite a few examples that do it this way just one random
pick from my inbox:
https://lore.kernel.org/lkml/20210316204252.427806-8-mic@xxxxxxxxxxx

Christian



[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux