* [Cluster-devel] [GFS2 PATCH] GFS2: Eliminate redundant ip->i_rgd in favor of ip->i_res.rs_rgd
@ 2017-06-30 16:40 Bob Peterson
2018-07-04 20:37 ` Andreas Gruenbacher
0 siblings, 1 reply; 4+ messages in thread
From: Bob Peterson @ 2017-06-30 16:40 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
Before this patch, GFS2 remembered the last rgrp it used by way of
a variable i_rgd. However, block allocations are made by way of a
reservations structure, ip->i_res, which also has a rgrp pointers.
These two values are at best redundant, and at worse, confuse the
logic and make GFS2 maintain and use two possibly opposing values.
The proper solution is to only use a single value. Since new block
allocations should be kept close to the last reservation, it makes
sense to only use the value in the reservations structure.
Therefore, this patch removes i_rgd.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index de42384..2ad003b 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -400,7 +400,6 @@ struct gfs2_inode {
struct gfs2_holder i_gh; /* for prepare/commit_write only */
struct gfs2_qadata *i_qadata; /* quota allocation data */
struct gfs2_blkreserv i_res; /* rgrp multi-block reservation */
- struct gfs2_rgrpd *i_rgd;
u64 i_goal; /* goal block for allocations */
struct rw_semaphore i_rw_mutex;
struct list_head i_ordered;
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 7c9b6d2..ad62bfb 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1971,8 +1971,9 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
return -EINVAL;
if (gfs2_rs_active(rs)) {
begin = rs->rs_rbm.rgd;
- } else if (ip->i_rgd && rgrp_contains_block(ip->i_rgd, ip->i_goal)) {
- rs->rs_rbm.rgd = begin = ip->i_rgd;
+ } else if (rs->rs_rbm.rgd &&
+ rgrp_contains_block(rs->rs_rbm.rgd, ip->i_goal)) {
+ begin = rs->rs_rbm.rgd;
} else {
check_and_update_goal(ip);
rs->rs_rbm.rgd = begin = gfs2_blk2rgrpd(sdp, ip->i_goal, 1);
@@ -2033,8 +2034,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
if (rs->rs_rbm.rgd->rd_free_clone >= ap->target ||
(loops == 2 && ap->min_target &&
rs->rs_rbm.rgd->rd_free_clone >= ap->min_target)) {
- ip->i_rgd = rs->rs_rbm.rgd;
- ap->allowed = ip->i_rgd->rd_free_clone;
+ ap->allowed = rs->rs_rbm.rgd->rd_free_clone;
return 0;
}
check_rgrp:
@@ -2311,7 +2311,7 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct buffer_head *dibh;
- struct gfs2_rbm rbm = { .rgd = ip->i_rgd, };
+ struct gfs2_rbm rbm = { .rgd = ip->i_res.rs_rbm.rgd, };
unsigned int ndata;
u64 block; /* block, within the file system scope */
int error;
@@ -2540,15 +2540,16 @@ void gfs2_rlist_add(struct gfs2_inode *ip, struct gfs2_rgrp_list *rlist,
if (gfs2_assert_warn(sdp, !rlist->rl_ghs))
return;
- if (ip->i_rgd && rgrp_contains_block(ip->i_rgd, block))
- rgd = ip->i_rgd;
+ if (ip->i_res.rs_rbm.rgd && rgrp_contains_block(ip->i_res.rs_rbm.rgd,
+ block))
+ rgd = ip->i_res.rs_rbm.rgd;
else
rgd = gfs2_blk2rgrpd(sdp, block, 1);
if (!rgd) {
fs_err(sdp, "rlist_add: no rgrp for block %llu\n", (unsigned long long)block);
return;
}
- ip->i_rgd = rgd;
+ ip->i_res.rs_rbm.rgd = rgd;
for (x = 0; x < rlist->rl_rgrps; x++)
if (rlist->rl_rgd[x] == rgd)
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index fdedec3..c83fd9c 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1660,7 +1660,6 @@ static struct inode *gfs2_alloc_inode(struct super_block *sb)
if (ip) {
ip->i_flags = 0;
ip->i_gl = NULL;
- ip->i_rgd = NULL;
memset(&ip->i_res, 0, sizeof(ip->i_res));
RB_CLEAR_NODE(&ip->i_res.rs_node);
ip->i_rahead = 0;
diff --git a/fs/gfs2/trans.h b/fs/gfs2/trans.h
index 1e6e7da..ec0a68d 100644
--- a/fs/gfs2/trans.h
+++ b/fs/gfs2/trans.h
@@ -30,9 +30,9 @@ struct gfs2_glock;
* block, or all of the blocks in the rg, whichever is smaller */
static inline unsigned int gfs2_rg_blocks(const struct gfs2_inode *ip, unsigned requested)
{
- if (requested < ip->i_rgd->rd_length)
+ if (requested < ip->i_res.rs_rbm.rgd->rd_length)
return requested + 1;
- return ip->i_rgd->rd_length;
+ return ip->i_res.rs_rbm.rgd->rd_length;
}
extern int gfs2_trans_begin(struct gfs2_sbd *sdp, unsigned int blocks,
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Cluster-devel] [GFS2 PATCH] GFS2: Eliminate redundant ip->i_rgd in favor of ip->i_res.rs_rgd
2017-06-30 16:40 [Cluster-devel] [GFS2 PATCH] GFS2: Eliminate redundant ip->i_rgd in favor of ip->i_res.rs_rgd Bob Peterson
@ 2018-07-04 20:37 ` Andreas Gruenbacher
2018-07-05 13:15 ` Bob Peterson
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Gruenbacher @ 2018-07-04 20:37 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 30 June 2017 at 18:40, Bob Peterson <rpeterso@redhat.com> wrote:
> Hi,
>
> Before this patch, GFS2 remembered the last rgrp it used by way of
> a variable i_rgd. However, block allocations are made by way of a
> reservations structure, ip->i_res, which also has a rgrp pointers.
> These two values are at best redundant, and at worse, confuse the
> logic and make GFS2 maintain and use two possibly opposing values.
> The proper solution is to only use a single value. Since new block
> allocations should be kept close to the last reservation, it makes
> sense to only use the value in the reservations structure.
> Therefore, this patch removes i_rgd.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index de42384..2ad003b 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -400,7 +400,6 @@ struct gfs2_inode {
> struct gfs2_holder i_gh; /* for prepare/commit_write only */
> struct gfs2_qadata *i_qadata; /* quota allocation data */
> struct gfs2_blkreserv i_res; /* rgrp multi-block reservation */
> - struct gfs2_rgrpd *i_rgd;
> u64 i_goal; /* goal block for allocations */
> struct rw_semaphore i_rw_mutex;
> struct list_head i_ordered;
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 7c9b6d2..ad62bfb 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1971,8 +1971,9 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
> return -EINVAL;
> if (gfs2_rs_active(rs)) {
> begin = rs->rs_rbm.rgd;
> - } else if (ip->i_rgd && rgrp_contains_block(ip->i_rgd, ip->i_goal)) {
> - rs->rs_rbm.rgd = begin = ip->i_rgd;
> + } else if (rs->rs_rbm.rgd &&
> + rgrp_contains_block(rs->rs_rbm.rgd, ip->i_goal)) {
> + begin = rs->rs_rbm.rgd;
> } else {
> check_and_update_goal(ip);
> rs->rs_rbm.rgd = begin = gfs2_blk2rgrpd(sdp, ip->i_goal, 1);
> @@ -2033,8 +2034,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
> if (rs->rs_rbm.rgd->rd_free_clone >= ap->target ||
> (loops == 2 && ap->min_target &&
> rs->rs_rbm.rgd->rd_free_clone >= ap->min_target)) {
> - ip->i_rgd = rs->rs_rbm.rgd;
> - ap->allowed = ip->i_rgd->rd_free_clone;
> + ap->allowed = rs->rs_rbm.rgd->rd_free_clone;
> return 0;
> }
> check_rgrp:
> @@ -2311,7 +2311,7 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
> {
> struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> struct buffer_head *dibh;
> - struct gfs2_rbm rbm = { .rgd = ip->i_rgd, };
> + struct gfs2_rbm rbm = { .rgd = ip->i_res.rs_rbm.rgd, };
> unsigned int ndata;
> u64 block; /* block, within the file system scope */
> int error;
> @@ -2540,15 +2540,16 @@ void gfs2_rlist_add(struct gfs2_inode *ip, struct gfs2_rgrp_list *rlist,
> if (gfs2_assert_warn(sdp, !rlist->rl_ghs))
> return;
>
> - if (ip->i_rgd && rgrp_contains_block(ip->i_rgd, block))
> - rgd = ip->i_rgd;
> + if (ip->i_res.rs_rbm.rgd && rgrp_contains_block(ip->i_res.rs_rbm.rgd,
> + block))
> + rgd = ip->i_res.rs_rbm.rgd;
> else
> rgd = gfs2_blk2rgrpd(sdp, block, 1);
> if (!rgd) {
> fs_err(sdp, "rlist_add: no rgrp for block %llu\n", (unsigned long long)block);
> return;
> }
> - ip->i_rgd = rgd;
> + ip->i_res.rs_rbm.rgd = rgd;
>
> for (x = 0; x < rlist->rl_rgrps; x++)
> if (rlist->rl_rgd[x] == rgd)
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index fdedec3..c83fd9c 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -1660,7 +1660,6 @@ static struct inode *gfs2_alloc_inode(struct super_block *sb)
> if (ip) {
> ip->i_flags = 0;
> ip->i_gl = NULL;
> - ip->i_rgd = NULL;
Oops, we want to initialize ip->i_res.rs_rbm.rgd here:
+ ip->i_res.rs_rbm.rgd = NULL;
> memset(&ip->i_res, 0, sizeof(ip->i_res));
> RB_CLEAR_NODE(&ip->i_res.rs_node);
> ip->i_rahead = 0;
> diff --git a/fs/gfs2/trans.h b/fs/gfs2/trans.h
> index 1e6e7da..ec0a68d 100644
> --- a/fs/gfs2/trans.h
> +++ b/fs/gfs2/trans.h
> @@ -30,9 +30,9 @@ struct gfs2_glock;
> * block, or all of the blocks in the rg, whichever is smaller */
> static inline unsigned int gfs2_rg_blocks(const struct gfs2_inode *ip, unsigned requested)
> {
> - if (requested < ip->i_rgd->rd_length)
> + if (requested < ip->i_res.rs_rbm.rgd->rd_length)
> return requested + 1;
> - return ip->i_rgd->rd_length;
> + return ip->i_res.rs_rbm.rgd->rd_length;
> }
>
> extern int gfs2_trans_begin(struct gfs2_sbd *sdp, unsigned int blocks,
Andreas
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Cluster-devel] [GFS2 PATCH] GFS2: Eliminate redundant ip->i_rgd in favor of ip->i_res.rs_rgd
2018-07-04 20:37 ` Andreas Gruenbacher
@ 2018-07-05 13:15 ` Bob Peterson
2018-07-05 14:28 ` Andreas Gruenbacher
0 siblings, 1 reply; 4+ messages in thread
From: Bob Peterson @ 2018-07-05 13:15 UTC (permalink / raw)
To: cluster-devel.redhat.com
----- Original Message -----
> On 30 June 2017 at 18:40, Bob Peterson <rpeterso@redhat.com> wrote:
> > Hi,
> >
> > Before this patch, GFS2 remembered the last rgrp it used by way of
> > a variable i_rgd. However, block allocations are made by way of a
> > reservations structure, ip->i_res, which also has a rgrp pointers.
> > These two values are at best redundant, and at worse, confuse the
> > logic and make GFS2 maintain and use two possibly opposing values.
> > The proper solution is to only use a single value. Since new block
> > allocations should be kept close to the last reservation, it makes
> > sense to only use the value in the reservations structure.
> > Therefore, this patch removes i_rgd.
> >
> > Signed-off-by: Bob Peterson <rpeterso@redhat.com>
(snip)
> > --- a/fs/gfs2/super.c
> > +++ b/fs/gfs2/super.c
> > @@ -1660,7 +1660,6 @@ static struct inode *gfs2_alloc_inode(struct
> > super_block *sb)
> > if (ip) {
> > ip->i_flags = 0;
> > ip->i_gl = NULL;
> > - ip->i_rgd = NULL;
>
> Oops, we want to initialize ip->i_res.rs_rbm.rgd here:
> + ip->i_res.rs_rbm.rgd = NULL;
>
> > memset(&ip->i_res, 0, sizeof(ip->i_res));
It shouldn't be necessary because of the memset below.
Bob Peterson
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Cluster-devel] [GFS2 PATCH] GFS2: Eliminate redundant ip->i_rgd in favor of ip->i_res.rs_rgd
2018-07-05 13:15 ` Bob Peterson
@ 2018-07-05 14:28 ` Andreas Gruenbacher
0 siblings, 0 replies; 4+ messages in thread
From: Andreas Gruenbacher @ 2018-07-05 14:28 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 5 July 2018 at 15:15, Bob Peterson <rpeterso@redhat.com> wrote:
> ----- Original Message -----
>> On 30 June 2017 at 18:40, Bob Peterson <rpeterso@redhat.com> wrote:
>> > Hi,
>> >
>> > Before this patch, GFS2 remembered the last rgrp it used by way of
>> > a variable i_rgd. However, block allocations are made by way of a
>> > reservations structure, ip->i_res, which also has a rgrp pointers.
>> > These two values are at best redundant, and at worse, confuse the
>> > logic and make GFS2 maintain and use two possibly opposing values.
>> > The proper solution is to only use a single value. Since new block
>> > allocations should be kept close to the last reservation, it makes
>> > sense to only use the value in the reservations structure.
>> > Therefore, this patch removes i_rgd.
>> >
>> > Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> (snip)
>> > --- a/fs/gfs2/super.c
>> > +++ b/fs/gfs2/super.c
>> > @@ -1660,7 +1660,6 @@ static struct inode *gfs2_alloc_inode(struct
>> > super_block *sb)
>> > if (ip) {
>> > ip->i_flags = 0;
>> > ip->i_gl = NULL;
>> > - ip->i_rgd = NULL;
>>
>> Oops, we want to initialize ip->i_res.rs_rbm.rgd here:
>> + ip->i_res.rs_rbm.rgd = NULL;
>>
>> > memset(&ip->i_res, 0, sizeof(ip->i_res));
>
> It shouldn't be necessary because of the memset below.
Right, sorry for the confusion.
Andreas
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-07-05 14:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-30 16:40 [Cluster-devel] [GFS2 PATCH] GFS2: Eliminate redundant ip->i_rgd in favor of ip->i_res.rs_rgd Bob Peterson
2018-07-04 20:37 ` Andreas Gruenbacher
2018-07-05 13:15 ` Bob Peterson
2018-07-05 14:28 ` 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.