* [PATCH] btrfs: extent-tree: Use round up to replace align macro
@ 2017-11-21 5:54 Qu Wenruo
2017-11-21 7:33 ` Nikolay Borisov
2017-11-21 13:36 ` David Sterba
0 siblings, 2 replies; 6+ messages in thread
From: Qu Wenruo @ 2017-11-21 5:54 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
To save reader seconds before checking if it's rounding up or down.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/extent-tree.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 309a109069f1..8ab18e25cb91 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2054,7 +2054,7 @@ static int btrfs_issue_discard(struct block_device *bdev, u64 start, u64 len,
{
int j, ret = 0;
u64 bytes_left, end;
- u64 aligned_start = ALIGN(start, 1 << 9);
+ u64 aligned_start = round_up(start, 1 << 9);
if (WARN_ON(start != aligned_start)) {
len -= aligned_start - start;
@@ -4266,7 +4266,7 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes)
int have_pinned_space;
/* make sure bytes are sectorsize aligned */
- bytes = ALIGN(bytes, fs_info->sectorsize);
+ bytes = round_up(bytes, fs_info->sectorsize);
if (btrfs_is_free_space_inode(inode)) {
need_commit = 0;
@@ -6080,7 +6080,7 @@ int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes)
if (delalloc_lock)
mutex_lock(&inode->delalloc_mutex);
- num_bytes = ALIGN(num_bytes, fs_info->sectorsize);
+ num_bytes = round_up(num_bytes, fs_info->sectorsize);
/* Add our new extents and calculate the new rsv size. */
spin_lock(&inode->lock);
@@ -6135,7 +6135,7 @@ void btrfs_delalloc_release_metadata(struct btrfs_inode *inode, u64 num_bytes)
{
struct btrfs_fs_info *fs_info = btrfs_sb(inode->vfs_inode.i_sb);
- num_bytes = ALIGN(num_bytes, fs_info->sectorsize);
+ num_bytes = round_up(num_bytes, fs_info->sectorsize);
spin_lock(&inode->lock);
inode->csum_bytes -= num_bytes;
btrfs_calculate_inode_block_rsv_size(fs_info, inode);
@@ -7818,7 +7818,7 @@ static noinline int find_free_extent(struct btrfs_fs_info *fs_info,
goto loop;
}
checks:
- search_start = ALIGN(offset, fs_info->stripesize);
+ search_start = round_up(offset, fs_info->stripesize);
/* move on to the next group */
if (search_start + num_bytes >
--
2.15.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] btrfs: extent-tree: Use round up to replace align macro
2017-11-21 5:54 [PATCH] btrfs: extent-tree: Use round up to replace align macro Qu Wenruo
@ 2017-11-21 7:33 ` Nikolay Borisov
2017-11-21 8:06 ` Qu Wenruo
2017-11-21 13:36 ` David Sterba
1 sibling, 1 reply; 6+ messages in thread
From: Nikolay Borisov @ 2017-11-21 7:33 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: dsterba
On 21.11.2017 07:54, Qu Wenruo wrote:
> To save reader seconds before checking if it's rounding up or down.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/extent-tree.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 309a109069f1..8ab18e25cb91 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2054,7 +2054,7 @@ static int btrfs_issue_discard(struct block_device *bdev, u64 start, u64 len,
> {
> int j, ret = 0;
> u64 bytes_left, end;
> - u64 aligned_start = ALIGN(start, 1 << 9);
> + u64 aligned_start = round_up(start, 1 << 9);
nit: 1 << 9 == SZ_512 or essentially sector size however looking at the
defines there is SECTOR_SIZE defined in ide.h which we do not include.
Or just use 512 no need to obscure it behind a shift.
>
> if (WARN_ON(start != aligned_start)) {
> len -= aligned_start - start;
> @@ -4266,7 +4266,7 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes)
> int have_pinned_space;
>
> /* make sure bytes are sectorsize aligned */
> - bytes = ALIGN(bytes, fs_info->sectorsize);
> + bytes = round_up(bytes, fs_info->sectorsize);
>
> if (btrfs_is_free_space_inode(inode)) {
> need_commit = 0;
> @@ -6080,7 +6080,7 @@ int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes)
> if (delalloc_lock)
> mutex_lock(&inode->delalloc_mutex);
>
> - num_bytes = ALIGN(num_bytes, fs_info->sectorsize);
> + num_bytes = round_up(num_bytes, fs_info->sectorsize);
>
> /* Add our new extents and calculate the new rsv size. */
> spin_lock(&inode->lock);
> @@ -6135,7 +6135,7 @@ void btrfs_delalloc_release_metadata(struct btrfs_inode *inode, u64 num_bytes)
> {
> struct btrfs_fs_info *fs_info = btrfs_sb(inode->vfs_inode.i_sb);
>
> - num_bytes = ALIGN(num_bytes, fs_info->sectorsize);
> + num_bytes = round_up(num_bytes, fs_info->sectorsize);
> spin_lock(&inode->lock);
> inode->csum_bytes -= num_bytes;
> btrfs_calculate_inode_block_rsv_size(fs_info, inode);
> @@ -7818,7 +7818,7 @@ static noinline int find_free_extent(struct btrfs_fs_info *fs_info,
> goto loop;
> }
> checks:
> - search_start = ALIGN(offset, fs_info->stripesize);
> + search_start = round_up(offset, fs_info->stripesize);
>
> /* move on to the next group */
> if (search_start + num_bytes >
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] btrfs: extent-tree: Use round up to replace align macro
2017-11-21 7:33 ` Nikolay Borisov
@ 2017-11-21 8:06 ` Qu Wenruo
2017-11-21 13:47 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2017-11-21 8:06 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs; +Cc: dsterba
[-- Attachment #1.1: Type: text/plain, Size: 2873 bytes --]
On 2017年11月21日 15:33, Nikolay Borisov wrote:
>
>
> On 21.11.2017 07:54, Qu Wenruo wrote:
>> To save reader seconds before checking if it's rounding up or down.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/extent-tree.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 309a109069f1..8ab18e25cb91 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -2054,7 +2054,7 @@ static int btrfs_issue_discard(struct block_device *bdev, u64 start, u64 len,
>> {
>> int j, ret = 0;
>> u64 bytes_left, end;
>> - u64 aligned_start = ALIGN(start, 1 << 9);
>> + u64 aligned_start = round_up(start, 1 << 9);
>
> nit: 1 << 9 == SZ_512 or essentially sector size however looking at the
> defines there is SECTOR_SIZE defined in ide.h which we do not include.
> Or just use 512 no need to obscure it behind a shift.
I also noticed that while I don't have any good idea which macro should
be used.
That SECTOR_SIZE defined in ide.h doesn't sound good to me.
And all other filesystems are using their own definition, without some
definition in fs/bio level.
Thanks,
Qu
>
>>
>> if (WARN_ON(start != aligned_start)) {
>> len -= aligned_start - start;
>> @@ -4266,7 +4266,7 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes)
>> int have_pinned_space;
>>
>> /* make sure bytes are sectorsize aligned */
>> - bytes = ALIGN(bytes, fs_info->sectorsize);
>> + bytes = round_up(bytes, fs_info->sectorsize);
>>
>> if (btrfs_is_free_space_inode(inode)) {
>> need_commit = 0;
>> @@ -6080,7 +6080,7 @@ int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes)
>> if (delalloc_lock)
>> mutex_lock(&inode->delalloc_mutex);
>>
>> - num_bytes = ALIGN(num_bytes, fs_info->sectorsize);
>> + num_bytes = round_up(num_bytes, fs_info->sectorsize);
>>
>> /* Add our new extents and calculate the new rsv size. */
>> spin_lock(&inode->lock);
>> @@ -6135,7 +6135,7 @@ void btrfs_delalloc_release_metadata(struct btrfs_inode *inode, u64 num_bytes)
>> {
>> struct btrfs_fs_info *fs_info = btrfs_sb(inode->vfs_inode.i_sb);
>>
>> - num_bytes = ALIGN(num_bytes, fs_info->sectorsize);
>> + num_bytes = round_up(num_bytes, fs_info->sectorsize);
>> spin_lock(&inode->lock);
>> inode->csum_bytes -= num_bytes;
>> btrfs_calculate_inode_block_rsv_size(fs_info, inode);
>> @@ -7818,7 +7818,7 @@ static noinline int find_free_extent(struct btrfs_fs_info *fs_info,
>> goto loop;
>> }
>> checks:
>> - search_start = ALIGN(offset, fs_info->stripesize);
>> + search_start = round_up(offset, fs_info->stripesize);
>>
>> /* move on to the next group */
>> if (search_start + num_bytes >
>>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] btrfs: extent-tree: Use round up to replace align macro
2017-11-21 8:06 ` Qu Wenruo
@ 2017-11-21 13:47 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2017-11-21 13:47 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Nikolay Borisov, linux-btrfs, dsterba
On Tue, Nov 21, 2017 at 04:06:54PM +0800, Qu Wenruo wrote:
> On 2017年11月21日 15:33, Nikolay Borisov wrote:
> > On 21.11.2017 07:54, Qu Wenruo wrote:
> >> @@ -2054,7 +2054,7 @@ static int btrfs_issue_discard(struct block_device *bdev, u64 start, u64 len,
> >> {
> >> int j, ret = 0;
> >> u64 bytes_left, end;
> >> - u64 aligned_start = ALIGN(start, 1 << 9);
> >> + u64 aligned_start = round_up(start, 1 << 9);
> >
> > nit: 1 << 9 == SZ_512 or essentially sector size however looking at the
> > defines there is SECTOR_SIZE defined in ide.h which we do not include.
> > Or just use 512 no need to obscure it behind a shift.
>
> I also noticed that while I don't have any good idea which macro should
> be used.
>
> That SECTOR_SIZE defined in ide.h doesn't sound good to me.
Heh, no we won't include ide.h just to get the macro definition.
> And all other filesystems are using their own definition, without some
> definition in fs/bio level.
That would be useful, as the sectorsize == 512 is part of the bio ABI.
I'd like to convert all the >> 9 shifts to *512 or /512, but this relies
on the compiler to convert namely the division to shifts where we use
u64.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs: extent-tree: Use round up to replace align macro
2017-11-21 5:54 [PATCH] btrfs: extent-tree: Use round up to replace align macro Qu Wenruo
2017-11-21 7:33 ` Nikolay Borisov
@ 2017-11-21 13:36 ` David Sterba
2017-11-21 14:36 ` Qu Wenruo
1 sibling, 1 reply; 6+ messages in thread
From: David Sterba @ 2017-11-21 13:36 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, dsterba
On Tue, Nov 21, 2017 at 01:54:45PM +0800, Qu Wenruo wrote:
> To save reader seconds before checking if it's rounding up or down.
Agreed, this is more clear. Please switch all instances of ALIGN to
round_up, there's not much point doing it file by file.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs: extent-tree: Use round up to replace align macro
2017-11-21 13:36 ` David Sterba
@ 2017-11-21 14:36 ` Qu Wenruo
0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2017-11-21 14:36 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 668 bytes --]
On 2017年11月21日 21:36, David Sterba wrote:
> On Tue, Nov 21, 2017 at 01:54:45PM +0800, Qu Wenruo wrote:
>> To save reader seconds before checking if it's rounding up or down.
>
> Agreed, this is more clear. Please switch all instances of ALIGN to
> round_up, there's not much point doing it file by file.
OK.
I just encountered this when digging space reservation related code.
Converting all won't be a problem.
Thanks,
Qu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-21 14:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-21 5:54 [PATCH] btrfs: extent-tree: Use round up to replace align macro Qu Wenruo
2017-11-21 7:33 ` Nikolay Borisov
2017-11-21 8:06 ` Qu Wenruo
2017-11-21 13:47 ` David Sterba
2017-11-21 13:36 ` David Sterba
2017-11-21 14:36 ` Qu Wenruo
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).