* [Cluster-devel] [PATCH] gfs2: hole_size improvement
@ 2018-04-10 17:39 Andreas Gruenbacher
2018-04-10 18:22 ` Andreas Gruenbacher
2018-04-11 13:00 ` Andrew Price
0 siblings, 2 replies; 5+ messages in thread
From: Andreas Gruenbacher @ 2018-04-10 17:39 UTC (permalink / raw)
To: cluster-devel.redhat.com
Rewrite function hole_size so that it computes the entire size of a
hole. Previously, the hole size returned wasn't guaranteed to span the
entire hole and multiple invocations of hole_size could be needed to
reach the next bit of data.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/bmap.c | 106 ++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 67 insertions(+), 39 deletions(-)
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 8c25a64d8ae0..14c21825b890 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -279,6 +279,12 @@ static inline __be64 *metapointer(unsigned int height, const struct metapath *mp
return p + mp->mp_list[height];
}
+static inline const __be64 *metaend(unsigned int height, const struct metapath *mp)
+{
+ const struct buffer_head *bh = mp->mp_bh[height];
+ return (const __be64 *)(bh->b_data + bh->b_size);
+}
+
static void gfs2_metapath_ra(struct gfs2_glock *gl, __be64 *start, __be64 *end)
{
const __be64 *t;
@@ -631,56 +637,78 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
* hole_size - figure out the size of a hole
* @inode: The inode
* @lblock: The logical starting block number
- * @mp: The metapath
+ * @mp: The metapath at lblock
+ * @pholesz: The hole size in bytes (output)
+ *
+ * This function modifies @mp.
*
- * Returns: The hole size in bytes
+ * Returns: errno on error
*
*/
-static u64 hole_size(struct inode *inode, sector_t lblock, struct metapath *mp)
+static int hole_size(struct inode *inode, sector_t lblock, struct metapath *mp,
+ u64 *pholesz)
{
+ sector_t lblock_end = (i_size_read(inode) + i_blocksize(inode) - 1) >>
+ inode->i_blkbits;
struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_sbd *sdp = GFS2_SB(inode);
- struct metapath mp_eof;
- u64 factor = 1;
- int hgt;
- u64 holesz = 0;
const __be64 *first, *end, *ptr;
- const struct buffer_head *bh;
- u64 lblock_stop = (i_size_read(inode) - 1) >> inode->i_blkbits;
- int zeroptrs;
- bool done = false;
-
- /* Get another metapath, to the very last byte */
- find_metapath(sdp, lblock_stop, &mp_eof, ip->i_height);
- for (hgt = ip->i_height - 1; hgt >= 0 && !done; hgt--) {
- bh = mp->mp_bh[hgt];
- if (bh) {
- zeroptrs = 0;
- first = metapointer(hgt, mp);
- end = (const __be64 *)(bh->b_data + bh->b_size);
-
- for (ptr = first; ptr < end; ptr++) {
- if (*ptr) {
- done = true;
- break;
- } else {
- zeroptrs++;
- }
+ u64 holesz = 0, factor = 1;
+ int hgt, ret;
+
+ for (hgt = ip->i_height - 1; hgt >= mp->mp_aheight; hgt--)
+ factor *= sdp->sd_inptrs;
+
+ for (;;) {
+ /* Count the number of zero pointers. */
+ first = metapointer(hgt, mp);
+ end = metaend(hgt, mp);
+ if (end - first > lblock_end - lblock - holesz)
+ end = first + lblock_end - lblock - holesz;
+ for (ptr = first; ptr < end; ptr++) {
+ if (*ptr) {
+ holesz += (ptr - first) * factor;
+ if (hgt == ip->i_height - 1)
+ goto out;
+ mp->mp_list[hgt] += (ptr - first);
+ goto fill_up_metapath;
}
- } else {
- zeroptrs = sdp->sd_inptrs;
}
- if (factor * zeroptrs >= lblock_stop - lblock + 1) {
- holesz = lblock_stop - lblock + 1;
- break;
+ holesz += (ptr - first) * factor;
+
+lower_metapath:
+ /* Decrease height of metapath. */
+ brelse(mp->mp_bh[hgt]);
+ mp->mp_bh[hgt] = NULL;
+ mp->mp_list[hgt] = 0;
+ if (!hgt)
+ goto out;
+ hgt--;
+ factor *= sdp->sd_inptrs;
+
+ /* Advance in metadata tree. */
+ (mp->mp_list[hgt])++;
+ first = metapointer(hgt, mp);
+ end = metaend(hgt, mp);
+ if (first >= end) {
+ if (!hgt)
+ goto out;
+ goto lower_metapath;
}
- holesz += factor * zeroptrs;
- factor *= sdp->sd_inptrs;
- if (hgt && (mp->mp_list[hgt - 1] < mp_eof.mp_list[hgt - 1]))
- (mp->mp_list[hgt - 1])++;
+fill_up_metapath:
+ /* Fill up metapath. */
+ ret = fillup_metapath(ip, mp, ip->i_height - 1);
+ if (ret < 0)
+ return ret;
+ while (ret--) {
+ hgt++;
+ factor /= sdp->sd_inptrs;
+ }
}
- return holesz << inode->i_blkbits;
+out:
+ *pholesz = holesz << inode->i_blkbits;
+ return 0;
}
static void gfs2_stuffed_iomap(struct inode *inode, struct iomap *iomap)
@@ -804,7 +832,7 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
if (pos >= size)
ret = -ENOENT;
else if (height <= ip->i_height)
- iomap->length = hole_size(inode, lblock, &mp);
+ ret = hole_size(inode, lblock, &mp, &iomap->length);
else
iomap->length = size - pos;
}
--
2.14.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Cluster-devel] [PATCH] gfs2: hole_size improvement
2018-04-10 17:39 [Cluster-devel] [PATCH] gfs2: hole_size improvement Andreas Gruenbacher
@ 2018-04-10 18:22 ` Andreas Gruenbacher
2018-04-10 19:28 ` Andreas Gruenbacher
2018-04-11 13:00 ` Andrew Price
1 sibling, 1 reply; 5+ messages in thread
From: Andreas Gruenbacher @ 2018-04-10 18:22 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 10 April 2018 at 19:39, Andreas Gruenbacher <agruenba@redhat.com> wrote:
> Rewrite function hole_size so that it computes the entire size of a
> hole. Previously, the hole size returned wasn't guaranteed to span the
> entire hole and multiple invocations of hole_size could be needed to
> reach the next bit of data.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
> fs/gfs2/bmap.c | 106 ++++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 67 insertions(+), 39 deletions(-)
>
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index 8c25a64d8ae0..14c21825b890 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -279,6 +279,12 @@ static inline __be64 *metapointer(unsigned int height, const struct metapath *mp
> return p + mp->mp_list[height];
> }
>
> +static inline const __be64 *metaend(unsigned int height, const struct metapath *mp)
> +{
> + const struct buffer_head *bh = mp->mp_bh[height];
> + return (const __be64 *)(bh->b_data + bh->b_size);
> +}
> +
> static void gfs2_metapath_ra(struct gfs2_glock *gl, __be64 *start, __be64 *end)
> {
> const __be64 *t;
> @@ -631,56 +637,78 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
> * hole_size - figure out the size of a hole
> * @inode: The inode
> * @lblock: The logical starting block number
> - * @mp: The metapath
> + * @mp: The metapath at lblock
> + * @pholesz: The hole size in bytes (output)
> + *
> + * This function modifies @mp.
> *
> - * Returns: The hole size in bytes
> + * Returns: errno on error
> *
> */
> -static u64 hole_size(struct inode *inode, sector_t lblock, struct metapath *mp)
> +static int hole_size(struct inode *inode, sector_t lblock, struct metapath *mp,
> + u64 *pholesz)
> {
> + sector_t lblock_end = (i_size_read(inode) + i_blocksize(inode) - 1) >>
> + inode->i_blkbits;
> struct gfs2_inode *ip = GFS2_I(inode);
> struct gfs2_sbd *sdp = GFS2_SB(inode);
> - struct metapath mp_eof;
> - u64 factor = 1;
> - int hgt;
> - u64 holesz = 0;
> const __be64 *first, *end, *ptr;
> - const struct buffer_head *bh;
> - u64 lblock_stop = (i_size_read(inode) - 1) >> inode->i_blkbits;
> - int zeroptrs;
> - bool done = false;
> -
> - /* Get another metapath, to the very last byte */
> - find_metapath(sdp, lblock_stop, &mp_eof, ip->i_height);
> - for (hgt = ip->i_height - 1; hgt >= 0 && !done; hgt--) {
> - bh = mp->mp_bh[hgt];
> - if (bh) {
> - zeroptrs = 0;
> - first = metapointer(hgt, mp);
> - end = (const __be64 *)(bh->b_data + bh->b_size);
> -
> - for (ptr = first; ptr < end; ptr++) {
> - if (*ptr) {
> - done = true;
> - break;
> - } else {
> - zeroptrs++;
> - }
> + u64 holesz = 0, factor = 1;
> + int hgt, ret;
> +
> + for (hgt = ip->i_height - 1; hgt >= mp->mp_aheight; hgt--)
> + factor *= sdp->sd_inptrs;
> +
> + for (;;) {
> + /* Count the number of zero pointers. */
> + first = metapointer(hgt, mp);
> + end = metaend(hgt, mp);
> + if (end - first > lblock_end - lblock - holesz)
> + end = first + lblock_end - lblock - holesz;
> + for (ptr = first; ptr < end; ptr++) {
> + if (*ptr) {
> + holesz += (ptr - first) * factor;
> + if (hgt == ip->i_height - 1)
> + goto out;
> + mp->mp_list[hgt] += (ptr - first);
> + goto fill_up_metapath;
> }
> - } else {
> - zeroptrs = sdp->sd_inptrs;
> }
> - if (factor * zeroptrs >= lblock_stop - lblock + 1) {
> - holesz = lblock_stop - lblock + 1;
> - break;
> + holesz += (ptr - first) * factor;
We probably want to check for EOF here as well to stop immediately
when we've had enough:
if (lblock + holesz >= lblock_end)
goto out;
> +
> +lower_metapath:
> + /* Decrease height of metapath. */
> + brelse(mp->mp_bh[hgt]);
> + mp->mp_bh[hgt] = NULL;
> + mp->mp_list[hgt] = 0;
> + if (!hgt)
> + goto out;
> + hgt--;
> + factor *= sdp->sd_inptrs;
> +
> + /* Advance in metadata tree. */
> + (mp->mp_list[hgt])++;
> + first = metapointer(hgt, mp);
> + end = metaend(hgt, mp);
> + if (first >= end) {
> + if (!hgt)
> + goto out;
> + goto lower_metapath;
> }
> - holesz += factor * zeroptrs;
>
> - factor *= sdp->sd_inptrs;
> - if (hgt && (mp->mp_list[hgt - 1] < mp_eof.mp_list[hgt - 1]))
> - (mp->mp_list[hgt - 1])++;
> +fill_up_metapath:
> + /* Fill up metapath. */
> + ret = fillup_metapath(ip, mp, ip->i_height - 1);
> + if (ret < 0)
> + return ret;
> + while (ret--) {
> + hgt++;
> + factor /= sdp->sd_inptrs;
> + }
> }
> - return holesz << inode->i_blkbits;
> +out:
> + *pholesz = holesz << inode->i_blkbits;
> + return 0;
> }
>
> static void gfs2_stuffed_iomap(struct inode *inode, struct iomap *iomap)
> @@ -804,7 +832,7 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
> if (pos >= size)
> ret = -ENOENT;
> else if (height <= ip->i_height)
> - iomap->length = hole_size(inode, lblock, &mp);
> + ret = hole_size(inode, lblock, &mp, &iomap->length);
> else
> iomap->length = size - pos;
> }
> --
> 2.14.3
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Cluster-devel] [PATCH] gfs2: hole_size improvement
2018-04-10 18:22 ` Andreas Gruenbacher
@ 2018-04-10 19:28 ` Andreas Gruenbacher
0 siblings, 0 replies; 5+ messages in thread
From: Andreas Gruenbacher @ 2018-04-10 19:28 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 10 April 2018 at 20:22, Andreas Gruenbacher <agruenba@redhat.com> wrote:
> On 10 April 2018 at 19:39, Andreas Gruenbacher <agruenba@redhat.com> wrote:
>> Rewrite function hole_size so that it computes the entire size of a
>> hole. Previously, the hole size returned wasn't guaranteed to span the
>> entire hole and multiple invocations of hole_size could be needed to
>> reach the next bit of data.
>>
>> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
>> ---
>> fs/gfs2/bmap.c | 106 ++++++++++++++++++++++++++++++++++++---------------------
>> 1 file changed, 67 insertions(+), 39 deletions(-)
>>
>> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
>> index 8c25a64d8ae0..14c21825b890 100644
>> --- a/fs/gfs2/bmap.c
>> +++ b/fs/gfs2/bmap.c
>> @@ -279,6 +279,12 @@ static inline __be64 *metapointer(unsigned int height, const struct metapath *mp
>> return p + mp->mp_list[height];
>> }
>>
>> +static inline const __be64 *metaend(unsigned int height, const struct metapath *mp)
>> +{
>> + const struct buffer_head *bh = mp->mp_bh[height];
>> + return (const __be64 *)(bh->b_data + bh->b_size);
>> +}
>> +
>> static void gfs2_metapath_ra(struct gfs2_glock *gl, __be64 *start, __be64 *end)
>> {
>> const __be64 *t;
>> @@ -631,56 +637,78 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
>> * hole_size - figure out the size of a hole
>> * @inode: The inode
>> * @lblock: The logical starting block number
>> - * @mp: The metapath
>> + * @mp: The metapath at lblock
>> + * @pholesz: The hole size in bytes (output)
>> + *
>> + * This function modifies @mp.
>> *
>> - * Returns: The hole size in bytes
>> + * Returns: errno on error
>> *
>> */
>> -static u64 hole_size(struct inode *inode, sector_t lblock, struct metapath *mp)
>> +static int hole_size(struct inode *inode, sector_t lblock, struct metapath *mp,
>> + u64 *pholesz)
>> {
>> + sector_t lblock_end = (i_size_read(inode) + i_blocksize(inode) - 1) >>
>> + inode->i_blkbits;
>> struct gfs2_inode *ip = GFS2_I(inode);
>> struct gfs2_sbd *sdp = GFS2_SB(inode);
>> - struct metapath mp_eof;
>> - u64 factor = 1;
>> - int hgt;
>> - u64 holesz = 0;
>> const __be64 *first, *end, *ptr;
>> - const struct buffer_head *bh;
>> - u64 lblock_stop = (i_size_read(inode) - 1) >> inode->i_blkbits;
>> - int zeroptrs;
>> - bool done = false;
>> -
>> - /* Get another metapath, to the very last byte */
>> - find_metapath(sdp, lblock_stop, &mp_eof, ip->i_height);
>> - for (hgt = ip->i_height - 1; hgt >= 0 && !done; hgt--) {
>> - bh = mp->mp_bh[hgt];
>> - if (bh) {
>> - zeroptrs = 0;
>> - first = metapointer(hgt, mp);
>> - end = (const __be64 *)(bh->b_data + bh->b_size);
>> -
>> - for (ptr = first; ptr < end; ptr++) {
>> - if (*ptr) {
>> - done = true;
>> - break;
>> - } else {
>> - zeroptrs++;
>> - }
>> + u64 holesz = 0, factor = 1;
>> + int hgt, ret;
>> +
>> + for (hgt = ip->i_height - 1; hgt >= mp->mp_aheight; hgt--)
>> + factor *= sdp->sd_inptrs;
>> +
>> + for (;;) {
>> + /* Count the number of zero pointers. */
>> + first = metapointer(hgt, mp);
>> + end = metaend(hgt, mp);
>> + if (end - first > lblock_end - lblock - holesz)
>> + end = first + lblock_end - lblock - holesz;
>> + for (ptr = first; ptr < end; ptr++) {
>> + if (*ptr) {
>> + holesz += (ptr - first) * factor;
>> + if (hgt == ip->i_height - 1)
>> + goto out;
>> + mp->mp_list[hgt] += (ptr - first);
>> + goto fill_up_metapath;
>> }
>> - } else {
>> - zeroptrs = sdp->sd_inptrs;
>> }
>> - if (factor * zeroptrs >= lblock_stop - lblock + 1) {
>> - holesz = lblock_stop - lblock + 1;
>> - break;
>> + holesz += (ptr - first) * factor;
>
> We probably want to check for EOF here as well to stop immediately
> when we've had enough:
>
> if (lblock + holesz >= lblock_end)
> goto out;
>
>> +
>> +lower_metapath:
>> + /* Decrease height of metapath. */
>> + brelse(mp->mp_bh[hgt]);
>> + mp->mp_bh[hgt] = NULL;
>> + mp->mp_list[hgt] = 0;
>> + if (!hgt)
>> + goto out;
>> + hgt--;
>> + factor *= sdp->sd_inptrs;
>> +
>> + /* Advance in metadata tree. */
>> + (mp->mp_list[hgt])++;
>> + first = metapointer(hgt, mp);
>> + end = metaend(hgt, mp);
>> + if (first >= end) {
>> + if (!hgt)
>> + goto out;
>> + goto lower_metapath;
>> }
>> - holesz += factor * zeroptrs;
>>
>> - factor *= sdp->sd_inptrs;
>> - if (hgt && (mp->mp_list[hgt - 1] < mp_eof.mp_list[hgt - 1]))
>> - (mp->mp_list[hgt - 1])++;
>> +fill_up_metapath:
>> + /* Fill up metapath. */
>> + ret = fillup_metapath(ip, mp, ip->i_height - 1);
>> + if (ret < 0)
>> + return ret;
>> + while (ret--) {
>> + hgt++;
>> + factor /= sdp->sd_inptrs;
This should have been do_div(factor, sdp->sd_inptrs).
>> + }
>> }
>> - return holesz << inode->i_blkbits;
>> +out:
>> + *pholesz = holesz << inode->i_blkbits;
>> + return 0;
>> }
>>
>> static void gfs2_stuffed_iomap(struct inode *inode, struct iomap *iomap)
>> @@ -804,7 +832,7 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
>> if (pos >= size)
>> ret = -ENOENT;
>> else if (height <= ip->i_height)
>> - iomap->length = hole_size(inode, lblock, &mp);
>> + ret = hole_size(inode, lblock, &mp, &iomap->length);
>> else
>> iomap->length = size - pos;
>> }
>> --
>> 2.14.3
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Cluster-devel] [PATCH] gfs2: hole_size improvement
2018-04-10 17:39 [Cluster-devel] [PATCH] gfs2: hole_size improvement Andreas Gruenbacher
2018-04-10 18:22 ` Andreas Gruenbacher
@ 2018-04-11 13:00 ` Andrew Price
2018-04-11 13:37 ` Andreas Gruenbacher
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Price @ 2018-04-11 13:00 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi Andreas,
I only have a few nitpicks...
On 10/04/18 18:39, Andreas Gruenbacher wrote:
> Rewrite function hole_size so that it computes the entire size of a
> hole. Previously, the hole size returned wasn't guaranteed to span the
> entire hole and multiple invocations of hole_size could be needed to
> reach the next bit of data.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
> fs/gfs2/bmap.c | 106 ++++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 67 insertions(+), 39 deletions(-)
>
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index 8c25a64d8ae0..14c21825b890 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -279,6 +279,12 @@ static inline __be64 *metapointer(unsigned int height, const struct metapath *mp
> return p + mp->mp_list[height];
> }
>
> +static inline const __be64 *metaend(unsigned int height, const struct metapath *mp)
> +{
> + const struct buffer_head *bh = mp->mp_bh[height];
> + return (const __be64 *)(bh->b_data + bh->b_size);
> +}
> +
> static void gfs2_metapath_ra(struct gfs2_glock *gl, __be64 *start, __be64 *end)
> {
> const __be64 *t;
> @@ -631,56 +637,78 @@ static int gfs2_iomap_alloc(struct inode *inode, struct iomap *iomap,
> * hole_size - figure out the size of a hole
> * @inode: The inode
> * @lblock: The logical starting block number
> - * @mp: The metapath
> + * @mp: The metapath at lblock
> + * @pholesz: The hole size in bytes (output)
> + *
> + * This function modifies @mp.
> *
> - * Returns: The hole size in bytes
> + * Returns: errno on error
> *
> */
> -static u64 hole_size(struct inode *inode, sector_t lblock, struct metapath *mp)
> +static int hole_size(struct inode *inode, sector_t lblock, struct metapath *mp,
> + u64 *pholesz)
> {
> + sector_t lblock_end = (i_size_read(inode) + i_blocksize(inode) - 1) >>
> + inode->i_blkbits;
> struct gfs2_inode *ip = GFS2_I(inode);
> struct gfs2_sbd *sdp = GFS2_SB(inode);
> - struct metapath mp_eof;
> - u64 factor = 1;
> - int hgt;
> - u64 holesz = 0;
> const __be64 *first, *end, *ptr;
> - const struct buffer_head *bh;
> - u64 lblock_stop = (i_size_read(inode) - 1) >> inode->i_blkbits;
> - int zeroptrs;
> - bool done = false;
> -
> - /* Get another metapath, to the very last byte */
> - find_metapath(sdp, lblock_stop, &mp_eof, ip->i_height);
> - for (hgt = ip->i_height - 1; hgt >= 0 && !done; hgt--) {
> - bh = mp->mp_bh[hgt];
> - if (bh) {
> - zeroptrs = 0;
> - first = metapointer(hgt, mp);
> - end = (const __be64 *)(bh->b_data + bh->b_size);
> -
> - for (ptr = first; ptr < end; ptr++) {
> - if (*ptr) {
> - done = true;
> - break;
> - } else {
> - zeroptrs++;
> - }
> + u64 holesz = 0, factor = 1;
> + int hgt, ret;
hgt could be unsigned for consistency and checking.
> +
> + for (hgt = ip->i_height - 1; hgt >= mp->mp_aheight; hgt--)
> + factor *= sdp->sd_inptrs;
> +
> + for (;;) {
> + /* Count the number of zero pointers. */
> + first = metapointer(hgt, mp);
> + end = metaend(hgt, mp);
> + if (end - first > lblock_end - lblock - holesz)
> + end = first + lblock_end - lblock - holesz;
> + for (ptr = first; ptr < end; ptr++) {
> + if (*ptr) {
> + holesz += (ptr - first) * factor;
> + if (hgt == ip->i_height - 1)
> + goto out;
> + mp->mp_list[hgt] += (ptr - first);
> + goto fill_up_metapath;
> }
> - } else {
> - zeroptrs = sdp->sd_inptrs;
> }
> - if (factor * zeroptrs >= lblock_stop - lblock + 1) {
> - holesz = lblock_stop - lblock + 1;
> - break;
> + holesz += (ptr - first) * factor;
> +
> +lower_metapath:
> + /* Decrease height of metapath. */
> + brelse(mp->mp_bh[hgt]);
> + mp->mp_bh[hgt] = NULL;
> + mp->mp_list[hgt] = 0;
> + if (!hgt)
> + goto out;
> + hgt--;
> + factor *= sdp->sd_inptrs;
> +
> + /* Advance in metadata tree. */
> + (mp->mp_list[hgt])++;
> + first = metapointer(hgt, mp);
> + end = metaend(hgt, mp);
> + if (first >= end) {
> + if (!hgt)
> + goto out;
> + goto lower_metapath;
> }
> - holesz += factor * zeroptrs;
>
> - factor *= sdp->sd_inptrs;
> - if (hgt && (mp->mp_list[hgt - 1] < mp_eof.mp_list[hgt - 1]))
> - (mp->mp_list[hgt - 1])++;
> +fill_up_metapath:
> + /* Fill up metapath. */
Comment could be removed as the function name and the label are the same
as the comment :)
> + ret = fillup_metapath(ip, mp, ip->i_height - 1);
> + if (ret < 0)
> + return ret;
> + while (ret--) {
> + hgt++;
Could be taken out of the loop as hgt += ret
The flow is a little hard to follow but I don't see any obvious
problems. Has it been tested with directories as well as files?
Reviewed-by: Andrew Price <anprice@redhat.com>
Andy
> + factor /= sdp->sd_inptrs;
> + }
> }
> - return holesz << inode->i_blkbits;
> +out:
> + *pholesz = holesz << inode->i_blkbits;
> + return 0;
> }
>
> static void gfs2_stuffed_iomap(struct inode *inode, struct iomap *iomap)
> @@ -804,7 +832,7 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
> if (pos >= size)
> ret = -ENOENT;
> else if (height <= ip->i_height)
> - iomap->length = hole_size(inode, lblock, &mp);
> + ret = hole_size(inode, lblock, &mp, &iomap->length);
> else
> iomap->length = size - pos;
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Cluster-devel] [PATCH] gfs2: hole_size improvement
2018-04-11 13:00 ` Andrew Price
@ 2018-04-11 13:37 ` Andreas Gruenbacher
0 siblings, 0 replies; 5+ messages in thread
From: Andreas Gruenbacher @ 2018-04-11 13:37 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi Andy,
On 11 April 2018 at 15:00, Andrew Price <anprice@redhat.com> wrote:
> Hi Andreas,
>
> I only have a few nitpicks...
>
> On 10/04/18 18:39, Andreas Gruenbacher wrote:
>>
>> Rewrite function hole_size so that it computes the entire size of a
>> hole. Previously, the hole size returned wasn't guaranteed to span the
>> entire hole and multiple invocations of hole_size could be needed to
>> reach the next bit of data.
>>
>> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
>> ---
>> fs/gfs2/bmap.c | 106
>> ++++++++++++++++++++++++++++++++++++---------------------
>> 1 file changed, 67 insertions(+), 39 deletions(-)
>>
>> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
>> index 8c25a64d8ae0..14c21825b890 100644
>> --- a/fs/gfs2/bmap.c
>> +++ b/fs/gfs2/bmap.c
>> @@ -279,6 +279,12 @@ static inline __be64 *metapointer(unsigned int
>> height, const struct metapath *mp
>> return p + mp->mp_list[height];
>> }
>> +static inline const __be64 *metaend(unsigned int height, const struct
>> metapath *mp)
>> +{
>> + const struct buffer_head *bh = mp->mp_bh[height];
>> + return (const __be64 *)(bh->b_data + bh->b_size);
>> +}
>> +
>> static void gfs2_metapath_ra(struct gfs2_glock *gl, __be64 *start,
>> __be64 *end)
>> {
>> const __be64 *t;
>> @@ -631,56 +637,78 @@ static int gfs2_iomap_alloc(struct inode *inode,
>> struct iomap *iomap,
>> * hole_size - figure out the size of a hole
>> * @inode: The inode
>> * @lblock: The logical starting block number
>> - * @mp: The metapath
>> + * @mp: The metapath at lblock
>> + * @pholesz: The hole size in bytes (output)
>> + *
>> + * This function modifies @mp.
>> *
>> - * Returns: The hole size in bytes
>> + * Returns: errno on error
>> *
>> */
>> -static u64 hole_size(struct inode *inode, sector_t lblock, struct
>> metapath *mp)
>> +static int hole_size(struct inode *inode, sector_t lblock, struct
>> metapath *mp,
>> + u64 *pholesz)
>> {
>> + sector_t lblock_end = (i_size_read(inode) + i_blocksize(inode) -
>> 1) >>
>> + inode->i_blkbits;
>> struct gfs2_inode *ip = GFS2_I(inode);
>> struct gfs2_sbd *sdp = GFS2_SB(inode);
>> - struct metapath mp_eof;
>> - u64 factor = 1;
>> - int hgt;
>> - u64 holesz = 0;
>> const __be64 *first, *end, *ptr;
>> - const struct buffer_head *bh;
>> - u64 lblock_stop = (i_size_read(inode) - 1) >> inode->i_blkbits;
>> - int zeroptrs;
>> - bool done = false;
>> -
>> - /* Get another metapath, to the very last byte */
>> - find_metapath(sdp, lblock_stop, &mp_eof, ip->i_height);
>> - for (hgt = ip->i_height - 1; hgt >= 0 && !done; hgt--) {
>> - bh = mp->mp_bh[hgt];
>> - if (bh) {
>> - zeroptrs = 0;
>> - first = metapointer(hgt, mp);
>> - end = (const __be64 *)(bh->b_data + bh->b_size);
>> -
>> - for (ptr = first; ptr < end; ptr++) {
>> - if (*ptr) {
>> - done = true;
>> - break;
>> - } else {
>> - zeroptrs++;
>> - }
>> + u64 holesz = 0, factor = 1;
>> + int hgt, ret;
>
> hgt could be unsigned for consistency and checking.
Right, it can't go negative (anymore).
>> +
>> + for (hgt = ip->i_height - 1; hgt >= mp->mp_aheight; hgt--)
>> + factor *= sdp->sd_inptrs;
>> +
>> + for (;;) {
>> + /* Count the number of zero pointers. */
>> + first = metapointer(hgt, mp);
>> + end = metaend(hgt, mp);
>> + if (end - first > lblock_end - lblock - holesz)
>> + end = first + lblock_end - lblock - holesz;
>> + for (ptr = first; ptr < end; ptr++) {
>> + if (*ptr) {
>> + holesz += (ptr - first) * factor;
>> + if (hgt == ip->i_height - 1)
>> + goto out;
>> + mp->mp_list[hgt] += (ptr - first);
>> + goto fill_up_metapath;
>> }
>> - } else {
>> - zeroptrs = sdp->sd_inptrs;
>> }
>> - if (factor * zeroptrs >= lblock_stop - lblock + 1) {
>> - holesz = lblock_stop - lblock + 1;
>> - break;
>> + holesz += (ptr - first) * factor;
>> +
>> +lower_metapath:
>> + /* Decrease height of metapath. */
>> + brelse(mp->mp_bh[hgt]);
>> + mp->mp_bh[hgt] = NULL;
>> + mp->mp_list[hgt] = 0;
>> + if (!hgt)
>> + goto out;
>> + hgt--;
>> + factor *= sdp->sd_inptrs;
>> +
>> + /* Advance in metadata tree. */
>> + (mp->mp_list[hgt])++;
>> + first = metapointer(hgt, mp);
>> + end = metaend(hgt, mp);
>> + if (first >= end) {
>> + if (!hgt)
>> + goto out;
>> + goto lower_metapath;
>> }
>> - holesz += factor * zeroptrs;
>> - factor *= sdp->sd_inptrs;
>> - if (hgt && (mp->mp_list[hgt - 1] < mp_eof.mp_list[hgt -
>> 1]))
>> - (mp->mp_list[hgt - 1])++;
>> +fill_up_metapath:
>> + /* Fill up metapath. */
>
> Comment could be removed as the function name and the label are the same as
> the comment :)
Ok.
>> + ret = fillup_metapath(ip, mp, ip->i_height - 1);
>> + if (ret < 0)
>> + return ret;
>> + while (ret--) {
>> + hgt++;
>
>
> Could be taken out of the loop as hgt += ret
>
> The flow is a little hard to follow but I don't see any obvious problems.
> Has it been tested with directories as well as files?
No, I need to think the directory case through some more. The only
"hole" in directories is at the end of the file though.
> Reviewed-by: Andrew Price <anprice@redhat.com>
>
> Andy
>
>> + factor /= sdp->sd_inptrs;
>> + }
>> }
>> - return holesz << inode->i_blkbits;
>> +out:
>> + *pholesz = holesz << inode->i_blkbits;
>> + return 0;
>> }
>> static void gfs2_stuffed_iomap(struct inode *inode, struct iomap
>> *iomap)
>> @@ -804,7 +832,7 @@ int gfs2_iomap_begin(struct inode *inode, loff_t pos,
>> loff_t length,
>> if (pos >= size)
>> ret = -ENOENT;
>> else if (height <= ip->i_height)
>> - iomap->length = hole_size(inode, lblock, &mp);
>> + ret = hole_size(inode, lblock, &mp,
>> &iomap->length);
>> else
>> iomap->length = size - pos;
>> }
Thanks,
Andreas
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-04-11 13:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-10 17:39 [Cluster-devel] [PATCH] gfs2: hole_size improvement Andreas Gruenbacher
2018-04-10 18:22 ` Andreas Gruenbacher
2018-04-10 19:28 ` Andreas Gruenbacher
2018-04-11 13:00 ` Andrew Price
2018-04-11 13:37 ` Andreas Gruenbacher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.