From mboxrd@z Thu Jan 1 00:00:00 1970 From: "J. Bruce Fields" Subject: Re: [RFC PATCH] locks: Show only file_locks created in the same pidns as current process Date: Tue, 2 Aug 2016 16:34:06 -0400 Message-ID: <20160802203406.GE15324@fieldses.org> References: <1470148943-21835-1-git-send-email-kernel@kyup.com> <87r3a7qhy0.fsf@x220.int.ebiederm.org> <20160802174003.GD11767@fieldses.org> <87invjq97h.fsf@x220.int.ebiederm.org> <20160802194437.GD15324@fieldses.org> <1470168082.15226.14.camel@poochiereds.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <1470168082.15226.14.camel-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Jeff Layton Cc: serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Nikolay Borisov , "Eric W. Biederman" , linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org List-Id: containers.vger.kernel.org T24gVHVlLCBBdWcgMDIsIDIwMTYgYXQgMDQ6MDE6MjJQTSAtMDQwMCwgSmVmZiBMYXl0b24gd3Jv dGU6Cj4gT24gVHVlLCAyMDE2LTA4LTAyIGF0IDE1OjQ0IC0wNDAwLCBKLiBCcnVjZSBGaWVsZHMg d3JvdGU6Cj4gPiBPbiBUdWUsIEF1ZyAwMiwgMjAxNiBhdCAwMjowOToyMlBNIC0wNTAwLCBFcmlj IFcuIEJpZWRlcm1hbiB3cm90ZToKPiA+ID4gCj4gPiA+ID4gPiAiSi4gQnJ1Y2UgRmllbGRzIiA8 YmZpZWxkc0BmaWVsZHNlcy5vcmc+IHdyaXRlczoKPiA+ID4gCj4gPiA+ID4gCj4gPiA+ID4gT24g VHVlLCBBdWcgMDIsIDIwMTYgYXQgMTE6MDA6MzlBTSAtMDUwMCwgRXJpYyBXLiBCaWVkZXJtYW4g d3JvdGU6Cj4gPiA+ID4gPiAKPiA+ID4gPiA+ID4gPiA+ID4gTmlrb2xheSBCb3Jpc292IDxrZXJu ZWxAa3l1cC5jb20+IHdyaXRlczoKPiA+ID4gPiA+IAo+ID4gPiA+ID4gPiAKPiA+ID4gPiA+ID4g Q3VycmVudGx5IHdoZW4gL3Byb2MvbG9ja3MgaXMgcmVhZCBpdCB3aWxsIHNob3cgYWxsIHRoZSBm aWxlIGxvY2tzCj4gPiA+ID4gPiA+IHdoaWNoIGFyZSBjdXJyZW50bHkgY3JlYXRlZCBvbiB0aGUg bWFjaGluZS4gT24gY29udGFpbmVycywgaG9zdGVkCj4gPiA+ID4gPiA+IG9uIGJ1c3kgc2VydmVy cyB0aGlzIG1lYW5zIHRoYXQgZG9pbmcgbHNvZiBjYW4gYmUgdmVyeSBzbG93LiBJCj4gPiA+ID4g PiA+IG9ic2VydmVkIHVwIHRvIDUgc2Vjb25kcyBzdGFsbHMgcmVhZGluZyA1MGsgbG9ja3MsIHdo aWxlIHRoZSBjb250YWluZXIKPiA+ID4gPiA+ID4gaXRzZWxmIGhhZCBvbmx5IGEgc21hbGwgbnVt YmVyIG9mIHJlbGV2YW50IGVudHJpZXMuIEZpeCBpdCBieQo+ID4gPiA+ID4gPiBmaWx0ZXJpbmcg dGhlIGxvY2tzIGxpc3RlZCBieSB0aGUgcGlkbnMgb2YgdGhlIGN1cnJlbnQgcHJvY2Vzcwo+ID4g PiA+ID4gPiBhbmQgdGhlIHByb2Nlc3Mgd2hpY2ggY3JlYXRlZCB0aGUgbG9jay4KPiA+ID4gPiA+ IAo+ID4gPiA+ID4gVGhlIGxvY2tzIGFsd2F5cyBjb25mdXNlIG1lIHNvIEkgYW0gbm90IDEwMCUg Y29ubmVjdGluZyBsb2Nrcwo+ID4gPiA+ID4gdG8gYSBwaWQgbmFtZXNwYWNlIGlzIGFwcHJvcHJp YXRlLgo+ID4gPiA+ID4gCj4gPiA+ID4gPiBUaGF0IHNhaWQgaWYgeW91IGFyZSBnb2luZyB0byBm aWx0ZXIgYnkgcGlkIG5hbWVzcGFjZSBwbGVhc2UgdXNlIHRoZSBwaWQKPiA+ID4gPiA+IG5hbWVz cGFjZSBvZiBwcm9jLCBub3QgdGhlIHBpZCBuYW1lc3BhY2Ugb2YgdGhlIHByb2Nlc3MgcmVhZGlu ZyB0aGUKPiA+ID4gPiA+IGZpbGUuCj4gPiA+ID4gCj4gPiA+ID4gT2gsIHRoYXQgbWFrZXMgc2Vu c2UsIHRoYW5rcy4KPiA+ID4gPiAKPiA+ID4gPiBXaGF0IGRvZXMgL3Byb2MvbW91bnRzIHVzZSwg b3V0IG9mIGN1cmlvc2l0eT/CoMKgVGhlIG1vdW50IG5hbWVzcGFjZSB0aGF0Cj4gPiA+ID4gL3By b2Mgd2FzIG9yaWdpbmFsbHkgbW91bnRlZCBpbj8KPiA+ID4gCj4gPiA+IC9wcm9jL21vdW50cyAt PiAvcHJvYy9zZWxmL21vdW50cwo+ID4gCj4gPiBEJ29oLCBJIGtuZXcgdGhhdC4KPiA+IAo+ID4g PiAKPiA+ID4gL3Byb2MvW3BpZF0vbW91bnRzIGxpc3RzIG1vdW50cyBmcm9tIHRoZSBtb3VudCBu YW1lc3BhY2Ugb2YgdGhlCj4gPiA+IGFwcHJvcHJpYXRlIHByb2Nlc3MuCj4gPiA+IAo+ID4gPiBU aGF0IGlzIGFub3RoZXIgd2F5IHRvIGdvIGJ1dCBpdCBpcyBhIHRyZWFkIGNhcmVmdWxseSB0aGlu ZyBhcyBjaGFuZ2luZwo+ID4gPiB0aGluZ3MgdGhhdCB3YXkgaXQgaXMgZWFzeSB0byBzdXJwcmlz ZSBhcHBhcm1vciBvciBzZWxpbnV4IHJ1bGVzIGFuZCBiZQo+ID4gPiBzdXJwcmlzZWQgeW91IGJy b2tlIHNvbWVvbmVzIHVzZXJzcGFjZSBpbiBhIHdheSB0aGF0IHByZXZlbnRzIGJvb3RpbmcuCj4g PiA+IEFsdGhvdWdoIEkgc3VzcGVjdCAvcHJvYy9sb2NrcyBpc24ndCB0b28gYmFkLgo+ID4gCj4g PiBPSywgdGhhbmtzLgo+ID4gCj4gPiAvcHJvYy9bcGlkXS9sb2NrcyBtaWdodCBiZSBjb25mdXNp bmcuwqDCoEknZCBleHBlY3QgaXQgdG8gYmUgImFsbCB0aGUKPiA+IGxvY2tzIG93bmVkIGJ5IHRo aXMgdGFzayIsIHJhdGhlciB0aGFuICJhbGwgdGhlIGxvY2tzIG93bmVkIGJ5IHBpZCdzIGluCj4g PiB0aGUgc2FtZSBwaWQgbmFtZXNwYWNlIiwgb3Igd2hhdGV2ZXIgY3JpdGVyaW9uIHdlIGNob29z ZS4KPiA+IAo+ID4gVWgsIEknbSBzdGlsbCB0cnlpbmcgdG8gdGhpbmsgb2YgdGhlIE9idmlvdXNs eSBSaWdodCBzb2x1dGlvbiBoZXJlLCBhbmQKPiA+IGl0J3Mgbm90IGNvbWluZy4KPiA+IAo+ID4g LS1iLgo+IAo+IAo+IEknbSBhIGxpdHRsZSBsZWVyeSBvZiBjaGFuZ2luZyBob3cgdGhpcyB3b3Jr cy4gSXQgaGFzIGFsd2F5cyBiZWVuCj4gbWFpbnRhaW5lZCBhcyBhIGxlZ2FjeSBpbnRlcmZhY2Us IHNvIGRvIHdlIHJ1biB0aGUgcmlzayBvZiBicmVha2luZwo+IHNvbWV0aGluZyBpZiB3ZSB0dXJu IGl0IGludG8gYSBwZXItbmFtZXNwYWNlIHRoaW5nPwoKVGhlIG5hbWVzcGFjZSB3b3JrIGlzIGFs bCBhYm91dCBtYWtpbmcgaW50ZXJmYWNlcyBwZXItbmFtZXNwYWNlLiAgSQpndWVzcyBpdCB3b3Jr cyBhcyBsb25nIGFzIGl0IGNvbnRyaWJ1dGVzIHRvIHRoZSBpbGx1c2lvbiB0aGF0IGVhY2gKY29u dGFpbmVyIGlzIGl0cyBvd24gbWFjaGluZS4KClRoaW5raW5nIGFib3V0IGl0LCBJIG1pZ2h0IGJl IHNvbGQgb24gdGhlIHBlci1waWRucyBhcHByb2FjaCAod2l0aApFcmljJ3MgbW9kaWZpY2F0aW9u IHRvIHVzZSB0aGUgcGlkbnMgb2YgL3Byb2Mgbm90IHRoZSByZWFkZXIpLgoKTXkgY29tcGxhaW50 IGFib3V0IG5vdCBiZWluZyBhYmxlIHRvIHNlZSBjb25mbGljdGluZyBsb2NrcyB3b3VsZCBhcHBs eQpqdXN0IGFzIHdlbGwgdG8gY29uZmxpY3RzIGZyb20gbmZzIGxvY2tzIGhlbGQgYnkgb3RoZXIg Y2xpZW50cy4gIEEgZGlzawpmaWxlc3lzdGVtIHNoYXJlZCBhY3Jvc3MgbXVsdGlwbGUgY29udGFp bmVycyBpcyBhIGxpdHRsZSBsaWtlIGFuIG5mcwpmaWxlc3lzdGVtIHNoYXJlZCBiZXR3ZWVuIG5m cyBjbGllbnRzLgoKVGhhdCdkIHNvbHZlIHRoaXMgaW1tZWRpYXRlIHByb2JsZW0gd2l0aG91dCBy ZXF1aXJpbmcgYW4gbHNvZiB1cGdyYWRlIGFzCndlbGwuCgo+IFRoaXMgYWxzbyBkb2Vzbid0Cj4g c29sdmUgdGhlIHByb2JsZW0gb2Ygc2xvdyB0cmF2ZXJzYWwgaW4gdGhlIGluaXRfcGlkX25zIC0t IG9ubHkgaW4gYQo+IGNvbnRhaW5lci4KPiAKPiBJIGFsc28gY2FuJ3QgaGVscCBidXQgZmVlbCB0 aGF0IC9wcm9jL2xvY2tzIGlzIGp1c3Qgc2hvd2luZyBpdHMgYWdlLiBJdAo+IHdhcyBmaW5lIGlu IHRoZSBsYXRlIDkwJ3MsIGJ1dCBpdHMgbGltaXRhdGlvbnMgYXJlIGp1c3QgYmVjb21pbmcgbW9y ZQo+IGFwcGFyZW50IGFzIHRoaW5ncyBnZXQgbW9yZSBjb21wbGV4LiBJdCB3YXMgbmV2ZXIgZGVz aWduZWQgZm9yCj4gcGVyZm9ybWFuY2UgYXMgeW91IGVuZCB1cCB0aHJhc2hpbmcgc2V2ZXJhbCBz cGlubG9ja3Mgd2hlbiByZWFkaW5nIGl0Lgo+IAo+IE1heWJlIGl0J3MgdGltZSB0byB0aGluayBh Ym91dCBwcmVzZW50aW5nIHRoaXMgaW5mbyBpbiBhbm90aGVyIHdheT8gQQo+IGdsb2JhbCB2aWV3 IG9mIGFsbCBsb2NrcyBvbiB0aGUgc3lzdGVtIGlzIGludGVyZXN0aW5nIGJ1dCBtYXliZSBpdAo+ IHdvdWxkIGJlIGJldHRlciB0byBwcmVzZW50IGl0IG1vcmUgZ3JhbnVsYXJseSBzb21laG93PwoK QnV0LCB5ZXMsIHRoYXQgbWlnaHQgYmUgYSBnb29kIGlkZWEuCgotLWIuCgo+IAo+IEkgZ3Vlc3Mg SSBzaG91bGQgZ28gbG9vayBhdCB3aGF0IGxzb2YgYWN0dWFsbHkgZG9lcyB3aXRoIHRoaXMgaW5m by4uLgo+IAo+IC0tIAo+IEplZmYgTGF5dG9uIDxqbGF5dG9uQHBvb2NoaWVyZWRzLm5ldD4KX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KQ29udGFpbmVycyBt YWlsaW5nIGxpc3QKQ29udGFpbmVyc0BsaXN0cy5saW51eC1mb3VuZGF0aW9uLm9yZwpodHRwczov L2xpc3RzLmxpbnV4Zm91bmRhdGlvbi5vcmcvbWFpbG1hbi9saXN0aW5mby9jb250YWluZXJz From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from fieldses.org ([173.255.197.46]:50044 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757648AbcHBUgZ (ORCPT ); Tue, 2 Aug 2016 16:36:25 -0400 Date: Tue, 2 Aug 2016 16:34:06 -0400 From: "J. Bruce Fields" To: Jeff Layton Cc: "Eric W. Biederman" , Nikolay Borisov , viro@zeniv.linux.org.uk, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, containers@lists.linux-foundation.org, serge.hallyn@canonical.com Subject: Re: [RFC PATCH] locks: Show only file_locks created in the same pidns as current process Message-ID: <20160802203406.GE15324@fieldses.org> References: <1470148943-21835-1-git-send-email-kernel@kyup.com> <87r3a7qhy0.fsf@x220.int.ebiederm.org> <20160802174003.GD11767@fieldses.org> <87invjq97h.fsf@x220.int.ebiederm.org> <20160802194437.GD15324@fieldses.org> <1470168082.15226.14.camel@poochiereds.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1470168082.15226.14.camel@poochiereds.net> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Aug 02, 2016 at 04:01:22PM -0400, Jeff Layton wrote: > On Tue, 2016-08-02 at 15:44 -0400, J. Bruce Fields wrote: > > On Tue, Aug 02, 2016 at 02:09:22PM -0500, Eric W. Biederman wrote: > > > > > > > > "J. Bruce Fields" writes: > > > > > > > > > > > On Tue, Aug 02, 2016 at 11:00:39AM -0500, Eric W. Biederman wrote: > > > > > > > > > > > > > > Nikolay Borisov writes: > > > > > > > > > > > > > > > > > Currently when /proc/locks is read it will show all the file locks > > > > > > which are currently created on the machine. On containers, hosted > > > > > > on busy servers this means that doing lsof can be very slow. I > > > > > > observed up to 5 seconds stalls reading 50k locks, while the container > > > > > > itself had only a small number of relevant entries. Fix it by > > > > > > filtering the locks listed by the pidns of the current process > > > > > > and the process which created the lock. > > > > > > > > > > The locks always confuse me so I am not 100% connecting locks > > > > > to a pid namespace is appropriate. > > > > > > > > > > That said if you are going to filter by pid namespace please use the pid > > > > > namespace of proc, not the pid namespace of the process reading the > > > > > file. > > > > > > > > Oh, that makes sense, thanks. > > > > > > > > What does /proc/mounts use, out of curiosity?  The mount namespace that > > > > /proc was originally mounted in? > > > > > > /proc/mounts -> /proc/self/mounts > > > > D'oh, I knew that. > > > > > > > > /proc/[pid]/mounts lists mounts from the mount namespace of the > > > appropriate process. > > > > > > That is another way to go but it is a tread carefully thing as changing > > > things that way it is easy to surprise apparmor or selinux rules and be > > > surprised you broke someones userspace in a way that prevents booting. > > > Although I suspect /proc/locks isn't too bad. > > > > OK, thanks. > > > > /proc/[pid]/locks might be confusing.  I'd expect it to be "all the > > locks owned by this task", rather than "all the locks owned by pid's in > > the same pid namespace", or whatever criterion we choose. > > > > Uh, I'm still trying to think of the Obviously Right solution here, and > > it's not coming. > > > > --b. > > > I'm a little leery of changing how this works. It has always been > maintained as a legacy interface, so do we run the risk of breaking > something if we turn it into a per-namespace thing? The namespace work is all about making interfaces per-namespace. I guess it works as long as it contributes to the illusion that each container is its own machine. Thinking about it, I might be sold on the per-pidns approach (with Eric's modification to use the pidns of /proc not the reader). My complaint about not being able to see conflicting locks would apply just as well to conflicts from nfs locks held by other clients. A disk filesystem shared across multiple containers is a little like an nfs filesystem shared between nfs clients. That'd solve this immediate problem without requiring an lsof upgrade as well. > This also doesn't > solve the problem of slow traversal in the init_pid_ns -- only in a > container. > > I also can't help but feel that /proc/locks is just showing its age. It > was fine in the late 90's, but its limitations are just becoming more > apparent as things get more complex. It was never designed for > performance as you end up thrashing several spinlocks when reading it. > > Maybe it's time to think about presenting this info in another way? A > global view of all locks on the system is interesting but maybe it > would be better to present it more granularly somehow? But, yes, that might be a good idea. --b. > > I guess I should go look at what lsof actually does with this info... > > -- > Jeff Layton