| Message ID | 20240425200017.GF1031757@ZenIV (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] erofs: switch erofs_bread() to passing offset instead of block number | expand |
Hi Al, On 2024/4/26 04:00, Al Viro wrote: > ... and be more idiomatic when calculating ->pageofs_in. > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- > fs/erofs/zdata.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c > index d417e189f1a0..a4ff20b54cc1 100644 > --- a/fs/erofs/zdata.c > +++ b/fs/erofs/zdata.c > @@ -868,7 +868,7 @@ static int z_erofs_pcluster_begin(struct z_erofs_decompress_frontend *fe) > } else { > void *mptr; > > - mptr = erofs_read_metabuf(&map->buf, sb, erofs_pos(sb, blknr), EROFS_NO_KMAP); > + mptr = erofs_read_metabuf(&map->buf, sb, map->m_pa, EROFS_NO_KMAP); This patch caused some corrupted failure, since here erofs_read_metabuf() is EROFS_NO_KMAP and it's no needed to get a maped-address since only a page reference is needed. > if (IS_ERR(mptr)) { > ret = PTR_ERR(mptr); > erofs_err(sb, "failed to get inline data %d", ret); > @@ -876,7 +876,7 @@ static int z_erofs_pcluster_begin(struct z_erofs_decompress_frontend *fe) > } > get_page(map->buf.page); > WRITE_ONCE(fe->pcl->compressed_bvecs[0].page, map->buf.page); > - fe->pcl->pageofs_in = map->m_pa & ~PAGE_MASK; > + fe->pcl->pageofs_in = offset_in_page(mptr); So it's unnecessary to change this line IMHO. BTW, would you mind routing this series through erofs tree with other erofs patches for -next (as long as this series isn't twisted with vfs and block stuffs...)? Since I may need to test more to ensure they don't break anything and could fix them immediately by hand... Thanks, Gao Xiang > fe->mode = Z_EROFS_PCLUSTER_FOLLOWED_NOINPLACE; > } > /* file-backed inplace I/O pages are traversed in reverse order */
On Fri, Apr 26, 2024 at 01:32:04PM +0800, Gao Xiang wrote: > Hi Al, > This patch caused some corrupted failure, since > here erofs_read_metabuf() is EROFS_NO_KMAP and > it's no needed to get a maped-address since only > a page reference is needed. > > > if (IS_ERR(mptr)) { > > ret = PTR_ERR(mptr); > > erofs_err(sb, "failed to get inline data %d", ret); > > @@ -876,7 +876,7 @@ static int z_erofs_pcluster_begin(struct z_erofs_decompress_frontend *fe) > > } > > get_page(map->buf.page); > > WRITE_ONCE(fe->pcl->compressed_bvecs[0].page, map->buf.page); > > - fe->pcl->pageofs_in = map->m_pa & ~PAGE_MASK; > > + fe->pcl->pageofs_in = offset_in_page(mptr); > > So it's unnecessary to change this line IMHO. *nod* thanks for catching that. > BTW, would you mind routing this series through erofs tree > with other erofs patches for -next (as long as this series > isn't twisted with vfs and block stuffs...)? Since I may > need to test more to ensure they don't break anything and > could fix them immediately by hand... FWIW, my immediate interest here is the first couple of patches. How about the following variant: #misc.erofs (the first two commits) is put into never-rebased mode; you pull it into your tree and do whatever's convenient with the rest. I merge the same branch into block_device work; that way it doesn't cause conflicts whatever else happens in our trees. Are you OK with that? At the moment I have ; git shortlog v6.9-rc2^..misc.erofs Al Viro (2): erofs: switch erofs_bread() to passing offset instead of block number erofs_buf: store address_space instead of inode Linus Torvalds (1): Linux 6.9-rc2 IOW, it's those two commits, based at -rc2. I can rebase that to other starting point if that'd be more convenient for you.
On 2024/5/3 12:15, Al Viro wrote: > On Fri, Apr 26, 2024 at 01:32:04PM +0800, Gao Xiang wrote: >> Hi Al, > >> This patch caused some corrupted failure, since >> here erofs_read_metabuf() is EROFS_NO_KMAP and >> it's no needed to get a maped-address since only >> a page reference is needed. >> >>> if (IS_ERR(mptr)) { >>> ret = PTR_ERR(mptr); >>> erofs_err(sb, "failed to get inline data %d", ret); >>> @@ -876,7 +876,7 @@ static int z_erofs_pcluster_begin(struct z_erofs_decompress_frontend *fe) >>> } >>> get_page(map->buf.page); >>> WRITE_ONCE(fe->pcl->compressed_bvecs[0].page, map->buf.page); >>> - fe->pcl->pageofs_in = map->m_pa & ~PAGE_MASK; >>> + fe->pcl->pageofs_in = offset_in_page(mptr); >> >> So it's unnecessary to change this line IMHO. > > *nod* > > thanks for catching that. > >> BTW, would you mind routing this series through erofs tree >> with other erofs patches for -next (as long as this series >> isn't twisted with vfs and block stuffs...)? Since I may >> need to test more to ensure they don't break anything and >> could fix them immediately by hand... > > FWIW, my immediate interest here is the first couple of patches. Yes, the first two patches are fine by me, you could submit directly. > > How about the following variant: > > #misc.erofs (the first two commits) is put into never-rebased mode; > you pull it into your tree and do whatever's convenient with the rest. > I merge the same branch into block_device work; that way it doesn't > cause conflicts whatever else happens in our trees. > > Are you OK with that? At the moment I have > ; git shortlog v6.9-rc2^..misc.erofs > Al Viro (2): > erofs: switch erofs_bread() to passing offset instead of block number > erofs_buf: store address_space instead of inode > > Linus Torvalds (1): > Linux 6.9-rc2 > > IOW, it's those two commits, based at -rc2. I can rebase that to other > starting point if that'd be more convenient for you. Yeah, thanks for that. I think I will submit two pull requests for the next cycle, and I will send the second pull request after your vfs work is landed upstream and it will include the remaining patches you sent (a bit off this week since we're on holiday here). Thanks, Gao Xiang
Hi Al, On 2024/5/3 21:01, Gao Xiang wrote: > > > On 2024/5/3 12:15, Al Viro wrote: >> On Fri, Apr 26, 2024 at 01:32:04PM +0800, Gao Xiang wrote: >>> Hi Al, >> >>> This patch caused some corrupted failure, since >>> here erofs_read_metabuf() is EROFS_NO_KMAP and >>> it's no needed to get a maped-address since only >>> a page reference is needed. >>> >>>> if (IS_ERR(mptr)) { >>>> ret = PTR_ERR(mptr); >>>> erofs_err(sb, "failed to get inline data %d", ret); >>>> @@ -876,7 +876,7 @@ static int z_erofs_pcluster_begin(struct z_erofs_decompress_frontend *fe) >>>> } >>>> get_page(map->buf.page); >>>> WRITE_ONCE(fe->pcl->compressed_bvecs[0].page, map->buf.page); >>>> - fe->pcl->pageofs_in = map->m_pa & ~PAGE_MASK; >>>> + fe->pcl->pageofs_in = offset_in_page(mptr); >>> >>> So it's unnecessary to change this line IMHO. >> >> *nod* >> >> thanks for catching that. >> >>> BTW, would you mind routing this series through erofs tree >>> with other erofs patches for -next (as long as this series >>> isn't twisted with vfs and block stuffs...)? Since I may >>> need to test more to ensure they don't break anything and >>> could fix them immediately by hand... >> >> FWIW, my immediate interest here is the first couple of patches. > > Yes, the first two patches are fine by me, you could submit > directly. > >> >> How about the following variant: >> >> #misc.erofs (the first two commits) is put into never-rebased mode; >> you pull it into your tree and do whatever's convenient with the rest. >> I merge the same branch into block_device work; that way it doesn't >> cause conflicts whatever else happens in our trees. >> >> Are you OK with that? At the moment I have >> ; git shortlog v6.9-rc2^..misc.erofs >> Al Viro (2): >> erofs: switch erofs_bread() to passing offset instead of block number >> erofs_buf: store address_space instead of inode >> >> Linus Torvalds (1): >> Linux 6.9-rc2 >> >> IOW, it's those two commits, based at -rc2. I can rebase that to other >> starting point if that'd be more convenient for you. > > Yeah, thanks for that. I think I will submit two pull requests for > the next cycle, and I will send the second pull request after your > vfs work is landed upstream and it will include the remaining > patches you sent (a bit off this week since we're on holiday here). Sorry for a bit delay... I will add #misc.erofs and the following patches with some fix to -next soon. Thanks, Gao Xiang
diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index d417e189f1a0..a4ff20b54cc1 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -868,7 +868,7 @@ static int z_erofs_pcluster_begin(struct z_erofs_decompress_frontend *fe) } else { void *mptr; - mptr = erofs_read_metabuf(&map->buf, sb, erofs_pos(sb, blknr), EROFS_NO_KMAP); + mptr = erofs_read_metabuf(&map->buf, sb, map->m_pa, EROFS_NO_KMAP); if (IS_ERR(mptr)) { ret = PTR_ERR(mptr); erofs_err(sb, "failed to get inline data %d", ret); @@ -876,7 +876,7 @@ static int z_erofs_pcluster_begin(struct z_erofs_decompress_frontend *fe) } get_page(map->buf.page); WRITE_ONCE(fe->pcl->compressed_bvecs[0].page, map->buf.page); - fe->pcl->pageofs_in = map->m_pa & ~PAGE_MASK; + fe->pcl->pageofs_in = offset_in_page(mptr); fe->mode = Z_EROFS_PCLUSTER_FOLLOWED_NOINPLACE; } /* file-backed inplace I/O pages are traversed in reverse order */
... and be more idiomatic when calculating ->pageofs_in. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- fs/erofs/zdata.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)