From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A3C27C43460 for ; Wed, 19 May 2021 00:44:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 865DB6108B for ; Wed, 19 May 2021 00:44:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346920AbhESAqD (ORCPT ); Tue, 18 May 2021 20:46:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52846 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244461AbhESAqC (ORCPT ); Tue, 18 May 2021 20:46:02 -0400 Received: from zeniv-ca.linux.org.uk (zeniv-ca.linux.org.uk [IPv6:2607:5300:60:148a::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 11B6AC06175F; Tue, 18 May 2021 17:44:44 -0700 (PDT) Received: from viro by zeniv-ca.linux.org.uk with local (Exim 4.94 #2 (Red Hat Linux)) id 1ljAIt-00G48Y-QL; Wed, 19 May 2021 00:43:15 +0000 Date: Wed, 19 May 2021 00:43:15 +0000 From: Al Viro To: Linus Torvalds Cc: Jia He , Petr Mladek , Steven Rostedt , Sergey Senozhatsky , Andy Shevchenko , Rasmus Villemoes , Jonathan Corbet , Al Viro , Heiko Carstens , Vasily Gorbik , Christian Borntraeger , "Eric W . Biederman" , "Darrick J. Wong" , "Peter Zijlstra (Intel)" , Ira Weiny , Eric Biggers , "Ahmed S. Darwish" , "open list:DOCUMENTATION" , Linux Kernel Mailing List , linux-s390 , linux-fsdevel Subject: [PATCHSET] d_path cleanups Message-ID: References: <20210508122530.1971-1-justin.he@arm.com> <20210508122530.1971-2-justin.he@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: Al Viro Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org First of all, apologies for delays (one of the disks on the main devel/testing box has become an ex-parrot, with... interesting recovery). Here's what I've got for carve-up of cleanups. This stuff lives in git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.d_path, individual patches in followups. 14 commits, all changes are to fs/d_path.c, total being +133/-191. Moderately tested, seems to work here. Review and testing would be welcome... Part 1: trivial preliminary cleanup 1/14) d_path: "\0" is {0,0}, not {0} A bunch of places used "\0" as a literal for 1-element array consisting of NULs. That should've been "", obviously... Part 2: untangling __dentry_path() 2/14) d_path: saner calling conventions for __dentry_path() __dentry_path() used to copy the calling conventions for dentry_path(). That was fine for use in dentry_path_raw(), but it created a lot of headache in dentry_path(), since we might need a suffix (//deleted) in there, without NUL between it and the pathname itself. So we had to 1) (possibly) put /deleted into buffer and remember where it went 2) let __dentry_path() prepend NUL-terminated pathname 3) override NUL with / if we had done (1) Life becomes much easier if __dentry_path() does *NOT* put NUL in there. Then dentry_path_raw() becomes 'put "" into buffer, then __dentry_path()' and dentry_path() - 'put "" or "//deleted" into buffer, then __dentry_path()'. Additionally, we switch the way buffer information is passed to one similar to what we do in prepend()/prepend_name()/etc., i.e. pass the pointer to the end of buffer instead of that to beginning. That's what we'd been using in __dentry_path() all along, and now that the callers are doing some prepend() before calling __dentry_path(), they that value already on hand. 3/14) d_path: regularize handling of root dentry in __dentry_path() All path-forming primitives boil down to sequence of prepend_name() on dentries encountered along the way toward root. Each time we prepend / + dentry name to the buffer. Normally that does exactly what we want, but there's a corner case when we don't call prepend_name() at all (in case of __dentry_path() that happens if we are given root dentry). We obviously want to end up with "/", rather than "", so this corner case needs to be handled. __dentry_path() used to manually put '/' in the end of buffer before doing anything else, to be overwritten by the first call of prepend_name() if one happens and to be left in place if we don't call prepend_name() at all. That required manually checking that we had space in the buffer (prepend_name() and prepend() take care of such checks themselves) and lead to clumsy keeping track of return value. A better approach is to check if the main loop has added anything into the buffer and prepend "/" if it hasn't. A side benefit of using prepend() is that it does the right thing if we'd already run out of buffer, making the overflow-handling logics simpler. NB: the above might be worth putting into commit message. Part 3: overflow handling cleanups We have an overcomplicated handling of overflows. Primitives (prepend() and prepend_name()) check if we'd run out of space and return -ENAMETOOLONG. Then it's propagated all the way out by call chain. However, the same primitives are safe to call in case we'd *already* run out of space and that condition is easily checked at any level of callchain. The next 5 commits use that to simplify the control flow. 4/14) d_path: get rid of path_with_deleted() expand into the sole caller, rearrange the suffix handing along the lines of dentry_path(). 5/14) getcwd(2): saner logics around prepend_path() call Turn prepend_path() if it says it has run out of space, fail with ENAMETOOLONG if it wants "(unreachable) " prepended do so if that says it has run out of space, fail with ENAMETOOLONG into prepend_path() if it wants "(unreachable) " prepended do so if we see we'd run out of space at some point fail with ENAMETOOLONG 6/14) d_path: don't bother with return value of prepend() Almost nothing is looking at return value of prepend() and it's easy to get rid of the last stragglers... 7/14) d_path: lift -ENAMETOOLONG handling into callers of prepend_path() It's easier to have prepend_path() return 0 on overflow and check for overflow in the callers. The logics is the same for all callers (ran out of space => ERR_PTR(-ENAMETOOLONG)), so we get a bit of boilerplate, but even with that the callers become simpler. Added boilerplate will be dealt with a couple of commits down the road. 8/14) d_path: make prepend_name() boolean Unlike the case of prepend(), callers of prepend_name() really want to see whether it has run out of space - the loops it's called in are lockless and we could, in principle, end up spinning there for indetermined amount of iterations. Dropping out of loop if we run out of space in the buffers serves as a backstop for (very unlikely) cases. However, all we care about is success/failure - we generate ENAMETOOLONG in the callers, if not callers of callers, so we can bloody well make it return bool. Part 4: introduction of prepend_buffer 9/14) d_path: introduce struct prepend_buffer We've a lot of places where we have pairs of form (pointer to end of buffer, amount of space left in front of that). These sit in pairs of variables located next to each other and usually passed by reference. Turn those into instances of new type (struct prepend_buffer) and pass reference to the pair instead of pairs of references to its fields. Initialization (of form {buf + len, len}) turned into a macro (DECLARE_BUF), to avoid brainos. Extraction of string (buffer contents if we hadn't run out of space, ERR_PTR(-ENAMETOOLONG) otherwise) is done by extract_string(); that eats the leftover boilerplate from earlier in the series. Part 5: extracting the lockless part of prepend_path() The thing that started the entire mess had been an attempt to use d_path machinery for vsprintf(); that can't grab rename_lock and mount_lock, so we needed a variant of prepend_path() that would try to go without those locks. The obvious approach is to lift the internal loop of prepend_path() into a new primitive. However, that needs some massage first to separate local variables - otherwise we end up with the argument list from hell. 10/14) d_path: prepend_path(): get rid of vfsmnt redundant - we maintain mnt and vfsmnt through the loop, with the latter being a pointer to mnt->mnt all along. 11/14) d_path: prepend_path(): lift resetting b in case when we'd return 3 out of loop the only place in the inner loop where we need p; we use it to reset b in case we would return 3. We can easily check that error is 3 after the loop and do resetting there. Note that we only need that after the *outer* loop - the body of that starts with assignment to b anyway. 12/14) d_path: prepend_path(): lift the inner loop into a new helper ta-da Part 6: followups 13/14) d_path: prepend_path() is unlikely to return non-zero t14/14) getcwd(2): clean up error handling