Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Date:   Mon, 9 Oct 2023 19:35:12 -0700
From:   Krister Johansen <kjlx@templeofstupid.com>
To:     Bernd Schubert <bernd.schubert@fastmail.fm>
Cc:     Krister Johansen <kjlx@templeofstupid.com>,
        Miklos Szeredi <miklos@szeredi.hu>,
        linux-fsdevel@vger.kernel.org,
        Miklos Szeredi <mszeredi@redhat.com>,
        linux-kernel@vger.kernel.org,
        German Maglione <gmaglione@redhat.com>,
        Greg Kurz <groug@kaod.org>, Max Reitz <mreitz@redhat.com>
Subject: Re: [resend PATCH v2 2/2] fuse: ensure that submounts lookup their
 parent
Message-ID: <20231010023512.GB1983@templeofstupid.com>
References: <cover.1696043833.git.kjlx@templeofstupid.com>
 <45778432fba32dce1fb1f5fd13272c89c95c3f52.1696043833.git.kjlx@templeofstupid.com>
 <3187f942-dcf0-4b2f-a106-0eb5d5a33949@fastmail.fm>
 <20231007004107.GA1967@templeofstupid.com>
 <968148ad-787e-4ccb-9d84-f32b5da88517@fastmail.fm>
 <20231009171525.GA1973@templeofstupid.com>
 <f5a431f8-fad9-4b1b-a3ae-86b6cff65b9b@fastmail.fm>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f5a431f8-fad9-4b1b-a3ae-86b6cff65b9b@fastmail.fm>
Precedence: bulk

On Mon, Oct 09, 2023 at 08:43:02PM +0200, Bernd Schubert wrote:
> On 10/9/23 19:15, Krister Johansen wrote:
> > Thanks, I had forgotten that d_make_root() would call iput() for me if
> > d_alloc_anon() fails.  Let me restate this to suggest that I account the
> > nlookup to the parent if fuse_dentry_revalidate_lookup() or fuse_iget()
> > fail instead.  Does that sound right?
> 
> Hmm, so server/daemon side uses the lookup count to have an inode reference
> - are you sure that parent is the right inode for the forget call? And what
> is the probability for such failures? I.e. is that performance critical?
> Wouldn't be much simpler and clearer to just avoid and doubt and to send an
> immediate forget?

Yeah, the server / daemon side need to track the lookup count so that it
knows when it can close the fd for the file on the server-side. (At
least for virtiofsd, anyway.).

The reason I had avoided doing the forget in the submount code is that
it needs a forget linkage in order to call fuse_queue_forget().  One of
these is allocated by fuse_alloc_inode().  A well formed parent should
always have one.  However, if the fuse_iget() for the submount root
fails, then there's no linkage to borrow from the new inode.  The code
could always call fuse_alloc_forget() directly, like is done elsewhere.
I thought it might be hard to get the memory for this allocation if
fuse_iget() also can't allocate enough, but I could move the allocation
earlier in the function and just free it if it's not used.

I'm not confident that would reduce the amount of code in the function,
but if you'd find it clearer, I'm happy to modify it accordingly.

-K