mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
To: "slava@dubeyko.com" <slava@dubeyko.com>,
	David Howells <dhowells@redhat.com>
Cc: Xiubo Li <xiubli@redhat.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>,
	"brauner@kernel.org" <brauner@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Alex Markuze <amarkuze@redhat.com>,
	"jlayton@kernel.org" <jlayton@kernel.org>,
	"idryomov@gmail.com" <idryomov@gmail.com>,
	"viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>
Subject: RE: Does ceph_fill_inode() mishandle I_NEW?
Date: Thu, 13 Mar 2025 21:46:54 +0000	[thread overview]
Message-ID: <afeb9082273098f47b26371a7e252381d1268c8e.camel@ibm.com> (raw)
In-Reply-To: <1468676.1741898867@warthog.procyon.org.uk>

On Thu, 2025-03-13 at 20:47 +0000, David Howells wrote:
> slava@dubeyko.com wrote:
> 
> > What do you mean by mishandling? Do you imply that Ceph has to set up
> > the I_NEW somehow? Is it not VFS responsibility?
> 
> No - I mean that if I_NEW *isn't* set when the function is called,
> ceph_fill_inode() will go and partially reinitialise the inode.  Now, having
> reviewed the code in more depth and talked to Jeff Layton about it, I think
> that the non-I_NEW pass will only change pointers with some sort of locking
> and will release the old target - though it may overwrite some pointers with
> the same value without protection (i_fops for example).
> 
> That said, if it's possible for *two* processes to be going through that
> function without I_NEW set, you can get places where both of them will try
> freeing the old data and replacing it with new without any locking - but I
> don't know if that can happen.
> 

I see your point now.

As far as I can see, ceph_fill_inode() has comment: "Populate an inode based on
info from mds. May be called on new or existing inodes". It sounds to me that
particular CephFS kernel client could have obsolete state of inode compared with
MDS's state. And we need to "re-new" the existing inode with the actual state
that we received from MDS side. My vision is that we need to take into account
the distributed nature of Ceph and inode metadata can be updated from multiple
CephFS kernel client instances. Am I right here?

Thanks,
Slava.


  reply	other threads:[~2025-03-13 21:47 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-13 10:17 David Howells
2025-03-13 19:14 ` slava
2025-03-13 20:47 ` David Howells
2025-03-13 21:46   ` Viacheslav Dubeyko [this message]
2025-03-13 22:47   ` Jeff Layton
2025-03-13 23:37   ` David Howells
2025-03-13 23:47   ` David Howells

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=afeb9082273098f47b26371a7e252381d1268c8e.camel@ibm.com \
    --to=slava.dubeyko@ibm.com \
    --cc=amarkuze@redhat.com \
    --cc=brauner@kernel.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=dhowells@redhat.com \
    --cc=idryomov@gmail.com \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=slava@dubeyko.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xiubli@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

all inboxes | Powered by JetHome®