linux-kernel - Re: [PATCH 1/1] vfs: umount

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 24 Apr 2009 10:06:34 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	Alessio Igor Bogani <abogani@...ware.it>,
	Jonathan Corbet <corbet@....net>,
	Fr??d??ric Weisbecker <fweisbec@...il.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	LKML <linux-kernel@...r.kernel.org>,
	LFSDEV <linux-fsdevel@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Matthew Wilcox <matthew@....cx>
Subject: Re: [PATCH 1/1] vfs: umount_begin BKL pushdown v2

* Al Viro <viro@...IV.linux.org.uk> wrote:

> On Fri, Apr 24, 2009 at 08:13:12AM +0100, Al Viro wrote:
> 
> > AFAICS, both CIFS_SB(sb) and ->tcon are assign-once, so lock_kernel() should
> > really go here (if it can't be removed completely, of course, but that's up
> > to CIFS folks).  Applied with such modification.
> 
> commit 208f6be8f9244f4a3e8de7b4c6ca97069698303a in 
> git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6.git/
> 
> if you want to see the version after that change (or wait for 
> linux-next to pick it).

You've not replied to my request (attached below) to put these 
trivial BKL-pushdown bits into a separate branch/tree and not into 
the VFS tree. You've now mixed that commit with other VFS changes.

Had it been in a separate branch, and had we tested it, Linus could 
have pulled the trivial BKL pushdown bits out of normal merge order 
as well. That is not possible now.

Furthermore, by doing this you are also hindering the 
tip:kill-the-BKL effort (which has been ongoing for a year chipping 
away at various BKL details) which facilitated these changes. 
Alessio did these fixes to fix bugs he can trigger in that tree.

You've also not explained why you have done it this way. It would 
cost you almost nothing to apply these bits into a separate branch 
and merge that branch into your main tree. Lots of other maintainer 
are doing that.

So if you've done this by mistake, i'd like to ask you to reconsider 
and put these bits into a separate, stable-commit-ID branch. If 
you've done this intentionally, i'd like you to explain the reasons 
for it, instead of just doing it silently without explanation.

Anwyay, if there's no resolution, i'll apply Alessio's fixes with a 
different commit ID, to not hold up the rather useful work that is 
going on in the kill-the-BKL tree. Later on i'll have to rebase that 
portion of the tree to avoid duplicate commit IDs. I just wanted to 
put it on the record why i have to do that rebase.

Thanks,

	Ingo

----- Forwarded message from Ingo Molnar <mingo@...e.hu> -----

Date: Thu, 23 Apr 2009 23:32:49 +0200
From: Ingo Molnar <mingo@...e.hu>
To: Al Viro <viro@...IV.linux.org.uk>
Subject: Re: [PATCH 0/5 -tip] umount_begin BKL pushdown
Cc: Alessio Igor Bogani <abogani@...ware.it>,
	Jonathan Corbet <corbet@....net>,
	Fr??d??ric Weisbecker <fweisbec@...il.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	LKML <linux-kernel@...r.kernel.org>,
	LFSDEV <linux-fsdevel@...r.kernel.org>

* Al Viro <viro@...IV.linux.org.uk> wrote:

> On Thu, Apr 23, 2009 at 09:12:00PM +0200, Alessio Igor Bogani wrote:
> > Push the BKL acquisition from vfs to every specific filesystems
> > with hope that it can be eliminated in a second moment.
> > 
> > The first 4 patches add BKL lock into umount_begin() functions 
> > (for the filesystems that have this handler). The last one 
> > remove lock_kernel()/unlock_kernel() from fs/namespace.c (the 
> > only point that invoke umount_begin() funtcions).
> 
> I'd rather collapse all these patches together; no point doing 
> that per-fs (for all 4 of them).  And CIFS side is bogus.
> 
> Another thing: -tip is no place for that.  I can put that into VFS 
> tree, provided that comments above are dealt with.

When that happens, could you please put it into a separate, 
append-only branch i could pull (after some initial test-time) into 
tip:kill-the-BKL?

Thanks,

	Ingo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/