public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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
Date: Mon, 9 Oct 2023 19:35:12 -0700	[thread overview]
Message-ID: <20231010023512.GB1983@templeofstupid.com> (raw)
In-Reply-To: <f5a431f8-fad9-4b1b-a3ae-86b6cff65b9b@fastmail.fm>

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

  reply	other threads:[~2023-10-10  2:35 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-02 15:24 [resend PATCH v2 0/2] virtiofs submounts that are still in use forgotten by shrinker Krister Johansen
2023-10-02 15:24 ` [resend PATCH v2 1/2] fuse: revalidate: move lookup into a separate function Krister Johansen
2023-10-02 15:24 ` [resend PATCH v2 2/2] fuse: ensure that submounts lookup their parent Krister Johansen
2023-10-06 17:13   ` Bernd Schubert
2023-10-07  0:41     ` Krister Johansen
2023-10-09 12:52       ` Bernd Schubert
2023-10-09 17:15         ` Krister Johansen
2023-10-09 18:43           ` Bernd Schubert
2023-10-10  2:35             ` Krister Johansen [this message]
2023-10-09 19:45   ` Miklos Szeredi
2023-10-10  2:35     ` Krister Johansen
2023-10-10  8:15       ` Miklos Szeredi
2023-10-11  1:25         ` Krister Johansen
2023-10-11  7:07           ` Miklos Szeredi
2023-10-11 16:32             ` Krister Johansen
2023-10-11 18:27               ` Miklos Szeredi
2023-10-18  1:33                 ` Krister Johansen
2023-10-18  1:33                 ` [PATCH v3] fuse: share lookup state between submount and its parent Krister Johansen
2023-10-19 12:39                   ` Miklos Szeredi
2023-10-20 21:33                     ` Krister Johansen
2023-10-20 21:34                     ` [PATCH v4] " Krister Johansen
2023-10-02 22:18 ` [resend PATCH v2 0/2] virtiofs submounts that are still in use forgotten by shrinker Bernd Schubert
2023-10-03 16:48   ` Krister Johansen
2023-10-03 22:54     ` Bernd Schubert
2023-10-04 13:58       ` Krister Johansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231010023512.GB1983@templeofstupid.com \
    --to=kjlx@templeofstupid.com \
    --cc=bernd.schubert@fastmail.fm \
    --cc=gmaglione@redhat.com \
    --cc=groug@kaod.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=mreitz@redhat.com \
    --cc=mszeredi@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox