Re: [PATCH v2] BTRFS/NFSD: provide more unique inode number for btrfs export

From: "NeilBrown" <neilb@suse.de>
To: "Christoph Hellwig" <hch@infradead.org>
Cc: "Christoph Hellwig" <hch@infradead.org>,
	"J. Bruce Fields" <bfields@fieldses.org>,
	"Chuck Lever" <chuck.lever@oracle.com>,
	linux-nfs@vger.kernel.org, "Josef Bacik" <josef@toxicpanda.com>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2] BTRFS/NFSD: provide more unique inode number for btrfs export
Date: Thu, 02 Sep 2021 14:06:54 +1000	[thread overview]
Message-ID: <163055561473.24419.12486186372497472066@noble.neil.brown.name> (raw)
In-Reply-To: <YS8ppl6SYsCC0cql@infradead.org>

On Wed, 01 Sep 2021, Christoph Hellwig wrote:
> On Tue, Aug 31, 2021 at 02:59:05PM +1000, NeilBrown wrote:
> > Making the change purely in btrfs is simply not possible.  There is no
> > way for btrfs to provide nfsd with a different inode number.  To move
> > the bulk of the change into btrfs code we would need - at the very least
> > - some way for nfsd to provide the filehandle when requesting stat
> > information.  We would also need to provide a reference filehandle when
> > requesting a dentry->filehandle conversion.  Cluttering the
> > export_operations like that just for btrfs doesn't seem like the right
> > balance.  I agree that cluttering kstat is not ideal, but it was a case
> > of choosing the minimum change for the maximum effect.
> 
> So you're papering over a btrfs bug by piling up cludges in the nsdd
> code that has not business even knowing about this btrfs bug, while
> leaving other users of inodes numbers and file handles broken?
> 
> If you only care about file handles:  this is what the export operations
> are for.  If you care about inode numbers:  well, it is up to btrfs
> to generate uniqueue inode numbers.  It currently doesn't do that, and
> no amount of papering over that in nfsd is going to fix the issue.
> 
> If XORing a little more entropy into the inode number is a good enough
> band aid (and I strongly disagree with that), do it inside btrfs for
> every place they report the inode number.  There is nothing NFS-specific
> about that.
> 

Hi Christoph,
 I have to say that I struggle with some of these conversations with
 you.
 I don't know if it is deliberate on your part, or inadvertent, or
 purely in my imagination, but your attitude often seems combative.  I
 find that to be a disincentive to continuing to engage, which I need to
 work hard to overcome.  If I'm misunderstanding you, I apologise and
 simply ask that you do what you can to compensate for my apparent
 sensitivity.

 Your attitude seems to be that this is a btrfs problem and must be
 fixed in btrfs.  I agree about the source of the problem - specifically:
  Commit 3394e1607eaf ("Btrfs: Give each subvol and snapshot their own anonymous devid")
 took a wrong turn.  But I don't think we can completely isolate any
 part of the kernel, and we need to work together to solve problems that
 affect us, no matter the cause.  Similarly our code needs to work
 together.

 Highlighting the various problems with the proposed solution doesn't
 help a lot - they are fairly obvious.  Proposing solutions would be
 much more helpful, and I have no doubt that your different experience
 and perspective could help me see things that I have missed.   Any help
 that you can provide would certainly be appreciated.

Thanks,
NeilBrown