Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Tejun Heo <htejun@gmail.com>
Date:   Mon, 28 Mar 2022 07:30:02 -1000
From:   Tejun Heo <tj@kernel.org>
To:     Imran Khan <imran.f.khan@oracle.com>
Cc:     Al Viro <viro@zeniv.linux.org.uk>, gregkh@linuxfoundation.org,
        linux-kernel@vger.kernel.org
Subject: Re: [RESEND PATCH v7 7/8] kernfs: Replace per-fs rwsem with hashed
 rwsems.
Message-ID: <YkHwmhOaSHuXPemP@slm.duckdns.org>
References: <20220317072612.163143-1-imran.f.khan@oracle.com>
 <20220317072612.163143-8-imran.f.khan@oracle.com>
 <YjPNOQJf/Wxa4YeV@zeniv-ca.linux.org.uk>
 <536f2392-45d2-2f43-5e9d-01ef50e33126@oracle.com>
 <YjgpaeFfFandY999@zeniv-ca.linux.org.uk>
 <Yjir/d5S3J1PTiux@slm.duckdns.org>
 <Yji8KT2K7ZKOQ+6S@zeniv-ca.linux.org.uk>
 <YjjP5ldCCGYqD+UV@slm.duckdns.org>
 <Yjk3Nqft/U6vDvd1@zeniv-ca.linux.org.uk>
 <a692a5af-158e-ad0c-63a1-6525ee6d3898@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a692a5af-158e-ad0c-63a1-6525ee6d3898@oracle.com>
Precedence: bulk

On Mon, Mar 28, 2022 at 11:15:30AM +1100, Imran Khan wrote:
> Hello Al,
> 
> On 22/3/22 1:40 pm, Al Viro wrote:
> [...]
> > 
> > Sorry, misread that thing - the reason it copies the damn thing at all is
> > the use of strsep().  Yecch...  Rule of the thumb regarding strsep() use,
> > be it in kernel or in the userland: don't.  It's almost never the right
> > primitive to use.
> > 
> > Lookups should use qstr; it has both the length and place for hash.
> > Switch kernfs_find_ns() to that (and lift the calculation of length
> > into the callers that do not have it - note that kernfs_iop_lookup()
> > does) and you don't need the strsep() shite (or copying) anymore.
> > 
> 
> Regarding using qstr in kernfs_find_ns, do you mean that I should remove
> ->name and ->hash with a ->qstr in kernfs_node and further modify
> kernfs_name_compare to make use of qstr.name and qstr.hash.
> 
> Also the suggestion about removing buffer copying from kernfs_walk_ns is
> not clear to me because kernfs_walk_ns invokes kernfs_find_ns with
> individual path components so we need some mechanism to separate path
> components.
> 
> Sorry if I have missed or misunderstood something in your suggestion.

qstr encodes the lenght of the string and doesn't need the terminating '\0',
so we can just walk qstr over the passed in path instead of copying and
overwriting '\0'. This is rather separate and we can revisit this later tho.

Thanks.

-- 
tejun