| Message ID | 20191107182410.12660-31-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [01/46] xfs: move incore structures out of xfs_da_format.h | expand |
On Thu, Nov 07, 2019 at 07:23:54PM +0100, Christoph Hellwig wrote: > Use an offset as the main means for iteration, and only do pointer > arithmetics to find the data/unused entries. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/xfs/xfs_dir2_readdir.c | 34 +++++++++++++++++----------------- > 1 file changed, 17 insertions(+), 17 deletions(-) > > diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c > index 0d234b649d65..c4314e9e3dd8 100644 > --- a/fs/xfs/xfs_dir2_readdir.c > +++ b/fs/xfs/xfs_dir2_readdir.c > @@ -351,13 +351,13 @@ xfs_dir2_leaf_getdents( > xfs_dir2_data_hdr_t *hdr; /* data block header */ gcc complained about this variable being set but not used. > xfs_dir2_data_entry_t *dep; /* data entry */ > xfs_dir2_data_unused_t *dup; /* unused entry */ > - char *ptr = NULL; /* pointer to current data */ > struct xfs_da_geometry *geo = args->geo; > xfs_dablk_t rablk = 0; /* current readahead block */ > xfs_dir2_off_t curoff; /* current overall offset */ > int length; /* temporary length value */ > int byteoff; /* offset in current block */ > int lock_mode; > + unsigned int offset = 0; This is the offset within the block, right? > int error = 0; /* error return value */ > > /* > @@ -384,7 +384,7 @@ xfs_dir2_leaf_getdents( > * If we have no buffer, or we're off the end of the > * current buffer, need to get another one. > */ > - if (!bp || ptr >= (char *)bp->b_addr + geo->blksize) { > + if (!bp || offset + geo->blksize) { ^^^^^^^^^^^^^^^^^^^^^ In which case, isn't this always true? Was this supposed to be offset >= geo->blksize? --D > if (bp) { > xfs_trans_brelse(args->trans, bp); > bp = NULL; > @@ -402,7 +402,7 @@ xfs_dir2_leaf_getdents( > /* > * Find our position in the block. > */ > - ptr = (char *)dp->d_ops->data_entry_p(hdr); > + offset = dp->d_ops->data_entry_offset; > byteoff = xfs_dir2_byte_to_off(geo, curoff); > /* > * Skip past the header. > @@ -413,20 +413,20 @@ xfs_dir2_leaf_getdents( > * Skip past entries until we reach our offset. > */ > else { > - while ((char *)ptr - (char *)hdr < byteoff) { > - dup = (xfs_dir2_data_unused_t *)ptr; > + while (offset < byteoff) { > + dup = bp->b_addr + offset; > > if (be16_to_cpu(dup->freetag) > == XFS_DIR2_DATA_FREE_TAG) { > > length = be16_to_cpu(dup->length); > - ptr += length; > + offset += length; > continue; > } > - dep = (xfs_dir2_data_entry_t *)ptr; > + dep = bp->b_addr + offset; > length = > dp->d_ops->data_entsize(dep->namelen); > - ptr += length; > + offset += length; > } > /* > * Now set our real offset. > @@ -434,28 +434,28 @@ xfs_dir2_leaf_getdents( > curoff = > xfs_dir2_db_off_to_byte(geo, > xfs_dir2_byte_to_db(geo, curoff), > - (char *)ptr - (char *)hdr); > - if (ptr >= (char *)hdr + geo->blksize) { > + offset); > + if (offset >= geo->blksize) > continue; > - } > } > } > + > /* > - * We have a pointer to an entry. > - * Is it a live one? > + * We have a pointer to an entry. Is it a live one? > */ > - dup = (xfs_dir2_data_unused_t *)ptr; > + dup = bp->b_addr + offset; > + > /* > * No, it's unused, skip over it. > */ > if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) { > length = be16_to_cpu(dup->length); > - ptr += length; > + offset += length; > curoff += length; > continue; > } > > - dep = (xfs_dir2_data_entry_t *)ptr; > + dep = bp->b_addr + offset; > length = dp->d_ops->data_entsize(dep->namelen); > filetype = dp->d_ops->data_get_ftype(dep); > > @@ -474,7 +474,7 @@ xfs_dir2_leaf_getdents( > /* > * Advance to next entry in the block. > */ > - ptr += length; > + offset += length; > curoff += length; > /* bufsize may have just been a guess; don't go negative */ > bufsize = bufsize > length ? bufsize - length : 0; > -- > 2.20.1 >
On Thu, Nov 07, 2019 at 02:41:57PM -0800, Darrick J. Wong wrote: > On Thu, Nov 07, 2019 at 07:23:54PM +0100, Christoph Hellwig wrote: > > Use an offset as the main means for iteration, and only do pointer > > arithmetics to find the data/unused entries. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > --- > > fs/xfs/xfs_dir2_readdir.c | 34 +++++++++++++++++----------------- > > 1 file changed, 17 insertions(+), 17 deletions(-) > > > > diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c > > index 0d234b649d65..c4314e9e3dd8 100644 > > --- a/fs/xfs/xfs_dir2_readdir.c > > +++ b/fs/xfs/xfs_dir2_readdir.c > > @@ -351,13 +351,13 @@ xfs_dir2_leaf_getdents( > > xfs_dir2_data_hdr_t *hdr; /* data block header */ > > gcc complained about this variable being set but not used. Your gcc is obviously smarted than mine, because it is unused. > > > xfs_dir2_data_entry_t *dep; /* data entry */ > > xfs_dir2_data_unused_t *dup; /* unused entry */ > > - char *ptr = NULL; /* pointer to current data */ > > struct xfs_da_geometry *geo = args->geo; > > xfs_dablk_t rablk = 0; /* current readahead block */ > > xfs_dir2_off_t curoff; /* current overall offset */ > > int length; /* temporary length value */ > > int byteoff; /* offset in current block */ > > int lock_mode; > > + unsigned int offset = 0; > > This is the offset within the block, right? Yes. > > > int error = 0; /* error return value */ > > > > /* > > @@ -384,7 +384,7 @@ xfs_dir2_leaf_getdents( > > * If we have no buffer, or we're off the end of the > > * current buffer, need to get another one. > > */ > > - if (!bp || ptr >= (char *)bp->b_addr + geo->blksize) { > > + if (!bp || offset + geo->blksize) { > > ^^^^^^^^^^^^^^^^^^^^^ > > In which case, isn't this always true? Was this supposed to be > offset >= geo->blksize? Yes, fixed up now.
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c index 0d234b649d65..c4314e9e3dd8 100644 --- a/fs/xfs/xfs_dir2_readdir.c +++ b/fs/xfs/xfs_dir2_readdir.c @@ -351,13 +351,13 @@ xfs_dir2_leaf_getdents( xfs_dir2_data_hdr_t *hdr; /* data block header */ xfs_dir2_data_entry_t *dep; /* data entry */ xfs_dir2_data_unused_t *dup; /* unused entry */ - char *ptr = NULL; /* pointer to current data */ struct xfs_da_geometry *geo = args->geo; xfs_dablk_t rablk = 0; /* current readahead block */ xfs_dir2_off_t curoff; /* current overall offset */ int length; /* temporary length value */ int byteoff; /* offset in current block */ int lock_mode; + unsigned int offset = 0; int error = 0; /* error return value */ /* @@ -384,7 +384,7 @@ xfs_dir2_leaf_getdents( * If we have no buffer, or we're off the end of the * current buffer, need to get another one. */ - if (!bp || ptr >= (char *)bp->b_addr + geo->blksize) { + if (!bp || offset + geo->blksize) { if (bp) { xfs_trans_brelse(args->trans, bp); bp = NULL; @@ -402,7 +402,7 @@ xfs_dir2_leaf_getdents( /* * Find our position in the block. */ - ptr = (char *)dp->d_ops->data_entry_p(hdr); + offset = dp->d_ops->data_entry_offset; byteoff = xfs_dir2_byte_to_off(geo, curoff); /* * Skip past the header. @@ -413,20 +413,20 @@ xfs_dir2_leaf_getdents( * Skip past entries until we reach our offset. */ else { - while ((char *)ptr - (char *)hdr < byteoff) { - dup = (xfs_dir2_data_unused_t *)ptr; + while (offset < byteoff) { + dup = bp->b_addr + offset; if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) { length = be16_to_cpu(dup->length); - ptr += length; + offset += length; continue; } - dep = (xfs_dir2_data_entry_t *)ptr; + dep = bp->b_addr + offset; length = dp->d_ops->data_entsize(dep->namelen); - ptr += length; + offset += length; } /* * Now set our real offset. @@ -434,28 +434,28 @@ xfs_dir2_leaf_getdents( curoff = xfs_dir2_db_off_to_byte(geo, xfs_dir2_byte_to_db(geo, curoff), - (char *)ptr - (char *)hdr); - if (ptr >= (char *)hdr + geo->blksize) { + offset); + if (offset >= geo->blksize) continue; - } } } + /* - * We have a pointer to an entry. - * Is it a live one? + * We have a pointer to an entry. Is it a live one? */ - dup = (xfs_dir2_data_unused_t *)ptr; + dup = bp->b_addr + offset; + /* * No, it's unused, skip over it. */ if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) { length = be16_to_cpu(dup->length); - ptr += length; + offset += length; curoff += length; continue; } - dep = (xfs_dir2_data_entry_t *)ptr; + dep = bp->b_addr + offset; length = dp->d_ops->data_entsize(dep->namelen); filetype = dp->d_ops->data_get_ftype(dep); @@ -474,7 +474,7 @@ xfs_dir2_leaf_getdents( /* * Advance to next entry in the block. */ - ptr += length; + offset += length; curoff += length; /* bufsize may have just been a guess; don't go negative */ bufsize = bufsize > length ? bufsize - length : 0;
Use an offset as the main means for iteration, and only do pointer arithmetics to find the data/unused entries. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_dir2_readdir.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)