On Wed, Apr 17, 2019 at 09:22:44AM +0000, Filipe Manana wrote: > On Wed, Apr 17, 2019 at 1:08 AM Zygo Blaxell > <ce3g8jdj@xxxxxxxxxxxxxxxxxxxxx> wrote: > > > > On Mon, Apr 15, 2019 at 09:29:00AM +0100, fdmanana@xxxxxxxxxx wrote: > > > From: Filipe Manana <fdmanana@xxxxxxxx> > > > > > > During fiemap, for regular extents (non inline) we need to check if they > > > are shared and if they are, set the shared bit. Checking if an extent is > > > shared requires checking the delayed references of the currently running > > > transaction, since some reference might have not yet hit the extent tree > > > and be only in the in-memory delayed references. > > > > > > However we were using a transaction join for this, which creates a new > > > transaction when there is no transaction currently running. That means > > > that two more potential failures can happen: creating the transaction and > > > committing it. Further, if no write activity is currently happening in the > > > system, and fiemap calls keep being done, we end up creating and > > > committing transactions that do nothing. > > > > Cool! > > > > Any chance we can do this for the LOGICAL_INO ioctl? Last time I checked > > (I admit that was a while ago), LOGICAL_INO cost about the same as > > fsync(), because it creates transactions with btrfs_join_transaction a > > few levels deep in the call stack, and gets blocked waiting for them > > to complete. > > So iterate_extent_inodes() should definitly use the attach API and not join for > the same reasons. And I can fix that separately. > > > > > It looks like btrfs_ioctl_logical_to_ino can set: > > > > path->search_commit_root = 1; > > > > just before calling iterate_inodes_from_logical, but I tried that once, > > and my test filesystem blew up a few days later, so there might be > > something subtle that I missed. Or maybe my test filesystem was going > > to blow up that day anyway--I just assumed that I don't know what I'm > > doing, and didn't repeat the test. > > Blew up? That's a very vague term... What do you mean with it? What > happened exactly? > > Setting search_commit_root should not cause any problems, the only > thing that can happen is giving you non-accurate results > because there might be relevant changes in the current transaction and > therefore they are not visible yet from the commit roots. If there's need to have both, ie. the accurate and non-accurate version, the v2 of the ioctl accepts flags so it could be done.
