| Message ID | 20181004212443.26519-2-hans.van.kranenburg@mendix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Chunk allocator DUP fix and cleanups | expand |
On 5.10.2018 00:24, Hans van Kranenburg wrote: > num_bytes is used to store the chunk length of the chunk that we're > allocating. Do not reuse it for something really different in the same > function. > > Signed-off-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com> nit: That's a minor cleanup and brings no functional changes. I think it even allows to give a more descriptive name of num_bytes such as chunk_size, especially since we have a max_chunk_size. I think they would pair nicely. Anyway, Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/volumes.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index f4405e430da6..9c9bb127eeee 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -4837,8 +4837,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, > goto error_del_extent; > > for (i = 0; i < map->num_stripes; i++) { > - num_bytes = map->stripes[i].dev->bytes_used + stripe_size; > - btrfs_device_set_bytes_used(map->stripes[i].dev, num_bytes); > + btrfs_device_set_bytes_used(map->stripes[i].dev, > + map->stripes[i].dev->bytes_used + > + stripe_size); > } > > atomic64_sub(stripe_size * map->num_stripes, &info->free_chunk_space); >
On 5.10.2018 11:59, Nikolay Borisov wrote: > > > On 5.10.2018 00:24, Hans van Kranenburg wrote: >> num_bytes is used to store the chunk length of the chunk that we're >> allocating. Do not reuse it for something really different in the same >> function. >> >> Signed-off-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com> > > nit: That's a minor cleanup and brings no functional changes. I think it > even allows to give a more descriptive name of num_bytes such as > chunk_size, especially since we have a max_chunk_size. I think they > would pair nicely. I saw that your patch 2 actually does that, so ignore this comment :) > > Anyway, > > Reviewed-by: Nikolay Borisov <nborisov@suse.com> > >> --- >> fs/btrfs/volumes.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index f4405e430da6..9c9bb127eeee 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -4837,8 +4837,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, >> goto error_del_extent; >> >> for (i = 0; i < map->num_stripes; i++) { >> - num_bytes = map->stripes[i].dev->bytes_used + stripe_size; >> - btrfs_device_set_bytes_used(map->stripes[i].dev, num_bytes); >> + btrfs_device_set_bytes_used(map->stripes[i].dev, >> + map->stripes[i].dev->bytes_used + >> + stripe_size); >> } >> >> atomic64_sub(stripe_size * map->num_stripes, &info->free_chunk_space); >>
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index f4405e430da6..9c9bb127eeee 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -4837,8 +4837,9 @@ static int __btrfs_alloc_chunk(struct btrfs_trans_handle *trans, goto error_del_extent; for (i = 0; i < map->num_stripes; i++) { - num_bytes = map->stripes[i].dev->bytes_used + stripe_size; - btrfs_device_set_bytes_used(map->stripes[i].dev, num_bytes); + btrfs_device_set_bytes_used(map->stripes[i].dev, + map->stripes[i].dev->bytes_used + + stripe_size); } atomic64_sub(stripe_size * map->num_stripes, &info->free_chunk_space);
num_bytes is used to store the chunk length of the chunk that we're allocating. Do not reuse it for something really different in the same function. Signed-off-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com> --- fs/btrfs/volumes.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)