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=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 6275EC636CA for ; Thu, 15 Jul 2021 20:17:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 479F1613C4 for ; Thu, 15 Jul 2021 20:17:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343601AbhGOUUu (ORCPT ); Thu, 15 Jul 2021 16:20:50 -0400 Received: from zeniv-ca.linux.org.uk ([142.44.231.140]:60670 "EHLO zeniv-ca.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241154AbhGOUUn (ORCPT ); Thu, 15 Jul 2021 16:20:43 -0400 Received: from viro by zeniv-ca.linux.org.uk with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1m47nj-000xOY-8c; Thu, 15 Jul 2021 20:17:43 +0000 Date: Thu, 15 Jul 2021 20:17:43 +0000 From: Al Viro To: Dmitry Kadashev Cc: Jens Axboe , Christian Brauner , Linus Torvalds , linux-fsdevel@vger.kernel.org, io-uring@vger.kernel.org Subject: Re: [PATCH 05/14] namei: prepare do_mkdirat for refactoring Message-ID: References: <20210715103600.3570667-1-dkadashev@gmail.com> <20210715103600.3570667-6-dkadashev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210715103600.3570667-6-dkadashev@gmail.com> Sender: Al Viro Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Thu, Jul 15, 2021 at 05:35:51PM +0700, Dmitry Kadashev wrote: > This is just a preparation for the move of the main mkdirat logic to a > separate function to make the logic easier to follow. This change > contains the flow changes so that the actual change to move the main > logic to a separate function does no change the flow at all. > > Just like the similar patches for rmdir and unlink a few commits before, > there two changes here: > > 1. Previously on filename_create() error the function used to exit > immediately, and now it will check the return code to see if ESTALE > retry is appropriate. The filename_create() does its own retries on > ESTALE (at least via filename_parentat() used inside), but this extra > check should be completely fine. This is the wrong way to go. Really. Look at it that way - LOOKUP_REVAL is the final stage of escalation; if we had to go there, there's no point being optimistic about the last dcache lookup, nevermind trying to retry the parent pathwalk if we fail with -ESTALE doing it. I'm not saying that it's something worth optimizing for; the problem is different - the logics makes no sense whatsoever that way. It's a matter of reader's cycles wasted on "what the fuck are we trying to do here?", not the CPU cycles wasted on execution. While we are at it, it makes no sense for filename_parentat() and its ilk to go for RCU and normal if it's been given LOOKUP_REVAL - I mean, look at the sequence of calls in there. And try to make sense of it. Especially of the "OK, RCU attempt told us to sod off and try normal; here, let's call path_parentat() with LOOKUP_REVAL for flags and if it says -ESTALE, call it again with exact same arguments" part. Seriously, look at that from the point of view of somebody who tries to make sense of the entire thing.