Re: [PATCH v6 1/5] erofs: use absolute position in xattr iterator

[Gao Xiang <hsiangkao@linux.alibaba.com>]

On 2023/6/8 19:30, Jingbo Xu wrote:
> Replace blkaddr/ofs with pos in 'struct erofs_xattr_iter'.
> 
> After erofs_bread() is introduced to replace raw page cache APIs for
> metadata I/Os handling, xattr_iter_fixup() is no longer needed anymore.
> 
> In addition, it is also unnecessary to check if the iterated position is
> span over the block boundary as absolute offset is used instead of
> blkaddr + offset pairs.
> 
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>

...

>   
> @@ -399,9 +365,10 @@ static int shared_getxattr(struct inode *inode, struct getxattr_iter *it)
>   
>   	for (i = 0; i < vi->xattr_shared_count; ++i) {
>   		xsid = vi->xattr_shared_xattrs[i];
> -		it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid);
> -		it->it.ofs = erofs_xattr_blkoff(sb, xsid);
> -		it->it.kaddr = erofs_bread(&it->it.buf, it->it.blkaddr, EROFS_KMAP);
> +		it->it.pos = erofs_pos(sb, EROFS_SB(sb)->xattr_blkaddr) +
> +				       xsid * sizeof(__u32);

					^ sizeof(__le32); ?

> +		it->it.kaddr = erofs_bread(&it->it.buf,
> +				erofs_blknr(sb, it->it.pos), EROFS_KMAP);
>   		if (IS_ERR(it->it.kaddr))
>   			return PTR_ERR(it->it.kaddr);
>   
> @@ -604,9 +571,10 @@ static int shared_listxattr(struct listxattr_iter *it)
>   
>   	for (i = 0; i < vi->xattr_shared_count; ++i) {
>   		xsid = vi->xattr_shared_xattrs[i];
> -		it->it.blkaddr = erofs_xattr_blkaddr(sb, xsid);
> -		it->it.ofs = erofs_xattr_blkoff(sb, xsid);
> -		it->it.kaddr = erofs_bread(&it->it.buf, it->it.blkaddr, EROFS_KMAP);
> +		it->it.pos = erofs_pos(sb, EROFS_SB(sb)->xattr_blkaddr) +
> +				       xsid * sizeof(__u32);

					^ sizeof(__le32); ?


Otherwise it looks good to me.

Thanks,
Gao Xiang