linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Nikolay Borisov <nborisov@suse.com>
Cc: dsterba@suse.cz, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 0/5] Convert seed devices to proper list API
Date: Fri, 21 Aug 2020 16:33:18 +0200	[thread overview]
Message-ID: <20200821143318.GF2026@twin.jikos.cz> (raw)
In-Reply-To: <dc2379cf-9e8f-031e-4214-d68f6e4d41b1@suse.com>

On Thu, Jul 23, 2020 at 11:02:14AM +0300, Nikolay Borisov wrote:
> Regarding patch 5 - I don't know what made you think there is a
> tree-like structure involved. Simply looking at the old way seeds are
> iterated:
> 
> 	while (seed_devices) {
> 		fs_devices = seed_devices;
> 		seed_devices = fs_devices->seed;
>  		close_fs_devices(fs_devices);
>  		free_fs_devices(fs_devices);
>  	}
> 
> There's no conditional deciding if we should go left/right of the tree.

A tree structure that has only one direction arrows, from leaves to the
root. Where the root is the seed fs and the leaves are the sprouts.

	 fs1 ---> seed
		  ^ ^
	 fs2 -----| |
                    |
	 fs3 -------|

The while loop in each fs goes by the pointers up to the seeding device
and always removes it's references.

I'm not sure about a deeper tree structure here, so the fs3 -> fs2 would
be possible.

> Or let's take for example deleting from a list of seed devices:
> 
> 		tmp_fs_devices = fs_info->fs_devices;
> 		while (tmp_fs_devices) {
> 			if (tmp_fs_devices->seed == fs_devices) {
> 				tmp_fs_devices->seed = fs_devices->seed;
> 				break;
> 			}
> 			tmp_fs_devices = tmp_fs_devices->seed;
> 		}
> 
> Here a simple linear search is performed and when a member of the seed
> list matches our fs_devices it simply pointed 1 past our fs_devices
> 
> When the if inside the loop matches we get the following situation:
> 
> [tmp_fs_devices]->[fs_devices]->[fs_devices->seeds]
> 
> Then we perform [tmp_fs_devices] -> [fs_devices->seed]
> 
> So a simple deletion, just the fact you were confused shows the old code
> is rather wonky.

Well, I still am after reading the above, the missing part is about the
device cloning and that each fs has it's own copy. The tree structure
implies sharing and would need reference counting, but this does not
seem to be so.

I'll add the series to for-next so we have a base for the other seeding
patches but I haven't reviewed it to the point where I'm convinced it's
correct.

  reply	other threads:[~2020-08-21 14:34 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-15 10:48 [PATCH 0/5] Convert seed devices to proper list API Nikolay Borisov
2020-07-15 10:48 ` [PATCH 1/5] btrfs: Factor out reada loop in __reada_start_machine Nikolay Borisov
2020-08-18 15:02   ` Josef Bacik
2020-08-29 15:06   ` Anand Jain
2020-08-31 12:24     ` David Sterba
2020-07-15 10:48 ` [PATCH 2/5] btrfs: Factor out loop logic from btrfs_free_extra_devids Nikolay Borisov
2020-07-15 12:32   ` kernel test robot
2020-07-15 12:39     ` Nikolay Borisov
2020-07-16  7:17   ` [PATCH v2] " Nikolay Borisov
2020-08-29 15:13     ` Anand Jain
2020-08-18 15:03   ` [PATCH 2/5] " Josef Bacik
2020-07-15 10:48 ` [PATCH 3/5] btrfs: Make close_fs_devices return void Nikolay Borisov
2020-08-18 15:05   ` Josef Bacik
2020-08-29 15:14   ` Anand Jain
2020-07-15 10:48 ` [PATCH 4/5] btrfs: Simplify setting/clearing fs_info to btrfs_fs_devices Nikolay Borisov
2020-08-18 15:08   ` Josef Bacik
2020-08-26 10:50   ` Anand Jain
2020-07-15 10:48 ` [PATCH 5/5] btrfs: Switch seed device to list api Nikolay Borisov
2020-07-15 13:14   ` kernel test robot
2020-07-16  7:25   ` [PATCH v2] " Nikolay Borisov
2020-08-18 15:19     ` Josef Bacik
2020-08-30 14:39     ` Anand Jain
2020-07-24  7:36   ` [PATCH 5/5] " Nikolay Borisov
2020-09-02 15:58   ` Anand Jain
2020-09-03  9:03     ` Nikolay Borisov
2020-09-03  9:33       ` Anand Jain
2020-09-10 16:28         ` David Sterba
2020-07-22 14:26 ` [PATCH 0/5] Convert seed devices to proper list API David Sterba
2020-07-23  8:02   ` Nikolay Borisov
2020-08-21 14:33     ` David Sterba [this message]
2020-08-17 19:19 ` Nikolay Borisov

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=20200821143318.GF2026@twin.jikos.cz \
    --to=dsterba@suse.cz \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=nborisov@suse.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;
as well as URLs for NNTP newsgroup(s).