* [PATCH 2/2] erofs-utils: refactor: remove end argument from erofs_mapbh
[not found] <20210101091158.13896-1-huww98@outlook.com>
@ 2021-01-01 9:11 ` 胡玮文
2021-01-02 5:14 ` Gao Xiang
0 siblings, 1 reply; 4+ messages in thread
From: 胡玮文 @ 2021-01-01 9:11 UTC (permalink / raw)
To: Li Guifu, Miao Xie, Fang Wei
Cc: linux-erofs, Gao Xiang, 胡玮文
Signed-off-by: Hu Weiwen <huww98@outlook.com>
---
include/erofs/cache.h | 2 +-
lib/cache.c | 3 +--
lib/compress.c | 2 +-
lib/inode.c | 10 +++++-----
lib/xattr.c | 2 +-
mkfs/main.c | 2 +-
6 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/include/erofs/cache.h b/include/erofs/cache.h
index 8c171f5..f8dff67 100644
--- a/include/erofs/cache.h
+++ b/include/erofs/cache.h
@@ -95,7 +95,7 @@ struct erofs_buffer_head *erofs_balloc(int type, erofs_off_t size,
struct erofs_buffer_head *erofs_battach(struct erofs_buffer_head *bh,
int type, unsigned int size);
-erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb, bool end);
+erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb);
bool erofs_bflush(struct erofs_buffer_block *bb);
void erofs_bdrop(struct erofs_buffer_head *bh, bool tryrevoke);
diff --git a/lib/cache.c b/lib/cache.c
index 3412a0b..9765cfd 100644
--- a/lib/cache.c
+++ b/lib/cache.c
@@ -329,9 +329,8 @@ static erofs_blk_t __erofs_mapbh(struct erofs_buffer_block *bb)
return blkaddr;
}
-erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb, bool end)
+erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb)
{
- DBG_BUGON(!end);
struct erofs_buffer_block *t = last_mapped_block;
while (1) {
t = list_next_entry(t, list);
diff --git a/lib/compress.c b/lib/compress.c
index 86db940..2b1f93c 100644
--- a/lib/compress.c
+++ b/lib/compress.c
@@ -416,7 +416,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
memset(compressmeta, 0, Z_EROFS_LEGACY_MAP_HEADER_SIZE);
- blkaddr = erofs_mapbh(bh->block, true); /* start_blkaddr */
+ blkaddr = erofs_mapbh(bh->block); /* start_blkaddr */
ctx.blkaddr = blkaddr;
ctx.metacur = compressmeta + Z_EROFS_LEGACY_MAP_HEADER_SIZE;
ctx.head = ctx.tail = 0;
diff --git a/lib/inode.c b/lib/inode.c
index 3d634fc..60666bb 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -150,7 +150,7 @@ static int __allocate_inode_bh_data(struct erofs_inode *inode,
inode->bh_data = bh;
/* get blkaddr of the bh */
- ret = erofs_mapbh(bh->block, true);
+ ret = erofs_mapbh(bh->block);
DBG_BUGON(ret < 0);
/* write blocks except for the tail-end block */
@@ -524,7 +524,7 @@ int erofs_prepare_tail_block(struct erofs_inode *inode)
bh->op = &erofs_skip_write_bhops;
/* get blkaddr of bh */
- ret = erofs_mapbh(bh->block, true);
+ ret = erofs_mapbh(bh->block);
DBG_BUGON(ret < 0);
inode->u.i_blkaddr = bh->block->blkaddr;
@@ -634,7 +634,7 @@ int erofs_write_tail_end(struct erofs_inode *inode)
int ret;
erofs_off_t pos;
- erofs_mapbh(bh->block, true);
+ erofs_mapbh(bh->block);
pos = erofs_btell(bh, true) - EROFS_BLKSIZ;
ret = dev_write(inode->idata, pos, inode->idata_size);
if (ret)
@@ -871,7 +871,7 @@ void erofs_fixup_meta_blkaddr(struct erofs_inode *rootdir)
struct erofs_buffer_head *const bh = rootdir->bh;
erofs_off_t off, meta_offset;
- erofs_mapbh(bh->block, true);
+ erofs_mapbh(bh->block);
off = erofs_btell(bh, false);
if (off > rootnid_maxoffset)
@@ -890,7 +890,7 @@ erofs_nid_t erofs_lookupnid(struct erofs_inode *inode)
if (!bh)
return inode->nid;
- erofs_mapbh(bh->block, true);
+ erofs_mapbh(bh->block);
off = erofs_btell(bh, false);
meta_offset = blknr_to_addr(sbi.meta_blkaddr);
diff --git a/lib/xattr.c b/lib/xattr.c
index 49ebb9c..8b7bcb1 100644
--- a/lib/xattr.c
+++ b/lib/xattr.c
@@ -575,7 +575,7 @@ int erofs_build_shared_xattrs_from_path(const char *path)
}
bh->op = &erofs_skip_write_bhops;
- erofs_mapbh(bh->block, true);
+ erofs_mapbh(bh->block);
off = erofs_btell(bh, false);
sbi.xattr_blkaddr = off / EROFS_BLKSIZ;
diff --git a/mkfs/main.c b/mkfs/main.c
index c63b274..1c23560 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -304,7 +304,7 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
round_up(EROFS_SUPER_END, EROFS_BLKSIZ);
char *buf;
- *blocks = erofs_mapbh(NULL, true);
+ *blocks = erofs_mapbh(NULL);
sb.blocks = cpu_to_le32(*blocks);
sb.root_nid = cpu_to_le16(root_nid);
memcpy(sb.uuid, sbi.uuid, sizeof(sb.uuid));
--
2.30.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] erofs-utils: refactor: remove end argument from erofs_mapbh
2021-01-01 9:11 ` [PATCH 2/2] erofs-utils: refactor: remove end argument from erofs_mapbh 胡玮文
@ 2021-01-02 5:14 ` Gao Xiang
2021-01-02 6:00 ` 胡 玮文
0 siblings, 1 reply; 4+ messages in thread
From: Gao Xiang @ 2021-01-02 5:14 UTC (permalink / raw)
To: 胡玮文; +Cc: Gao Xiang, linux-erofs, Miao Xie
Hi Weiwen,
On Fri, Jan 01, 2021 at 05:11:58PM +0800, 胡玮文 wrote:
> Signed-off-by: Hu Weiwen <huww98@outlook.com>
It seems that it drops the needed argument `end'.
The original purpose of that is to get the beginning/end blkaddr of a buffer block
in preparation for later use, some specific reasons to get rid of it?
(also it only drops one extra line considering the diffstat....)
Thanks,
Gao Xiang
> ---
> include/erofs/cache.h | 2 +-
> lib/cache.c | 3 +--
> lib/compress.c | 2 +-
> lib/inode.c | 10 +++++-----
> lib/xattr.c | 2 +-
> mkfs/main.c | 2 +-
> 6 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/include/erofs/cache.h b/include/erofs/cache.h
> index 8c171f5..f8dff67 100644
> --- a/include/erofs/cache.h
> +++ b/include/erofs/cache.h
> @@ -95,7 +95,7 @@ struct erofs_buffer_head *erofs_balloc(int type, erofs_off_t size,
> struct erofs_buffer_head *erofs_battach(struct erofs_buffer_head *bh,
> int type, unsigned int size);
>
> -erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb, bool end);
> +erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb);
> bool erofs_bflush(struct erofs_buffer_block *bb);
>
> void erofs_bdrop(struct erofs_buffer_head *bh, bool tryrevoke);
> diff --git a/lib/cache.c b/lib/cache.c
> index 3412a0b..9765cfd 100644
> --- a/lib/cache.c
> +++ b/lib/cache.c
> @@ -329,9 +329,8 @@ static erofs_blk_t __erofs_mapbh(struct erofs_buffer_block *bb)
> return blkaddr;
> }
>
> -erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb, bool end)
> +erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb)
> {
> - DBG_BUGON(!end);
> struct erofs_buffer_block *t = last_mapped_block;
> while (1) {
> t = list_next_entry(t, list);
> diff --git a/lib/compress.c b/lib/compress.c
> index 86db940..2b1f93c 100644
> --- a/lib/compress.c
> +++ b/lib/compress.c
> @@ -416,7 +416,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
>
> memset(compressmeta, 0, Z_EROFS_LEGACY_MAP_HEADER_SIZE);
>
> - blkaddr = erofs_mapbh(bh->block, true); /* start_blkaddr */
> + blkaddr = erofs_mapbh(bh->block); /* start_blkaddr */
> ctx.blkaddr = blkaddr;
> ctx.metacur = compressmeta + Z_EROFS_LEGACY_MAP_HEADER_SIZE;
> ctx.head = ctx.tail = 0;
> diff --git a/lib/inode.c b/lib/inode.c
> index 3d634fc..60666bb 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -150,7 +150,7 @@ static int __allocate_inode_bh_data(struct erofs_inode *inode,
> inode->bh_data = bh;
>
> /* get blkaddr of the bh */
> - ret = erofs_mapbh(bh->block, true);
> + ret = erofs_mapbh(bh->block);
> DBG_BUGON(ret < 0);
>
> /* write blocks except for the tail-end block */
> @@ -524,7 +524,7 @@ int erofs_prepare_tail_block(struct erofs_inode *inode)
> bh->op = &erofs_skip_write_bhops;
>
> /* get blkaddr of bh */
> - ret = erofs_mapbh(bh->block, true);
> + ret = erofs_mapbh(bh->block);
> DBG_BUGON(ret < 0);
> inode->u.i_blkaddr = bh->block->blkaddr;
>
> @@ -634,7 +634,7 @@ int erofs_write_tail_end(struct erofs_inode *inode)
> int ret;
> erofs_off_t pos;
>
> - erofs_mapbh(bh->block, true);
> + erofs_mapbh(bh->block);
> pos = erofs_btell(bh, true) - EROFS_BLKSIZ;
> ret = dev_write(inode->idata, pos, inode->idata_size);
> if (ret)
> @@ -871,7 +871,7 @@ void erofs_fixup_meta_blkaddr(struct erofs_inode *rootdir)
> struct erofs_buffer_head *const bh = rootdir->bh;
> erofs_off_t off, meta_offset;
>
> - erofs_mapbh(bh->block, true);
> + erofs_mapbh(bh->block);
> off = erofs_btell(bh, false);
>
> if (off > rootnid_maxoffset)
> @@ -890,7 +890,7 @@ erofs_nid_t erofs_lookupnid(struct erofs_inode *inode)
> if (!bh)
> return inode->nid;
>
> - erofs_mapbh(bh->block, true);
> + erofs_mapbh(bh->block);
> off = erofs_btell(bh, false);
>
> meta_offset = blknr_to_addr(sbi.meta_blkaddr);
> diff --git a/lib/xattr.c b/lib/xattr.c
> index 49ebb9c..8b7bcb1 100644
> --- a/lib/xattr.c
> +++ b/lib/xattr.c
> @@ -575,7 +575,7 @@ int erofs_build_shared_xattrs_from_path(const char *path)
> }
> bh->op = &erofs_skip_write_bhops;
>
> - erofs_mapbh(bh->block, true);
> + erofs_mapbh(bh->block);
> off = erofs_btell(bh, false);
>
> sbi.xattr_blkaddr = off / EROFS_BLKSIZ;
> diff --git a/mkfs/main.c b/mkfs/main.c
> index c63b274..1c23560 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -304,7 +304,7 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
> round_up(EROFS_SUPER_END, EROFS_BLKSIZ);
> char *buf;
>
> - *blocks = erofs_mapbh(NULL, true);
> + *blocks = erofs_mapbh(NULL);
> sb.blocks = cpu_to_le32(*blocks);
> sb.root_nid = cpu_to_le16(root_nid);
> memcpy(sb.uuid, sbi.uuid, sizeof(sb.uuid));
> --
> 2.30.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] erofs-utils: refactor: remove end argument from erofs_mapbh
2021-01-02 5:14 ` Gao Xiang
@ 2021-01-02 6:00 ` 胡 玮文
2021-01-02 9:27 ` Gao Xiang
0 siblings, 1 reply; 4+ messages in thread
From: 胡 玮文 @ 2021-01-02 6:00 UTC (permalink / raw)
To: Gao Xiang; +Cc: linux-erofs
Hi Xiang,
If I understand it correctly, when ‘end == false’, the last buffer block will not get its ‘blkaddr’. I can’t see why we need this, but I might be missing something. And every invocation of this function just pass ‘true’ to this argument. I’m intended to remove some dead code which is never invoked.
> 在 2021年1月2日，13:15，Gao Xiang <hsiangkao@redhat.com> 写道：
>
> Hi Weiwen,
>
>> On Fri, Jan 01, 2021 at 05:11:58PM +0800, 胡玮文 wrote:
>> Signed-off-by: Hu Weiwen <huww98@outlook.com>
>
> It seems that it drops the needed argument `end'.
>
> The original purpose of that is to get the beginning/end blkaddr of a buffer block
> in preparation for later use, some specific reasons to get rid of it?
> (also it only drops one extra line considering the diffstat....)
That’s because I already assumed ‘end == true’ and rewritten this function in [PATCH 1/2]
Thanks for your reply.
Hu Weiwen
>
> Thanks,
> Gao Xiang
>
>> ---
>> include/erofs/cache.h | 2 +-
>> lib/cache.c | 3 +--
>> lib/compress.c | 2 +-
>> lib/inode.c | 10 +++++-----
>> lib/xattr.c | 2 +-
>> mkfs/main.c | 2 +-
>> 6 files changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/erofs/cache.h b/include/erofs/cache.h
>> index 8c171f5..f8dff67 100644
>> --- a/include/erofs/cache.h
>> +++ b/include/erofs/cache.h
>> @@ -95,7 +95,7 @@ struct erofs_buffer_head *erofs_balloc(int type, erofs_off_t size,
>> struct erofs_buffer_head *erofs_battach(struct erofs_buffer_head *bh,
>> int type, unsigned int size);
>>
>> -erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb, bool end);
>> +erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb);
>> bool erofs_bflush(struct erofs_buffer_block *bb);
>>
>> void erofs_bdrop(struct erofs_buffer_head *bh, bool tryrevoke);
>> diff --git a/lib/cache.c b/lib/cache.c
>> index 3412a0b..9765cfd 100644
>> --- a/lib/cache.c
>> +++ b/lib/cache.c
>> @@ -329,9 +329,8 @@ static erofs_blk_t __erofs_mapbh(struct erofs_buffer_block *bb)
>> return blkaddr;
>> }
>>
>> -erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb, bool end)
>> +erofs_blk_t erofs_mapbh(struct erofs_buffer_block *bb)
>> {
>> - DBG_BUGON(!end);
>> struct erofs_buffer_block *t = last_mapped_block;
>> while (1) {
>> t = list_next_entry(t, list);
>> diff --git a/lib/compress.c b/lib/compress.c
>> index 86db940..2b1f93c 100644
>> --- a/lib/compress.c
>> +++ b/lib/compress.c
>> @@ -416,7 +416,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
>>
>> memset(compressmeta, 0, Z_EROFS_LEGACY_MAP_HEADER_SIZE);
>>
>> - blkaddr = erofs_mapbh(bh->block, true); /* start_blkaddr */
>> + blkaddr = erofs_mapbh(bh->block); /* start_blkaddr */
>> ctx.blkaddr = blkaddr;
>> ctx.metacur = compressmeta + Z_EROFS_LEGACY_MAP_HEADER_SIZE;
>> ctx.head = ctx.tail = 0;
>> diff --git a/lib/inode.c b/lib/inode.c
>> index 3d634fc..60666bb 100644
>> --- a/lib/inode.c
>> +++ b/lib/inode.c
>> @@ -150,7 +150,7 @@ static int __allocate_inode_bh_data(struct erofs_inode *inode,
>> inode->bh_data = bh;
>>
>> /* get blkaddr of the bh */
>> - ret = erofs_mapbh(bh->block, true);
>> + ret = erofs_mapbh(bh->block);
>> DBG_BUGON(ret < 0);
>>
>> /* write blocks except for the tail-end block */
>> @@ -524,7 +524,7 @@ int erofs_prepare_tail_block(struct erofs_inode *inode)
>> bh->op = &erofs_skip_write_bhops;
>>
>> /* get blkaddr of bh */
>> - ret = erofs_mapbh(bh->block, true);
>> + ret = erofs_mapbh(bh->block);
>> DBG_BUGON(ret < 0);
>> inode->u.i_blkaddr = bh->block->blkaddr;
>>
>> @@ -634,7 +634,7 @@ int erofs_write_tail_end(struct erofs_inode *inode)
>> int ret;
>> erofs_off_t pos;
>>
>> - erofs_mapbh(bh->block, true);
>> + erofs_mapbh(bh->block);
>> pos = erofs_btell(bh, true) - EROFS_BLKSIZ;
>> ret = dev_write(inode->idata, pos, inode->idata_size);
>> if (ret)
>> @@ -871,7 +871,7 @@ void erofs_fixup_meta_blkaddr(struct erofs_inode *rootdir)
>> struct erofs_buffer_head *const bh = rootdir->bh;
>> erofs_off_t off, meta_offset;
>>
>> - erofs_mapbh(bh->block, true);
>> + erofs_mapbh(bh->block);
>> off = erofs_btell(bh, false);
>>
>> if (off > rootnid_maxoffset)
>> @@ -890,7 +890,7 @@ erofs_nid_t erofs_lookupnid(struct erofs_inode *inode)
>> if (!bh)
>> return inode->nid;
>>
>> - erofs_mapbh(bh->block, true);
>> + erofs_mapbh(bh->block);
>> off = erofs_btell(bh, false);
>>
>> meta_offset = blknr_to_addr(sbi.meta_blkaddr);
>> diff --git a/lib/xattr.c b/lib/xattr.c
>> index 49ebb9c..8b7bcb1 100644
>> --- a/lib/xattr.c
>> +++ b/lib/xattr.c
>> @@ -575,7 +575,7 @@ int erofs_build_shared_xattrs_from_path(const char *path)
>> }
>> bh->op = &erofs_skip_write_bhops;
>>
>> - erofs_mapbh(bh->block, true);
>> + erofs_mapbh(bh->block);
>> off = erofs_btell(bh, false);
>>
>> sbi.xattr_blkaddr = off / EROFS_BLKSIZ;
>> diff --git a/mkfs/main.c b/mkfs/main.c
>> index c63b274..1c23560 100644
>> --- a/mkfs/main.c
>> +++ b/mkfs/main.c
>> @@ -304,7 +304,7 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,
>> round_up(EROFS_SUPER_END, EROFS_BLKSIZ);
>> char *buf;
>>
>> - *blocks = erofs_mapbh(NULL, true);
>> + *blocks = erofs_mapbh(NULL);
>> sb.blocks = cpu_to_le32(*blocks);
>> sb.root_nid = cpu_to_le16(root_nid);
>> memcpy(sb.uuid, sbi.uuid, sizeof(sb.uuid));
>> --
>> 2.30.0
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] erofs-utils: refactor: remove end argument from erofs_mapbh
2021-01-02 6:00 ` 胡 玮文
@ 2021-01-02 9:27 ` Gao Xiang
0 siblings, 0 replies; 4+ messages in thread
From: Gao Xiang @ 2021-01-02 9:27 UTC (permalink / raw)
To: 胡 玮文; +Cc: linux-erofs
On Sat, Jan 02, 2021 at 06:00:06AM +0000, 胡 玮文 wrote:
> Hi Xiang,
>
> If I understand it correctly, when ‘end == false’, the last buffer block will not get its ‘blkaddr’. I can’t see why we need this, but I might be missing something. And every invocation of this function just pass ‘true’ to this argument. I’m intended to remove some dead code which is never invoked.
Yeah, your understanding is correct. The original purpose when 'end == false' was to
return the start blkaddr of a buffer block in case of (or reserve for) later use (e.g.
get the blkaddr of a inode or some data block that will need). and 'end == true' was
to return the tail blkaddr.... But, I think the behavior of 'end == false' was
completely broken at some time...
>
> > 在 2021年1月2日，13:15，Gao Xiang <hsiangkao@redhat.com> 写道：
> >
> > Hi Weiwen,
> >
> >> On Fri, Jan 01, 2021 at 05:11:58PM +0800, 胡玮文 wrote:
> >> Signed-off-by: Hu Weiwen <huww98@outlook.com>
> >
> > It seems that it drops the needed argument `end'.
> >
> > The original purpose of that is to get the beginning/end blkaddr of a buffer block
> > in preparation for later use, some specific reasons to get rid of it?
> > (also it only drops one extra line considering the diffstat....)
>
> That’s because I already assumed ‘end == true’ and rewritten this function in [PATCH 1/2]
>
Ok, let me seek some time read into it :) the complexity issue of buffer allocation
is a known issue and can be optimized by some caching (Although Android don't have
too many files) :)
Thanks,
Gao Xiang
> Thanks for your reply.
>
> Hu Weiwen
>
> >
> > Thanks,
> > Gao Xiang
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-02 9:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20210101091158.13896-1-huww98@outlook.com>
2021-01-01 9:11 ` [PATCH 2/2] erofs-utils: refactor: remove end argument from erofs_mapbh 胡玮文
2021-01-02 5:14 ` Gao Xiang
2021-01-02 6:00 ` 胡 玮文
2021-01-02 9:27 ` Gao Xiang
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).