| Message ID | 20190507142428.6531-1-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Add comments on locking of several device-related fields | expand |
On Tue, May 07, 2019 at 05:24:28PM +0300, Nikolay Borisov wrote: > Signed-off-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/volumes.h | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 3b97e8092ba7..514799362244 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -52,8 +52,8 @@ struct btrfs_io_geometry { > #define BTRFS_DEV_STATE_FLUSH_SENT (4) > > struct btrfs_device { > - struct list_head dev_list; > - struct list_head dev_alloc_list; > + struct list_head dev_list; /* device_list_mutex */ > + struct list_head dev_alloc_list; /* chunk mutex */ > struct list_head post_commit_list; /* chunk mutex */ Please update the documentation in the comment 'Device locking' in volumes.c
On 9.05.19 г. 16:55 ч., David Sterba wrote: > On Tue, May 07, 2019 at 05:24:28PM +0300, Nikolay Borisov wrote: >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> >> --- >> fs/btrfs/volumes.h | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h >> index 3b97e8092ba7..514799362244 100644 >> --- a/fs/btrfs/volumes.h >> +++ b/fs/btrfs/volumes.h >> @@ -52,8 +52,8 @@ struct btrfs_io_geometry { >> #define BTRFS_DEV_STATE_FLUSH_SENT (4) >> >> struct btrfs_device { >> - struct list_head dev_list; >> - struct list_head dev_alloc_list; >> + struct list_head dev_list; /* device_list_mutex */ >> + struct list_head dev_alloc_list; /* chunk mutex */ >> struct list_head post_commit_list; /* chunk mutex */ > > Please update the documentation in the comment 'Device locking' in > volumes.c > Right the only thing that is missing from the Device Locking comment is the mention of post_commit_list. However, dev_list and dev_alloc_list are essentially "the other side" of btrfs_fs_devices::alloc_list hence I've added them as inline comments. I think it makes more sense to those small comments next to the variables at least as a remainder.
On Thu, May 09, 2019 at 05:12:16PM +0300, Nikolay Borisov wrote: > > > On 9.05.19 г. 16:55 ч., David Sterba wrote: > > On Tue, May 07, 2019 at 05:24:28PM +0300, Nikolay Borisov wrote: > >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> > >> --- > >> fs/btrfs/volumes.h | 11 ++++++++--- > >> 1 file changed, 8 insertions(+), 3 deletions(-) > >> > >> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > >> index 3b97e8092ba7..514799362244 100644 > >> --- a/fs/btrfs/volumes.h > >> +++ b/fs/btrfs/volumes.h > >> @@ -52,8 +52,8 @@ struct btrfs_io_geometry { > >> #define BTRFS_DEV_STATE_FLUSH_SENT (4) > >> > >> struct btrfs_device { > >> - struct list_head dev_list; > >> - struct list_head dev_alloc_list; > >> + struct list_head dev_list; /* device_list_mutex */ > >> + struct list_head dev_alloc_list; /* chunk mutex */ > >> struct list_head post_commit_list; /* chunk mutex */ > > > > Please update the documentation in the comment 'Device locking' in > > volumes.c > > Right the only thing that is missing from the Device Locking comment is > the mention of post_commit_list. However, dev_list and dev_alloc_list > are essentially "the other side" of btrfs_fs_devices::alloc_list hence > I've added them as inline comments. I think it makes more sense to those > small comments next to the variables at least as a remainder. Ok.
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 3b97e8092ba7..514799362244 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -52,8 +52,8 @@ struct btrfs_io_geometry { #define BTRFS_DEV_STATE_FLUSH_SENT (4) struct btrfs_device { - struct list_head dev_list; - struct list_head dev_alloc_list; + struct list_head dev_list; /* device_list_mutex */ + struct list_head dev_alloc_list; /* chunk mutex */ struct list_head post_commit_list; /* chunk mutex */ struct btrfs_fs_devices *fs_devices; struct btrfs_fs_info *fs_info; @@ -238,9 +238,14 @@ struct btrfs_fs_devices { * this mutex lock. */ struct mutex device_list_mutex; + + /* List of all devices, protected by device_list_mutex */ struct list_head devices; - /* devices not currently being allocated */ + /* + * Devices which can satisfy space allocation. Protected by + * chunk_mutex + */ struct list_head alloc_list; struct btrfs_fs_devices *seed;
Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/volumes.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)