Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755999AbbDTVv0 (ORCPT ); Mon, 20 Apr 2015 17:51:26 -0400 Received: from zeniv.linux.org.uk ([195.92.253.2]:35025 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755989AbbDTVvX (ORCPT ); Mon, 20 Apr 2015 17:51:23 -0400 Date: Mon, 20 Apr 2015 22:51:20 +0100 From: Al Viro To: Linus Torvalds Cc: Neil Brown , Linux Kernel Mailing List , linux-fsdevel Subject: Re: [PATCH 16/24] link_path_walk: kill the recursion Message-ID: <20150420215120.GO889@ZenIV.linux.org.uk> References: <20150420181222.GK889@ZenIV.linux.org.uk> <1429553588-24764-16-git-send-email-viro@ZenIV.linux.org.uk> <20150420213247.GN889@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2185 Lines: 51 On Mon, Apr 20, 2015 at 02:39:43PM -0700, Linus Torvalds wrote: > On Mon, Apr 20, 2015 at 2:32 PM, Al Viro wrote: > > > > What kilobyte? It's 9*4 pointers, IOW, 288 bytes total (assuming 64bit box). > > You also said that you were going to up the recursion limit to 40.. So > 40*3*8 bytes.. Er... That's exactly what || We could reduce it further (see below), but I'm not sure it's worth || doing - it's not much extra complexity, but we only squeeze out ~250 bytes || that way, with the worst-case footprints (those are triggered by rename()) || are around 1.4Kb (instead of about twice as much in mainline). OTOH, || that extra complexity would've allowed us to get rid of the nesting limit || entirely (i.e. we get ELOOP when we encounter 40 symlinks, no matter in || which manner they are nested). That might be worth considering... had been about. And yes, it is easy to implement - new nameidata flag for "need to kfree() nd->stack", if (unlikely(current->link_count >= MAX_NESTED_LINKS)) { path_put_conditional(&next, nd); path_put(&nd->path); return -ELOOP; } BUG_ON(nd->depth >= MAX_NESTED_LINKS); nd->depth++; replaced with if (nd->depth == 2 && !(nd->flags & LOOKUP_KFREE)) { struct saved *p = kmalloc(41 * sizeof(*p)); if (!p) { path_put_conditional(&next, nd); path_put(&nd->path); return -ENOMEM; } memcpy(p, nd->stack, 2 * sizeof(*p)); nd->stack = p; nd->flags |= LOOKUP_KFREE; } nd->depth++; with obvious logics for freeing that crap afterwards. I really don't like the idea of putting it into nameidata, BTW - consider e.g. rename(). We don't need the contents of that thing after the link_path_walk() returns; no point duplicating it... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/