* [Cluster-devel] [PATCH 1/4] GFS2: Reduce size of incore inode
2015-10-28 14:20 [Cluster-devel] [PATCH 0/4] Fix erroneous ETXTBSY problems with GFS2 Bob Peterson
@ 2015-10-28 14:20 ` Bob Peterson
2015-10-30 10:34 ` Steven Whitehouse
2015-10-28 14:20 ` [Cluster-devel] [PATCH 2/4] GFS2: Reduce inode size by using 32-bit i_generation Bob Peterson
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Bob Peterson @ 2015-10-28 14:20 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch makes no functional changes. Its goal is to reduce the
size of the gfs2 inode in memory by rearranging structures and
changing the size of some variables within the structure.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/file.c | 2 +-
fs/gfs2/glock.c | 10 +++++-----
fs/gfs2/glock.h | 26 +++++++++++++-------------
fs/gfs2/incore.h | 7 +++----
fs/gfs2/quota.c | 10 +++++-----
5 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 71cd138..7a99270 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -1018,7 +1018,7 @@ static int do_flock(struct file *file, int cmd, struct file_lock *fl)
struct gfs2_inode *ip = GFS2_I(file_inode(file));
struct gfs2_glock *gl;
unsigned int state;
- int flags;
+ u16 flags;
int error = 0;
int sleeptime;
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 9bd1244..f1590b5 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -446,7 +446,7 @@ __acquires(&gl->gl_spin)
{
const struct gfs2_glock_operations *glops = gl->gl_ops;
struct gfs2_sbd *sdp = gl->gl_name.ln_sbd;
- unsigned int lck_flags = gh ? gh->gh_flags : 0;
+ unsigned int lck_flags = (unsigned int)(gh ? gh->gh_flags : 0);
int ret;
lck_flags &= (LM_FLAG_TRY | LM_FLAG_TRY_1CB | LM_FLAG_NOEXP |
@@ -750,7 +750,7 @@ again:
*
*/
-void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, unsigned flags,
+void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, u16 flags,
struct gfs2_holder *gh)
{
INIT_LIST_HEAD(&gh->gh_list);
@@ -774,7 +774,7 @@ void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state, unsigned flags,
*
*/
-void gfs2_holder_reinit(unsigned int state, unsigned flags, struct gfs2_holder *gh)
+void gfs2_holder_reinit(unsigned int state, u16 flags, struct gfs2_holder *gh)
{
gh->gh_state = state;
gh->gh_flags = flags;
@@ -1080,7 +1080,7 @@ void gfs2_glock_dq_uninit(struct gfs2_holder *gh)
int gfs2_glock_nq_num(struct gfs2_sbd *sdp, u64 number,
const struct gfs2_glock_operations *glops,
- unsigned int state, int flags, struct gfs2_holder *gh)
+ unsigned int state, u16 flags, struct gfs2_holder *gh)
{
struct gfs2_glock *gl;
int error;
@@ -1539,7 +1539,7 @@ static const char *state2str(unsigned state)
return "??";
}
-static const char *hflags2str(char *buf, unsigned flags, unsigned long iflags)
+static const char *hflags2str(char *buf, u16 flags, unsigned long iflags)
{
char *p = buf;
if (flags & LM_FLAG_TRY)
diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
index 32572f7..268c733 100644
--- a/fs/gfs2/glock.h
+++ b/fs/gfs2/glock.h
@@ -79,15 +79,15 @@ enum {
* requested had acquired and released the lock.
*/
-#define LM_FLAG_TRY 0x00000001
-#define LM_FLAG_TRY_1CB 0x00000002
-#define LM_FLAG_NOEXP 0x00000004
-#define LM_FLAG_ANY 0x00000008
-#define LM_FLAG_PRIORITY 0x00000010
-#define GL_ASYNC 0x00000040
-#define GL_EXACT 0x00000080
-#define GL_SKIP 0x00000100
-#define GL_NOCACHE 0x00000400
+#define LM_FLAG_TRY 0x0001
+#define LM_FLAG_TRY_1CB 0x0002
+#define LM_FLAG_NOEXP 0x0004
+#define LM_FLAG_ANY 0x0008
+#define LM_FLAG_PRIORITY 0x0010
+#define GL_ASYNC 0x0040
+#define GL_EXACT 0x0080
+#define GL_SKIP 0x0100
+#define GL_NOCACHE 0x0400
/*
* lm_async_cb return flags
@@ -183,8 +183,8 @@ extern int gfs2_glock_get(struct gfs2_sbd *sdp, u64 number,
int create, struct gfs2_glock **glp);
extern void gfs2_glock_put(struct gfs2_glock *gl);
extern void gfs2_holder_init(struct gfs2_glock *gl, unsigned int state,
- unsigned flags, struct gfs2_holder *gh);
-extern void gfs2_holder_reinit(unsigned int state, unsigned flags,
+ u16 flags, struct gfs2_holder *gh);
+extern void gfs2_holder_reinit(unsigned int state, u16 flags,
struct gfs2_holder *gh);
extern void gfs2_holder_uninit(struct gfs2_holder *gh);
extern int gfs2_glock_nq(struct gfs2_holder *gh);
@@ -195,7 +195,7 @@ extern void gfs2_glock_dq_wait(struct gfs2_holder *gh);
extern void gfs2_glock_dq_uninit(struct gfs2_holder *gh);
extern int gfs2_glock_nq_num(struct gfs2_sbd *sdp, u64 number,
const struct gfs2_glock_operations *glops,
- unsigned int state, int flags,
+ unsigned int state, u16 flags,
struct gfs2_holder *gh);
extern int gfs2_glock_nq_m(unsigned int num_gh, struct gfs2_holder *ghs);
extern void gfs2_glock_dq_m(unsigned int num_gh, struct gfs2_holder *ghs);
@@ -215,7 +215,7 @@ void gfs2_print_dbg(struct seq_file *seq, const char *fmt, ...);
*/
static inline int gfs2_glock_nq_init(struct gfs2_glock *gl,
- unsigned int state, int flags,
+ unsigned int state, u16 flags,
struct gfs2_holder *gh)
{
int error;
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 121ed08..76c9c77 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -259,12 +259,12 @@ struct gfs2_holder {
struct gfs2_glock *gh_gl;
struct pid *gh_owner_pid;
- unsigned int gh_state;
- unsigned gh_flags;
int gh_error;
unsigned long gh_iflags; /* HIF_... */
unsigned long gh_ip;
+ u16 gh_flags;
+ u8 gh_state;
};
/* Number of quota types we support */
@@ -289,10 +289,9 @@ struct gfs2_blkreserv {
u32 rs_free; /* how many blocks are still free */
u64 rs_inum; /* Inode number for reservation */
- /* ancillary quota stuff */
struct gfs2_quota_data *rs_qa_qd[2 * GFS2_MAXQUOTAS];
struct gfs2_holder rs_qa_qd_ghs[2 * GFS2_MAXQUOTAS];
- unsigned int rs_qa_qd_num;
+ u32 rs_qa_qd_num;
};
/*
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 3a31226..6a9c847 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -587,7 +587,7 @@ out:
void gfs2_quota_unhold(struct gfs2_inode *ip)
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
- unsigned int x;
+ u32 x;
if (ip->i_res == NULL)
return;
@@ -1003,7 +1003,7 @@ int gfs2_quota_lock(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct gfs2_quota_data *qd;
- unsigned int x;
+ u32 x;
int error = 0;
error = gfs2_quota_hold(ip, uid, gid);
@@ -1076,7 +1076,7 @@ void gfs2_quota_unlock(struct gfs2_inode *ip)
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct gfs2_quota_data *qda[4];
unsigned int count = 0;
- unsigned int x;
+ u32 x;
int found;
if (!test_and_clear_bit(GIF_QD_LOCKED, &ip->i_flags))
@@ -1158,7 +1158,7 @@ int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid,
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct gfs2_quota_data *qd;
s64 value, warn, limit;
- unsigned int x;
+ u32 x;
int error = 0;
ap->allowed = UINT_MAX; /* Assume we are permitted a whole lot */
@@ -1216,7 +1216,7 @@ void gfs2_quota_change(struct gfs2_inode *ip, s64 change,
kuid_t uid, kgid_t gid)
{
struct gfs2_quota_data *qd;
- unsigned int x;
+ u32 x;
if (gfs2_assert_warn(GFS2_SB(&ip->i_inode), change))
return;
--
2.4.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Cluster-devel] [PATCH 1/4] GFS2: Reduce size of incore inode
2015-10-28 14:20 ` [Cluster-devel] [PATCH 1/4] GFS2: Reduce size of incore inode Bob Peterson
@ 2015-10-30 10:34 ` Steven Whitehouse
0 siblings, 0 replies; 11+ messages in thread
From: Steven Whitehouse @ 2015-10-30 10:34 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On 28/10/15 14:20, Bob Peterson wrote:
[snip]
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index 121ed08..76c9c77 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -259,12 +259,12 @@ struct gfs2_holder {
>
> struct gfs2_glock *gh_gl;
> struct pid *gh_owner_pid;
> - unsigned int gh_state;
> - unsigned gh_flags;
>
> int gh_error;
> unsigned long gh_iflags; /* HIF_... */
> unsigned long gh_ip;
> + u16 gh_flags;
> + u8 gh_state;
> };
>
Is this really much smaller after the change? Looks like it is probably
not packed correctly and there is likely to be a hole in it, depending
on where the 64 bit boundaries lie. Probably better off making the
gh_state and gh_flags 16 bit, but leaving them where they are in the
structure,
Steve.
> /* Number of quota types we support */
> @@ -289,10 +289,9 @@ struct gfs2_blkreserv {
> u32 rs_free; /* how many blocks are still free */
> u64 rs_inum; /* Inode number for reservation */
>
> - /* ancillary quota stuff */
> struct gfs2_quota_data *rs_qa_qd[2 * GFS2_MAXQUOTAS];
> struct gfs2_holder rs_qa_qd_ghs[2 * GFS2_MAXQUOTAS];
> - unsigned int rs_qa_qd_num;
> + u32 rs_qa_qd_num;
> };
Again, does this really save any space?
Steve.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Cluster-devel] [PATCH 2/4] GFS2: Reduce inode size by using 32-bit i_generation
2015-10-28 14:20 [Cluster-devel] [PATCH 0/4] Fix erroneous ETXTBSY problems with GFS2 Bob Peterson
2015-10-28 14:20 ` [Cluster-devel] [PATCH 1/4] GFS2: Reduce size of incore inode Bob Peterson
@ 2015-10-28 14:20 ` Bob Peterson
2015-10-29 17:21 ` Andrew Price
2015-10-30 10:42 ` [Cluster-devel] [PATCH 2/4] " Steven Whitehouse
2015-10-28 14:20 ` [Cluster-devel] [PATCH 3/4] GFS2: Extract quota data from reservations structure (revert 5407e24) Bob Peterson
` (2 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Bob Peterson @ 2015-10-28 14:20 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch removes variable i_generation from gfs2_inode. In its
place, we use inode->i_generation which is pretty much the same thing,
in 32-bits. The loss of 32 bits should not be a problem.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/glops.c | 2 +-
fs/gfs2/incore.h | 9 +++++++--
fs/gfs2/inode.c | 5 +++--
fs/gfs2/rgrp.c | 16 ++++++++--------
fs/gfs2/rgrp.h | 2 +-
fs/gfs2/super.c | 2 +-
include/uapi/linux/gfs2_ondisk.h | 25 +++++++++++++++++++++----
7 files changed, 42 insertions(+), 19 deletions(-)
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 1f6c9c3..79cd4a9 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -358,7 +358,7 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
ip->i_inode.i_ctime.tv_nsec = be32_to_cpu(str->di_ctime_nsec);
ip->i_goal = be64_to_cpu(str->di_goal_meta);
- ip->i_generation = be64_to_cpu(str->di_generation);
+ ip->i_inode.i_generation = be32_to_cpu(str->di_gen2);
ip->i_diskflags = be32_to_cpu(str->di_flags);
ip->i_eattr = be64_to_cpu(str->di_eattr);
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 76c9c77..8ca88b9 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -88,7 +88,13 @@ struct gfs2_rgrpd {
u32 rd_reserved; /* number of blocks reserved */
u32 rd_free_clone;
u32 rd_dinodes;
- u64 rd_igeneration;
+ union {
+ u64 rd_igeneration;
+ struct {
+ u32 rd_igen1;
+ u32 rd_igen2;
+ };
+ };
struct gfs2_bitmap *rd_bits;
struct gfs2_sbd *rd_sbd;
struct gfs2_rgrp_lvb *rd_rgl;
@@ -385,7 +391,6 @@ struct gfs2_inode {
struct inode i_inode;
u64 i_no_addr;
u64 i_no_formal_ino;
- u64 i_generation;
u64 i_eattr;
unsigned long i_flags; /* GIF_... */
struct gfs2_glock *i_gl; /* Move into i_gh? */
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 063fdfc..cc815ab 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -394,8 +394,9 @@ static int alloc_dinode(struct gfs2_inode *ip, u32 flags, unsigned *dblocks)
if (error)
goto out_ipreserv;
- error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1, &ip->i_generation);
- ip->i_no_formal_ino = ip->i_generation;
+ error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1,
+ &ip->i_inode.i_generation);
+ ip->i_no_formal_ino = ip->i_inode.i_generation;
ip->i_inode.i_ino = ip->i_no_addr;
ip->i_goal = ip->i_no_addr;
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 475985d..34c42a8 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1062,7 +1062,7 @@ static void gfs2_rgrp_in(struct gfs2_rgrpd *rgd, const void *buf)
rgd->rd_flags |= rg_flags;
rgd->rd_free = be32_to_cpu(str->rg_free);
rgd->rd_dinodes = be32_to_cpu(str->rg_dinodes);
- rgd->rd_igeneration = be64_to_cpu(str->rg_igeneration);
+ rgd->rd_igen2 = be32_to_cpu(str->rg_igen2);
}
static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
@@ -1073,7 +1073,7 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
str->rg_free = cpu_to_be32(rgd->rd_free);
str->rg_dinodes = cpu_to_be32(rgd->rd_dinodes);
str->__pad = cpu_to_be32(0);
- str->rg_igeneration = cpu_to_be64(rgd->rd_igeneration);
+ str->rg_igen2 = cpu_to_be64(rgd->rd_igen2);
memset(&str->rg_reserved, 0, sizeof(str->rg_reserved));
}
@@ -1084,7 +1084,7 @@ static int gfs2_rgrp_lvb_valid(struct gfs2_rgrpd *rgd)
if (rgl->rl_flags != str->rg_flags || rgl->rl_free != str->rg_free ||
rgl->rl_dinodes != str->rg_dinodes ||
- rgl->rl_igeneration != str->rg_igeneration)
+ rgl->rl_igen2 != str->rg_igen2)
return 0;
return 1;
}
@@ -1097,7 +1097,7 @@ static void gfs2_rgrp_ondisk2lvb(struct gfs2_rgrp_lvb *rgl, const void *buf)
rgl->rl_flags = str->rg_flags;
rgl->rl_free = str->rg_free;
rgl->rl_dinodes = str->rg_dinodes;
- rgl->rl_igeneration = str->rg_igeneration;
+ rgl->rl_igen2 = str->rg_igen2;
rgl->__pad = 0UL;
}
@@ -1229,7 +1229,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd)
rgd->rd_free = be32_to_cpu(rgd->rd_rgl->rl_free);
rgd->rd_free_clone = rgd->rd_free;
rgd->rd_dinodes = be32_to_cpu(rgd->rd_rgl->rl_dinodes);
- rgd->rd_igeneration = be64_to_cpu(rgd->rd_rgl->rl_igeneration);
+ rgd->rd_igen2 = be64_to_cpu(rgd->rd_rgl->rl_igen2);
return 0;
}
@@ -2334,7 +2334,7 @@ static void gfs2_set_alloc_start(struct gfs2_rbm *rbm,
*/
int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
- bool dinode, u64 *generation)
+ bool dinode, u32 *generation)
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct buffer_head *dibh;
@@ -2390,9 +2390,9 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
rbm.rgd->rd_free -= *nblocks;
if (dinode) {
rbm.rgd->rd_dinodes++;
- *generation = rbm.rgd->rd_igeneration++;
+ *generation = rbm.rgd->rd_igen2++;
if (*generation == 0)
- *generation = rbm.rgd->rd_igeneration++;
+ *generation = rbm.rgd->rd_igen2++;
}
gfs2_trans_add_meta(rbm.rgd->rd_gl, rbm.rgd->rd_bits[0].bi_bh);
diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
index c0ab33f..71c1a8e 100644
--- a/fs/gfs2/rgrp.h
+++ b/fs/gfs2/rgrp.h
@@ -47,7 +47,7 @@ extern int gfs2_inplace_reserve(struct gfs2_inode *ip,
extern void gfs2_inplace_release(struct gfs2_inode *ip);
extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
- bool dinode, u64 *generation);
+ bool dinode, u32 *generation);
extern int gfs2_rs_alloc(struct gfs2_inode *ip);
extern void gfs2_rs_deltree(struct gfs2_blkreserv *rs);
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 894fb01..928be2f 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -714,7 +714,7 @@ void gfs2_dinode_out(const struct gfs2_inode *ip, void *buf)
str->di_goal_meta = cpu_to_be64(ip->i_goal);
str->di_goal_data = cpu_to_be64(ip->i_goal);
- str->di_generation = cpu_to_be64(ip->i_generation);
+ str->di_gen2 = cpu_to_be64(ip->i_inode.i_generation);
str->di_flags = cpu_to_be32(ip->i_diskflags);
str->di_height = cpu_to_be16(ip->i_height);
diff --git a/include/uapi/linux/gfs2_ondisk.h b/include/uapi/linux/gfs2_ondisk.h
index 1a763ea..bef244e 100644
--- a/include/uapi/linux/gfs2_ondisk.h
+++ b/include/uapi/linux/gfs2_ondisk.h
@@ -175,7 +175,13 @@ struct gfs2_rgrp_lvb {
__be32 rl_flags;
__be32 rl_free;
__be32 rl_dinodes;
- __be64 rl_igeneration;
+ union {
+ __be64 rl_igeneration;
+ struct {
+ __be32 rl_igen1;
+ __be32 rl_igen2;
+ };
+ };
__be32 rl_unlinked;
__be32 __pad;
};
@@ -187,7 +193,13 @@ struct gfs2_rgrp {
__be32 rg_free;
__be32 rg_dinodes;
__be32 __pad;
- __be64 rg_igeneration;
+ union {
+ __be64 rg_igeneration;
+ struct {
+ __be32 rg_igen1;
+ __be32 rg_igen2;
+ };
+ };
__u8 rg_reserved[80]; /* Several fields from gfs1 now reserved */
};
@@ -268,8 +280,13 @@ struct gfs2_dinode {
*/
__be64 di_goal_meta; /* rgrp to alloc from next */
__be64 di_goal_data; /* data block goal */
- __be64 di_generation; /* generation number for NFS */
-
+ union {
+ __be64 di_generation; /* generation number for NFS */
+ struct {
+ __be32 di_gen1;
+ __be32 di_gen2;
+ };
+ };
__be32 di_flags; /* GFS2_DIF_... */
__be32 di_payload_format; /* GFS2_FORMAT_... */
__u16 __pad1; /* Was ditype in gfs1 */
--
2.4.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Cluster-devel] [PATCH 2/4] GFS2: Reduce inode size by using 32-bit i_generation
2015-10-28 14:20 ` [Cluster-devel] [PATCH 2/4] GFS2: Reduce inode size by using 32-bit i_generation Bob Peterson
@ 2015-10-29 17:21 ` Andrew Price
2015-10-29 18:39 ` Bob Peterson
2015-10-30 10:42 ` [Cluster-devel] [PATCH 2/4] " Steven Whitehouse
1 sibling, 1 reply; 11+ messages in thread
From: Andrew Price @ 2015-10-29 17:21 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi Bob,
On 28/10/15 14:20, Bob Peterson wrote:
> This patch removes variable i_generation from gfs2_inode. In its
> place, we use inode->i_generation which is pretty much the same thing,
> in 32-bits. The loss of 32 bits should not be a problem.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> fs/gfs2/glops.c | 2 +-
> fs/gfs2/incore.h | 9 +++++++--
> fs/gfs2/inode.c | 5 +++--
> fs/gfs2/rgrp.c | 16 ++++++++--------
> fs/gfs2/rgrp.h | 2 +-
> fs/gfs2/super.c | 2 +-
> include/uapi/linux/gfs2_ondisk.h | 25 +++++++++++++++++++++----
> 7 files changed, 42 insertions(+), 19 deletions(-)
>
> diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> index 1f6c9c3..79cd4a9 100644
> --- a/fs/gfs2/glops.c
> +++ b/fs/gfs2/glops.c
> @@ -358,7 +358,7 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
> ip->i_inode.i_ctime.tv_nsec = be32_to_cpu(str->di_ctime_nsec);
>
> ip->i_goal = be64_to_cpu(str->di_goal_meta);
> - ip->i_generation = be64_to_cpu(str->di_generation);
> + ip->i_inode.i_generation = be32_to_cpu(str->di_gen2);
>
> ip->i_diskflags = be32_to_cpu(str->di_flags);
> ip->i_eattr = be64_to_cpu(str->di_eattr);
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index 76c9c77..8ca88b9 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -88,7 +88,13 @@ struct gfs2_rgrpd {
> u32 rd_reserved; /* number of blocks reserved */
> u32 rd_free_clone;
> u32 rd_dinodes;
> - u64 rd_igeneration;
> + union {
> + u64 rd_igeneration;
> + struct {
> + u32 rd_igen1;
> + u32 rd_igen2;
> + };
> + };
Is this union necessary given that it's not an on-disk structure?
> struct gfs2_bitmap *rd_bits;
> struct gfs2_sbd *rd_sbd;
> struct gfs2_rgrp_lvb *rd_rgl;
> @@ -385,7 +391,6 @@ struct gfs2_inode {
> struct inode i_inode;
> u64 i_no_addr;
> u64 i_no_formal_ino;
> - u64 i_generation;
> u64 i_eattr;
> unsigned long i_flags; /* GIF_... */
> struct gfs2_glock *i_gl; /* Move into i_gh? */
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index 063fdfc..cc815ab 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -394,8 +394,9 @@ static int alloc_dinode(struct gfs2_inode *ip, u32 flags, unsigned *dblocks)
> if (error)
> goto out_ipreserv;
>
> - error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1, &ip->i_generation);
> - ip->i_no_formal_ino = ip->i_generation;
> + error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1,
> + &ip->i_inode.i_generation);
> + ip->i_no_formal_ino = ip->i_inode.i_generation;
> ip->i_inode.i_ino = ip->i_no_addr;
> ip->i_goal = ip->i_no_addr;
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 475985d..34c42a8 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1062,7 +1062,7 @@ static void gfs2_rgrp_in(struct gfs2_rgrpd *rgd, const void *buf)
> rgd->rd_flags |= rg_flags;
> rgd->rd_free = be32_to_cpu(str->rg_free);
> rgd->rd_dinodes = be32_to_cpu(str->rg_dinodes);
> - rgd->rd_igeneration = be64_to_cpu(str->rg_igeneration);
> + rgd->rd_igen2 = be32_to_cpu(str->rg_igen2);
> }
>
> static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
> @@ -1073,7 +1073,7 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
> str->rg_free = cpu_to_be32(rgd->rd_free);
> str->rg_dinodes = cpu_to_be32(rgd->rd_dinodes);
> str->__pad = cpu_to_be32(0);
> - str->rg_igeneration = cpu_to_be64(rgd->rd_igeneration);
> + str->rg_igen2 = cpu_to_be64(rgd->rd_igen2);
Probably should be using cpu_to_be32()
> memset(&str->rg_reserved, 0, sizeof(str->rg_reserved));
> }
>
> @@ -1084,7 +1084,7 @@ static int gfs2_rgrp_lvb_valid(struct gfs2_rgrpd *rgd)
>
> if (rgl->rl_flags != str->rg_flags || rgl->rl_free != str->rg_free ||
> rgl->rl_dinodes != str->rg_dinodes ||
> - rgl->rl_igeneration != str->rg_igeneration)
> + rgl->rl_igen2 != str->rg_igen2)
> return 0;
> return 1;
> }
> @@ -1097,7 +1097,7 @@ static void gfs2_rgrp_ondisk2lvb(struct gfs2_rgrp_lvb *rgl, const void *buf)
> rgl->rl_flags = str->rg_flags;
> rgl->rl_free = str->rg_free;
> rgl->rl_dinodes = str->rg_dinodes;
> - rgl->rl_igeneration = str->rg_igeneration;
> + rgl->rl_igen2 = str->rg_igen2;
> rgl->__pad = 0UL;
> }
>
> @@ -1229,7 +1229,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd)
> rgd->rd_free = be32_to_cpu(rgd->rd_rgl->rl_free);
> rgd->rd_free_clone = rgd->rd_free;
> rgd->rd_dinodes = be32_to_cpu(rgd->rd_rgl->rl_dinodes);
> - rgd->rd_igeneration = be64_to_cpu(rgd->rd_rgl->rl_igeneration);
> + rgd->rd_igen2 = be64_to_cpu(rgd->rd_rgl->rl_igen2);
Probably should be using be32_to_cpu()
> return 0;
> }
>
> @@ -2334,7 +2334,7 @@ static void gfs2_set_alloc_start(struct gfs2_rbm *rbm,
> */
>
> int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
> - bool dinode, u64 *generation)
> + bool dinode, u32 *generation)
> {
> struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> struct buffer_head *dibh;
> @@ -2390,9 +2390,9 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
> rbm.rgd->rd_free -= *nblocks;
> if (dinode) {
> rbm.rgd->rd_dinodes++;
> - *generation = rbm.rgd->rd_igeneration++;
> + *generation = rbm.rgd->rd_igen2++;
> if (*generation == 0)
> - *generation = rbm.rgd->rd_igeneration++;
> + *generation = rbm.rgd->rd_igen2++;
> }
>
> gfs2_trans_add_meta(rbm.rgd->rd_gl, rbm.rgd->rd_bits[0].bi_bh);
> diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
> index c0ab33f..71c1a8e 100644
> --- a/fs/gfs2/rgrp.h
> +++ b/fs/gfs2/rgrp.h
> @@ -47,7 +47,7 @@ extern int gfs2_inplace_reserve(struct gfs2_inode *ip,
> extern void gfs2_inplace_release(struct gfs2_inode *ip);
>
> extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> - bool dinode, u64 *generation);
> + bool dinode, u32 *generation);
>
> extern int gfs2_rs_alloc(struct gfs2_inode *ip);
> extern void gfs2_rs_deltree(struct gfs2_blkreserv *rs);
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 894fb01..928be2f 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -714,7 +714,7 @@ void gfs2_dinode_out(const struct gfs2_inode *ip, void *buf)
>
> str->di_goal_meta = cpu_to_be64(ip->i_goal);
> str->di_goal_data = cpu_to_be64(ip->i_goal);
> - str->di_generation = cpu_to_be64(ip->i_generation);
> + str->di_gen2 = cpu_to_be64(ip->i_inode.i_generation);
Again here.
>
> str->di_flags = cpu_to_be32(ip->i_diskflags);
> str->di_height = cpu_to_be16(ip->i_height);
> diff --git a/include/uapi/linux/gfs2_ondisk.h b/include/uapi/linux/gfs2_ondisk.h
> index 1a763ea..bef244e 100644
> --- a/include/uapi/linux/gfs2_ondisk.h
> +++ b/include/uapi/linux/gfs2_ondisk.h
> @@ -175,7 +175,13 @@ struct gfs2_rgrp_lvb {
> __be32 rl_flags;
> __be32 rl_free;
> __be32 rl_dinodes;
> - __be64 rl_igeneration;
> + union {
> + __be64 rl_igeneration;
> + struct {
> + __be32 rl_igen1;
> + __be32 rl_igen2;
> + };
> + };
> __be32 rl_unlinked;
> __be32 __pad;
> };
> @@ -187,7 +193,13 @@ struct gfs2_rgrp {
> __be32 rg_free;
> __be32 rg_dinodes;
> __be32 __pad;
> - __be64 rg_igeneration;
> + union {
> + __be64 rg_igeneration;
> + struct {
> + __be32 rg_igen1;
> + __be32 rg_igen2;
> + };
> + };
>
> __u8 rg_reserved[80]; /* Several fields from gfs1 now reserved */
> };
> @@ -268,8 +280,13 @@ struct gfs2_dinode {
> */
> __be64 di_goal_meta; /* rgrp to alloc from next */
> __be64 di_goal_data; /* data block goal */
> - __be64 di_generation; /* generation number for NFS */
> -
> + union {
> + __be64 di_generation; /* generation number for NFS */
> + struct {
> + __be32 di_gen1;
> + __be32 di_gen2;
> + };
> + };
> __be32 di_flags; /* GFS2_DIF_... */
> __be32 di_payload_format; /* GFS2_FORMAT_... */
> __u16 __pad1; /* Was ditype in gfs1 */
>
It would be useful to have some description of how the two u32s are
meant to be used now that the u64 is split. Although, I notice that only
the *gen2 fields are being used now, so perhaps the *gen2 fields could
be given a more descriptive name and the *gen1 fields could follow the
__pad convention instead.
I assume there are no endianness problems here as the generation number
is always big-endian on-disk so it will always be found in *gen2.
I'm not too familiar with how the generation number is used so I hope
it's also safe to assume that inode->i_generation will never be bumped
up to 64 bits to avoid some kind of i_generation-Y2K bug :)
Cheers,
Andy
^ permalink raw reply [flat|nested] 11+ messages in thread* [Cluster-devel] [PATCH 2/4] GFS2: Reduce inode size by using 32-bit i_generation
2015-10-29 17:21 ` Andrew Price
@ 2015-10-29 18:39 ` Bob Peterson
2015-10-29 18:43 ` [Cluster-devel] [GFS2 PATCH 2/4 v2] " Bob Peterson
0 siblings, 1 reply; 11+ messages in thread
From: Bob Peterson @ 2015-10-29 18:39 UTC (permalink / raw)
To: cluster-devel.redhat.com
----- Original Message -----
> Hi Bob,
>
> On 28/10/15 14:20, Bob Peterson wrote:
> > This patch removes variable i_generation from gfs2_inode. In its
> > place, we use inode->i_generation which is pretty much the same thing,
> > in 32-bits. The loss of 32 bits should not be a problem.
> >
> > Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> > ---
> > fs/gfs2/glops.c | 2 +-
> > fs/gfs2/incore.h | 9 +++++++--
> > fs/gfs2/inode.c | 5 +++--
> > fs/gfs2/rgrp.c | 16 ++++++++--------
> > fs/gfs2/rgrp.h | 2 +-
> > fs/gfs2/super.c | 2 +-
> > include/uapi/linux/gfs2_ondisk.h | 25 +++++++++++++++++++++----
> > 7 files changed, 42 insertions(+), 19 deletions(-)
> >
> > diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> > index 1f6c9c3..79cd4a9 100644
> > --- a/fs/gfs2/glops.c
> > +++ b/fs/gfs2/glops.c
> > @@ -358,7 +358,7 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const
> > void *buf)
> > ip->i_inode.i_ctime.tv_nsec = be32_to_cpu(str->di_ctime_nsec);
> >
> > ip->i_goal = be64_to_cpu(str->di_goal_meta);
> > - ip->i_generation = be64_to_cpu(str->di_generation);
> > + ip->i_inode.i_generation = be32_to_cpu(str->di_gen2);
> >
> > ip->i_diskflags = be32_to_cpu(str->di_flags);
> > ip->i_eattr = be64_to_cpu(str->di_eattr);
> > diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> > index 76c9c77..8ca88b9 100644
> > --- a/fs/gfs2/incore.h
> > +++ b/fs/gfs2/incore.h
> > @@ -88,7 +88,13 @@ struct gfs2_rgrpd {
> > u32 rd_reserved; /* number of blocks reserved */
> > u32 rd_free_clone;
> > u32 rd_dinodes;
> > - u64 rd_igeneration;
> > + union {
> > + u64 rd_igeneration;
> > + struct {
> > + u32 rd_igen1;
> > + u32 rd_igen2;
> > + };
> > + };
>
> Is this union necessary given that it's not an on-disk structure?
>
> > struct gfs2_bitmap *rd_bits;
> > struct gfs2_sbd *rd_sbd;
> > struct gfs2_rgrp_lvb *rd_rgl;
> > @@ -385,7 +391,6 @@ struct gfs2_inode {
> > struct inode i_inode;
> > u64 i_no_addr;
> > u64 i_no_formal_ino;
> > - u64 i_generation;
> > u64 i_eattr;
> > unsigned long i_flags; /* GIF_... */
> > struct gfs2_glock *i_gl; /* Move into i_gh? */
> > diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> > index 063fdfc..cc815ab 100644
> > --- a/fs/gfs2/inode.c
> > +++ b/fs/gfs2/inode.c
> > @@ -394,8 +394,9 @@ static int alloc_dinode(struct gfs2_inode *ip, u32
> > flags, unsigned *dblocks)
> > if (error)
> > goto out_ipreserv;
> >
> > - error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1,
> > &ip->i_generation);
> > - ip->i_no_formal_ino = ip->i_generation;
> > + error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1,
> > + &ip->i_inode.i_generation);
> > + ip->i_no_formal_ino = ip->i_inode.i_generation;
> > ip->i_inode.i_ino = ip->i_no_addr;
> > ip->i_goal = ip->i_no_addr;
> >
> > diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> > index 475985d..34c42a8 100644
> > --- a/fs/gfs2/rgrp.c
> > +++ b/fs/gfs2/rgrp.c
> > @@ -1062,7 +1062,7 @@ static void gfs2_rgrp_in(struct gfs2_rgrpd *rgd,
> > const void *buf)
> > rgd->rd_flags |= rg_flags;
> > rgd->rd_free = be32_to_cpu(str->rg_free);
> > rgd->rd_dinodes = be32_to_cpu(str->rg_dinodes);
> > - rgd->rd_igeneration = be64_to_cpu(str->rg_igeneration);
> > + rgd->rd_igen2 = be32_to_cpu(str->rg_igen2);
> > }
> >
> > static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
> > @@ -1073,7 +1073,7 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd,
> > void *buf)
> > str->rg_free = cpu_to_be32(rgd->rd_free);
> > str->rg_dinodes = cpu_to_be32(rgd->rd_dinodes);
> > str->__pad = cpu_to_be32(0);
> > - str->rg_igeneration = cpu_to_be64(rgd->rd_igeneration);
> > + str->rg_igen2 = cpu_to_be64(rgd->rd_igen2);
>
> Probably should be using cpu_to_be32()
>
> > memset(&str->rg_reserved, 0, sizeof(str->rg_reserved));
> > }
> >
> > @@ -1084,7 +1084,7 @@ static int gfs2_rgrp_lvb_valid(struct gfs2_rgrpd
> > *rgd)
> >
> > if (rgl->rl_flags != str->rg_flags || rgl->rl_free != str->rg_free ||
> > rgl->rl_dinodes != str->rg_dinodes ||
> > - rgl->rl_igeneration != str->rg_igeneration)
> > + rgl->rl_igen2 != str->rg_igen2)
> > return 0;
> > return 1;
> > }
> > @@ -1097,7 +1097,7 @@ static void gfs2_rgrp_ondisk2lvb(struct gfs2_rgrp_lvb
> > *rgl, const void *buf)
> > rgl->rl_flags = str->rg_flags;
> > rgl->rl_free = str->rg_free;
> > rgl->rl_dinodes = str->rg_dinodes;
> > - rgl->rl_igeneration = str->rg_igeneration;
> > + rgl->rl_igen2 = str->rg_igen2;
> > rgl->__pad = 0UL;
> > }
> >
> > @@ -1229,7 +1229,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd)
> > rgd->rd_free = be32_to_cpu(rgd->rd_rgl->rl_free);
> > rgd->rd_free_clone = rgd->rd_free;
> > rgd->rd_dinodes = be32_to_cpu(rgd->rd_rgl->rl_dinodes);
> > - rgd->rd_igeneration = be64_to_cpu(rgd->rd_rgl->rl_igeneration);
> > + rgd->rd_igen2 = be64_to_cpu(rgd->rd_rgl->rl_igen2);
>
> Probably should be using be32_to_cpu()
>
> > return 0;
> > }
> >
> > @@ -2334,7 +2334,7 @@ static void gfs2_set_alloc_start(struct gfs2_rbm
> > *rbm,
> > */
> >
> > int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int
> > *nblocks,
> > - bool dinode, u64 *generation)
> > + bool dinode, u32 *generation)
> > {
> > struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> > struct buffer_head *dibh;
> > @@ -2390,9 +2390,9 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn,
> > unsigned int *nblocks,
> > rbm.rgd->rd_free -= *nblocks;
> > if (dinode) {
> > rbm.rgd->rd_dinodes++;
> > - *generation = rbm.rgd->rd_igeneration++;
> > + *generation = rbm.rgd->rd_igen2++;
> > if (*generation == 0)
> > - *generation = rbm.rgd->rd_igeneration++;
> > + *generation = rbm.rgd->rd_igen2++;
> > }
> >
> > gfs2_trans_add_meta(rbm.rgd->rd_gl, rbm.rgd->rd_bits[0].bi_bh);
> > diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
> > index c0ab33f..71c1a8e 100644
> > --- a/fs/gfs2/rgrp.h
> > +++ b/fs/gfs2/rgrp.h
> > @@ -47,7 +47,7 @@ extern int gfs2_inplace_reserve(struct gfs2_inode *ip,
> > extern void gfs2_inplace_release(struct gfs2_inode *ip);
> >
> > extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int
> > *n,
> > - bool dinode, u64 *generation);
> > + bool dinode, u32 *generation);
> >
> > extern int gfs2_rs_alloc(struct gfs2_inode *ip);
> > extern void gfs2_rs_deltree(struct gfs2_blkreserv *rs);
> > diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> > index 894fb01..928be2f 100644
> > --- a/fs/gfs2/super.c
> > +++ b/fs/gfs2/super.c
> > @@ -714,7 +714,7 @@ void gfs2_dinode_out(const struct gfs2_inode *ip, void
> > *buf)
> >
> > str->di_goal_meta = cpu_to_be64(ip->i_goal);
> > str->di_goal_data = cpu_to_be64(ip->i_goal);
> > - str->di_generation = cpu_to_be64(ip->i_generation);
> > + str->di_gen2 = cpu_to_be64(ip->i_inode.i_generation);
>
> Again here.
>
> >
> > str->di_flags = cpu_to_be32(ip->i_diskflags);
> > str->di_height = cpu_to_be16(ip->i_height);
> > diff --git a/include/uapi/linux/gfs2_ondisk.h
> > b/include/uapi/linux/gfs2_ondisk.h
> > index 1a763ea..bef244e 100644
> > --- a/include/uapi/linux/gfs2_ondisk.h
> > +++ b/include/uapi/linux/gfs2_ondisk.h
> > @@ -175,7 +175,13 @@ struct gfs2_rgrp_lvb {
> > __be32 rl_flags;
> > __be32 rl_free;
> > __be32 rl_dinodes;
> > - __be64 rl_igeneration;
> > + union {
> > + __be64 rl_igeneration;
> > + struct {
> > + __be32 rl_igen1;
> > + __be32 rl_igen2;
> > + };
> > + };
> > __be32 rl_unlinked;
> > __be32 __pad;
> > };
> > @@ -187,7 +193,13 @@ struct gfs2_rgrp {
> > __be32 rg_free;
> > __be32 rg_dinodes;
> > __be32 __pad;
> > - __be64 rg_igeneration;
> > + union {
> > + __be64 rg_igeneration;
> > + struct {
> > + __be32 rg_igen1;
> > + __be32 rg_igen2;
> > + };
> > + };
> >
> > __u8 rg_reserved[80]; /* Several fields from gfs1 now reserved */
> > };
> > @@ -268,8 +280,13 @@ struct gfs2_dinode {
> > */
> > __be64 di_goal_meta; /* rgrp to alloc from next */
> > __be64 di_goal_data; /* data block goal */
> > - __be64 di_generation; /* generation number for NFS */
> > -
> > + union {
> > + __be64 di_generation; /* generation number for NFS */
> > + struct {
> > + __be32 di_gen1;
> > + __be32 di_gen2;
> > + };
> > + };
> > __be32 di_flags; /* GFS2_DIF_... */
> > __be32 di_payload_format; /* GFS2_FORMAT_... */
> > __u16 __pad1; /* Was ditype in gfs1 */
> >
>
> It would be useful to have some description of how the two u32s are
> meant to be used now that the u64 is split. Although, I notice that only
> the *gen2 fields are being used now, so perhaps the *gen2 fields could
> be given a more descriptive name and the *gen1 fields could follow the
> __pad convention instead.
>
> I assume there are no endianness problems here as the generation number
> is always big-endian on-disk so it will always be found in *gen2.
>
> I'm not too familiar with how the generation number is used so I hope
> it's also safe to assume that inode->i_generation will never be bumped
> up to 64 bits to avoid some kind of i_generation-Y2K bug :)
>
> Cheers,
> Andy
>
Hi Andy,
Thanks for reviewing this. Good catches. I'll post a version 2 patch
to replace it shortly.
Regards,
Bob Peterson
Red Hat File Systems
^ permalink raw reply [flat|nested] 11+ messages in thread* [Cluster-devel] [GFS2 PATCH 2/4 v2] GFS2: Reduce inode size by using 32-bit i_generation
2015-10-29 18:39 ` Bob Peterson
@ 2015-10-29 18:43 ` Bob Peterson
0 siblings, 0 replies; 11+ messages in thread
From: Bob Peterson @ 2015-10-29 18:43 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
This patch fixes the problems Andy Price pointed out.
I also added more explanation to the patch text, as requested.
Regards,
Bob Peterson
Red Hat File Systems
---
GFS2: Reduce inode size by using 32-bit i_generation
This patch removes variable i_generation from gfs2_inode. In its
place, we use inode->i_generation which is pretty much the same thing,
in 32-bits. The loss of 32 bits should not be a problem.
The generation number is kept 64 bits for the on-disk structures,
since that cannot change, so only the on-disk structures are changed.
For inodes, the generation number is only used for debugging purposes:
if there is file system corruption, we may be able to track back the
revision numbers through the journals to determine the cause.
The patch also does the same thing for resource group generation
numbers, although there's another purpose besides debugging: For
resource groups, the generation number is also used to determine if
a resource group has changed on a different node based on whether
the generation number has changed in the lvb (forcing an invalidation
and re-reading the media). The change from 64 to 32 bits should not
be a problem there either, since a wrap-around in the generation
number still counts as a change of the rgrp, and therefore it will be
handled the same way. Note how function gfs2_rgrp_lvb_valid() checks
the lvb is valid with "rgl->rl_igen2 != str->rg_igen2". If this was
"<=" or ">=" we'd have a problem, but "!=" is safe.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index f348cfb..017a8b3 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -358,7 +358,7 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
ip->i_inode.i_ctime.tv_nsec = be32_to_cpu(str->di_ctime_nsec);
ip->i_goal = be64_to_cpu(str->di_goal_meta);
- ip->i_generation = be64_to_cpu(str->di_generation);
+ ip->i_inode.i_generation = be32_to_cpu(str->di_gen2);
ip->i_diskflags = be32_to_cpu(str->di_flags);
ip->i_eattr = be64_to_cpu(str->di_eattr);
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 7113284..fcdacf1 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -88,7 +88,7 @@ struct gfs2_rgrpd {
u32 rd_reserved; /* number of blocks reserved */
u32 rd_free_clone;
u32 rd_dinodes;
- u64 rd_igeneration;
+ u32 rd_igeneration;
struct gfs2_bitmap *rd_bits;
struct gfs2_sbd *rd_sbd;
struct gfs2_rgrp_lvb *rd_rgl;
@@ -384,7 +384,6 @@ struct gfs2_inode {
struct inode i_inode;
u64 i_no_addr;
u64 i_no_formal_ino;
- u64 i_generation;
u64 i_eattr;
unsigned long i_flags; /* GIF_... */
struct gfs2_glock *i_gl; /* Move into i_gh? */
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 063fdfc..cc815ab 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -394,8 +394,9 @@ static int alloc_dinode(struct gfs2_inode *ip, u32 flags, unsigned *dblocks)
if (error)
goto out_ipreserv;
- error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1, &ip->i_generation);
- ip->i_no_formal_ino = ip->i_generation;
+ error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1,
+ &ip->i_inode.i_generation);
+ ip->i_no_formal_ino = ip->i_inode.i_generation;
ip->i_inode.i_ino = ip->i_no_addr;
ip->i_goal = ip->i_no_addr;
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index d29dd0c..682f6a3 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1062,7 +1062,7 @@ static void gfs2_rgrp_in(struct gfs2_rgrpd *rgd, const void *buf)
rgd->rd_flags |= rg_flags;
rgd->rd_free = be32_to_cpu(str->rg_free);
rgd->rd_dinodes = be32_to_cpu(str->rg_dinodes);
- rgd->rd_igeneration = be64_to_cpu(str->rg_igeneration);
+ rgd->rd_igeneration = be32_to_cpu(str->rg_igen2);
}
static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
@@ -1073,7 +1073,7 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
str->rg_free = cpu_to_be32(rgd->rd_free);
str->rg_dinodes = cpu_to_be32(rgd->rd_dinodes);
str->__pad = cpu_to_be32(0);
- str->rg_igeneration = cpu_to_be64(rgd->rd_igeneration);
+ str->rg_igen2 = cpu_to_be32(rgd->rd_igeneration);
memset(&str->rg_reserved, 0, sizeof(str->rg_reserved));
}
@@ -1084,7 +1084,7 @@ static int gfs2_rgrp_lvb_valid(struct gfs2_rgrpd *rgd)
if (rgl->rl_flags != str->rg_flags || rgl->rl_free != str->rg_free ||
rgl->rl_dinodes != str->rg_dinodes ||
- rgl->rl_igeneration != str->rg_igeneration)
+ rgl->rl_igen2 != str->rg_igen2)
return 0;
return 1;
}
@@ -1097,7 +1097,7 @@ static void gfs2_rgrp_ondisk2lvb(struct gfs2_rgrp_lvb *rgl, const void *buf)
rgl->rl_flags = str->rg_flags;
rgl->rl_free = str->rg_free;
rgl->rl_dinodes = str->rg_dinodes;
- rgl->rl_igeneration = str->rg_igeneration;
+ rgl->rl_igen2 = str->rg_igen2;
rgl->__pad = 0UL;
}
@@ -1229,7 +1229,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd)
rgd->rd_free = be32_to_cpu(rgd->rd_rgl->rl_free);
rgd->rd_free_clone = rgd->rd_free;
rgd->rd_dinodes = be32_to_cpu(rgd->rd_rgl->rl_dinodes);
- rgd->rd_igeneration = be64_to_cpu(rgd->rd_rgl->rl_igeneration);
+ rgd->rd_igeneration = be32_to_cpu(rgd->rd_rgl->rl_igen2);
return 0;
}
@@ -2334,7 +2334,7 @@ static void gfs2_set_alloc_start(struct gfs2_rbm *rbm,
*/
int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
- bool dinode, u64 *generation)
+ bool dinode, u32 *generation)
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct buffer_head *dibh;
diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
index c0ab33f..71c1a8e 100644
--- a/fs/gfs2/rgrp.h
+++ b/fs/gfs2/rgrp.h
@@ -47,7 +47,7 @@ extern int gfs2_inplace_reserve(struct gfs2_inode *ip,
extern void gfs2_inplace_release(struct gfs2_inode *ip);
extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
- bool dinode, u64 *generation);
+ bool dinode, u32 *generation);
extern int gfs2_rs_alloc(struct gfs2_inode *ip);
extern void gfs2_rs_deltree(struct gfs2_blkreserv *rs);
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 894fb01..45a5566 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -714,7 +714,7 @@ void gfs2_dinode_out(const struct gfs2_inode *ip, void *buf)
str->di_goal_meta = cpu_to_be64(ip->i_goal);
str->di_goal_data = cpu_to_be64(ip->i_goal);
- str->di_generation = cpu_to_be64(ip->i_generation);
+ str->di_gen2 = cpu_to_be32(ip->i_inode.i_generation);
str->di_flags = cpu_to_be32(ip->i_diskflags);
str->di_height = cpu_to_be16(ip->i_height);
diff --git a/include/uapi/linux/gfs2_ondisk.h b/include/uapi/linux/gfs2_ondisk.h
index 1a763ea..62e469d 100644
--- a/include/uapi/linux/gfs2_ondisk.h
+++ b/include/uapi/linux/gfs2_ondisk.h
@@ -175,7 +175,13 @@ struct gfs2_rgrp_lvb {
__be32 rl_flags;
__be32 rl_free;
__be32 rl_dinodes;
- __be64 rl_igeneration;
+ union {
+ __be64 rl_igeneration;
+ struct {
+ __be32 __pad1;
+ __be32 rl_igen2;
+ };
+ };
__be32 rl_unlinked;
__be32 __pad;
};
@@ -187,7 +193,13 @@ struct gfs2_rgrp {
__be32 rg_free;
__be32 rg_dinodes;
__be32 __pad;
- __be64 rg_igeneration;
+ union {
+ __be64 rg_igeneration;
+ struct {
+ __be32 __pad1;
+ __be32 rg_igen2;
+ };
+ };
__u8 rg_reserved[80]; /* Several fields from gfs1 now reserved */
};
@@ -268,8 +280,13 @@ struct gfs2_dinode {
*/
__be64 di_goal_meta; /* rgrp to alloc from next */
__be64 di_goal_data; /* data block goal */
- __be64 di_generation; /* generation number for NFS */
-
+ union {
+ __be64 di_generation; /* generation number for NFS */
+ struct {
+ __be32 __pad5;
+ __be32 di_gen2;
+ };
+ };
__be32 di_flags; /* GFS2_DIF_... */
__be32 di_payload_format; /* GFS2_FORMAT_... */
__u16 __pad1; /* Was ditype in gfs1 */
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Cluster-devel] [PATCH 2/4] GFS2: Reduce inode size by using 32-bit i_generation
2015-10-28 14:20 ` [Cluster-devel] [PATCH 2/4] GFS2: Reduce inode size by using 32-bit i_generation Bob Peterson
2015-10-29 17:21 ` Andrew Price
@ 2015-10-30 10:42 ` Steven Whitehouse
1 sibling, 0 replies; 11+ messages in thread
From: Steven Whitehouse @ 2015-10-30 10:42 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
I don't think we can get away with shrinking i_generation. It is 64 bits
on disk, and in any event the union that you've used for the ondisk
structures doesn't take into account endianess. We do need to keep all
64 bits in order to ensure that we never land up with a problem with NFS
lookups. I guess that you could try and chop it in half, and keep the
lower bits in the vfs inode and the upper bits in the gfs2 inode, if
that helps, but I'm not sure that it is worth the effort of doing that
overall,
Steve.
On 28/10/15 14:20, Bob Peterson wrote:
> This patch removes variable i_generation from gfs2_inode. In its
> place, we use inode->i_generation which is pretty much the same thing,
> in 32-bits. The loss of 32 bits should not be a problem.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> fs/gfs2/glops.c | 2 +-
> fs/gfs2/incore.h | 9 +++++++--
> fs/gfs2/inode.c | 5 +++--
> fs/gfs2/rgrp.c | 16 ++++++++--------
> fs/gfs2/rgrp.h | 2 +-
> fs/gfs2/super.c | 2 +-
> include/uapi/linux/gfs2_ondisk.h | 25 +++++++++++++++++++++----
> 7 files changed, 42 insertions(+), 19 deletions(-)
>
> diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> index 1f6c9c3..79cd4a9 100644
> --- a/fs/gfs2/glops.c
> +++ b/fs/gfs2/glops.c
> @@ -358,7 +358,7 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
> ip->i_inode.i_ctime.tv_nsec = be32_to_cpu(str->di_ctime_nsec);
>
> ip->i_goal = be64_to_cpu(str->di_goal_meta);
> - ip->i_generation = be64_to_cpu(str->di_generation);
> + ip->i_inode.i_generation = be32_to_cpu(str->di_gen2);
>
> ip->i_diskflags = be32_to_cpu(str->di_flags);
> ip->i_eattr = be64_to_cpu(str->di_eattr);
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index 76c9c77..8ca88b9 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -88,7 +88,13 @@ struct gfs2_rgrpd {
> u32 rd_reserved; /* number of blocks reserved */
> u32 rd_free_clone;
> u32 rd_dinodes;
> - u64 rd_igeneration;
> + union {
> + u64 rd_igeneration;
> + struct {
> + u32 rd_igen1;
> + u32 rd_igen2;
> + };
> + };
> struct gfs2_bitmap *rd_bits;
> struct gfs2_sbd *rd_sbd;
> struct gfs2_rgrp_lvb *rd_rgl;
> @@ -385,7 +391,6 @@ struct gfs2_inode {
> struct inode i_inode;
> u64 i_no_addr;
> u64 i_no_formal_ino;
> - u64 i_generation;
> u64 i_eattr;
> unsigned long i_flags; /* GIF_... */
> struct gfs2_glock *i_gl; /* Move into i_gh? */
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index 063fdfc..cc815ab 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -394,8 +394,9 @@ static int alloc_dinode(struct gfs2_inode *ip, u32 flags, unsigned *dblocks)
> if (error)
> goto out_ipreserv;
>
> - error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1, &ip->i_generation);
> - ip->i_no_formal_ino = ip->i_generation;
> + error = gfs2_alloc_blocks(ip, &ip->i_no_addr, dblocks, 1,
> + &ip->i_inode.i_generation);
> + ip->i_no_formal_ino = ip->i_inode.i_generation;
> ip->i_inode.i_ino = ip->i_no_addr;
> ip->i_goal = ip->i_no_addr;
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 475985d..34c42a8 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1062,7 +1062,7 @@ static void gfs2_rgrp_in(struct gfs2_rgrpd *rgd, const void *buf)
> rgd->rd_flags |= rg_flags;
> rgd->rd_free = be32_to_cpu(str->rg_free);
> rgd->rd_dinodes = be32_to_cpu(str->rg_dinodes);
> - rgd->rd_igeneration = be64_to_cpu(str->rg_igeneration);
> + rgd->rd_igen2 = be32_to_cpu(str->rg_igen2);
> }
>
> static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
> @@ -1073,7 +1073,7 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
> str->rg_free = cpu_to_be32(rgd->rd_free);
> str->rg_dinodes = cpu_to_be32(rgd->rd_dinodes);
> str->__pad = cpu_to_be32(0);
> - str->rg_igeneration = cpu_to_be64(rgd->rd_igeneration);
> + str->rg_igen2 = cpu_to_be64(rgd->rd_igen2);
> memset(&str->rg_reserved, 0, sizeof(str->rg_reserved));
> }
>
> @@ -1084,7 +1084,7 @@ static int gfs2_rgrp_lvb_valid(struct gfs2_rgrpd *rgd)
>
> if (rgl->rl_flags != str->rg_flags || rgl->rl_free != str->rg_free ||
> rgl->rl_dinodes != str->rg_dinodes ||
> - rgl->rl_igeneration != str->rg_igeneration)
> + rgl->rl_igen2 != str->rg_igen2)
> return 0;
> return 1;
> }
> @@ -1097,7 +1097,7 @@ static void gfs2_rgrp_ondisk2lvb(struct gfs2_rgrp_lvb *rgl, const void *buf)
> rgl->rl_flags = str->rg_flags;
> rgl->rl_free = str->rg_free;
> rgl->rl_dinodes = str->rg_dinodes;
> - rgl->rl_igeneration = str->rg_igeneration;
> + rgl->rl_igen2 = str->rg_igen2;
> rgl->__pad = 0UL;
> }
>
> @@ -1229,7 +1229,7 @@ static int update_rgrp_lvb(struct gfs2_rgrpd *rgd)
> rgd->rd_free = be32_to_cpu(rgd->rd_rgl->rl_free);
> rgd->rd_free_clone = rgd->rd_free;
> rgd->rd_dinodes = be32_to_cpu(rgd->rd_rgl->rl_dinodes);
> - rgd->rd_igeneration = be64_to_cpu(rgd->rd_rgl->rl_igeneration);
> + rgd->rd_igen2 = be64_to_cpu(rgd->rd_rgl->rl_igen2);
> return 0;
> }
>
> @@ -2334,7 +2334,7 @@ static void gfs2_set_alloc_start(struct gfs2_rbm *rbm,
> */
>
> int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
> - bool dinode, u64 *generation)
> + bool dinode, u32 *generation)
> {
> struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> struct buffer_head *dibh;
> @@ -2390,9 +2390,9 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
> rbm.rgd->rd_free -= *nblocks;
> if (dinode) {
> rbm.rgd->rd_dinodes++;
> - *generation = rbm.rgd->rd_igeneration++;
> + *generation = rbm.rgd->rd_igen2++;
> if (*generation == 0)
> - *generation = rbm.rgd->rd_igeneration++;
> + *generation = rbm.rgd->rd_igen2++;
> }
>
> gfs2_trans_add_meta(rbm.rgd->rd_gl, rbm.rgd->rd_bits[0].bi_bh);
> diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
> index c0ab33f..71c1a8e 100644
> --- a/fs/gfs2/rgrp.h
> +++ b/fs/gfs2/rgrp.h
> @@ -47,7 +47,7 @@ extern int gfs2_inplace_reserve(struct gfs2_inode *ip,
> extern void gfs2_inplace_release(struct gfs2_inode *ip);
>
> extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
> - bool dinode, u64 *generation);
> + bool dinode, u32 *generation);
>
> extern int gfs2_rs_alloc(struct gfs2_inode *ip);
> extern void gfs2_rs_deltree(struct gfs2_blkreserv *rs);
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index 894fb01..928be2f 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -714,7 +714,7 @@ void gfs2_dinode_out(const struct gfs2_inode *ip, void *buf)
>
> str->di_goal_meta = cpu_to_be64(ip->i_goal);
> str->di_goal_data = cpu_to_be64(ip->i_goal);
> - str->di_generation = cpu_to_be64(ip->i_generation);
> + str->di_gen2 = cpu_to_be64(ip->i_inode.i_generation);
>
> str->di_flags = cpu_to_be32(ip->i_diskflags);
> str->di_height = cpu_to_be16(ip->i_height);
> diff --git a/include/uapi/linux/gfs2_ondisk.h b/include/uapi/linux/gfs2_ondisk.h
> index 1a763ea..bef244e 100644
> --- a/include/uapi/linux/gfs2_ondisk.h
> +++ b/include/uapi/linux/gfs2_ondisk.h
> @@ -175,7 +175,13 @@ struct gfs2_rgrp_lvb {
> __be32 rl_flags;
> __be32 rl_free;
> __be32 rl_dinodes;
> - __be64 rl_igeneration;
> + union {
> + __be64 rl_igeneration;
> + struct {
> + __be32 rl_igen1;
> + __be32 rl_igen2;
> + };
> + };
> __be32 rl_unlinked;
> __be32 __pad;
> };
> @@ -187,7 +193,13 @@ struct gfs2_rgrp {
> __be32 rg_free;
> __be32 rg_dinodes;
> __be32 __pad;
> - __be64 rg_igeneration;
> + union {
> + __be64 rg_igeneration;
> + struct {
> + __be32 rg_igen1;
> + __be32 rg_igen2;
> + };
> + };
>
> __u8 rg_reserved[80]; /* Several fields from gfs1 now reserved */
> };
> @@ -268,8 +280,13 @@ struct gfs2_dinode {
> */
> __be64 di_goal_meta; /* rgrp to alloc from next */
> __be64 di_goal_data; /* data block goal */
> - __be64 di_generation; /* generation number for NFS */
> -
> + union {
> + __be64 di_generation; /* generation number for NFS */
> + struct {
> + __be32 di_gen1;
> + __be32 di_gen2;
> + };
> + };
> __be32 di_flags; /* GFS2_DIF_... */
> __be32 di_payload_format; /* GFS2_FORMAT_... */
> __u16 __pad1; /* Was ditype in gfs1 */
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Cluster-devel] [PATCH 3/4] GFS2: Extract quota data from reservations structure (revert 5407e24)
2015-10-28 14:20 [Cluster-devel] [PATCH 0/4] Fix erroneous ETXTBSY problems with GFS2 Bob Peterson
2015-10-28 14:20 ` [Cluster-devel] [PATCH 1/4] GFS2: Reduce size of incore inode Bob Peterson
2015-10-28 14:20 ` [Cluster-devel] [PATCH 2/4] GFS2: Reduce inode size by using 32-bit i_generation Bob Peterson
@ 2015-10-28 14:20 ` Bob Peterson
2015-10-28 14:20 ` [Cluster-devel] [PATCH 4/4] GFS2: Make rgrp reservations part of the gfs2_inode structure Bob Peterson
2015-10-29 17:21 ` [Cluster-devel] [PATCH 0/4] Fix erroneous ETXTBSY problems with GFS2 Andrew Price
4 siblings, 0 replies; 11+ messages in thread
From: Bob Peterson @ 2015-10-28 14:20 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch basically reverts the majority of patch 5407e24.
That patch eliminated the gfs2_qadata structure in favor of just
using the reservations structure. The problem with doing that is that
it increases the size of the reservations structure. That is not an
issue until it comes time to fold the reservations structure into the
inode in memory so we know it's always there. By separating out the
quota structure again, we aren't punishing the non-quota users by
making all the inodes bigger, requiring more slab space. This patch
creates a new slab area to allocate the quota stuff so it's managed
a little more sanely.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/aops.c | 2 +-
fs/gfs2/incore.h | 12 ++++++++----
fs/gfs2/main.c | 11 +++++++++++
fs/gfs2/quota.c | 48 ++++++++++++++++++++++++------------------------
fs/gfs2/rgrp.c | 16 ++++++++++++++++
fs/gfs2/util.c | 1 +
fs/gfs2/util.h | 1 +
7 files changed, 62 insertions(+), 29 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 1caee05..93f0746 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -914,7 +914,7 @@ static int gfs2_write_end(struct file *file, struct address_space *mapping,
failed:
gfs2_trans_end(sdp);
gfs2_inplace_release(ip);
- if (ip->i_res->rs_qa_qd_num)
+ if (ip->i_qadata && ip->i_qadata->qa_qd_num)
gfs2_quota_unlock(ip);
if (inode == sdp->sd_rindex) {
gfs2_glock_dq(&m_ip->i_gh);
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 8ca88b9..72d2a47 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -276,6 +276,13 @@ struct gfs2_holder {
/* Number of quota types we support */
#define GFS2_MAXQUOTAS 2
+struct gfs2_qadata { /* quota allocation data */
+ /* Quota stuff */
+ struct gfs2_quota_data *qa_qd[2 * GFS2_MAXQUOTAS];
+ struct gfs2_holder qa_qd_ghs[2 * GFS2_MAXQUOTAS];
+ unsigned int qa_qd_num;
+};
+
/* Resource group multi-block reservation, in order of appearance:
Step 1. Function prepares to write, allocates a mb, sets the size hint.
@@ -294,10 +301,6 @@ struct gfs2_blkreserv {
struct gfs2_rbm rs_rbm; /* Start of reservation */
u32 rs_free; /* how many blocks are still free */
u64 rs_inum; /* Inode number for reservation */
-
- struct gfs2_quota_data *rs_qa_qd[2 * GFS2_MAXQUOTAS];
- struct gfs2_holder rs_qa_qd_ghs[2 * GFS2_MAXQUOTAS];
- u32 rs_qa_qd_num;
};
/*
@@ -396,6 +399,7 @@ struct gfs2_inode {
struct gfs2_glock *i_gl; /* Move into i_gh? */
struct gfs2_holder i_iopen_gh;
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 */
diff --git a/fs/gfs2/main.c b/fs/gfs2/main.c
index 241a399..f760aca 100644
--- a/fs/gfs2/main.c
+++ b/fs/gfs2/main.c
@@ -41,6 +41,7 @@ static void gfs2_init_inode_once(void *foo)
inode_init_once(&ip->i_inode);
init_rwsem(&ip->i_rw_mutex);
INIT_LIST_HEAD(&ip->i_trunc_list);
+ ip->i_qadata = NULL;
ip->i_res = NULL;
ip->i_hash_cache = NULL;
}
@@ -135,6 +136,12 @@ static int __init init_gfs2_fs(void)
if (!gfs2_quotad_cachep)
goto fail;
+ gfs2_qadata_cachep = kmem_cache_create("gfs2_qadata",
+ sizeof(struct gfs2_qadata),
+ 0, 0, NULL);
+ if (!gfs2_qadata_cachep)
+ goto fail;
+
gfs2_rsrv_cachep = kmem_cache_create("gfs2_mblk",
sizeof(struct gfs2_blkreserv),
0, 0, NULL);
@@ -196,6 +203,9 @@ fail_lru:
if (gfs2_rsrv_cachep)
kmem_cache_destroy(gfs2_rsrv_cachep);
+ if (gfs2_qadata_cachep)
+ kmem_cache_destroy(gfs2_qadata_cachep);
+
if (gfs2_quotad_cachep)
kmem_cache_destroy(gfs2_quotad_cachep);
@@ -239,6 +249,7 @@ static void __exit exit_gfs2_fs(void)
mempool_destroy(gfs2_page_pool);
kmem_cache_destroy(gfs2_rsrv_cachep);
+ kmem_cache_destroy(gfs2_qadata_cachep);
kmem_cache_destroy(gfs2_quotad_cachep);
kmem_cache_destroy(gfs2_rgrpd_cachep);
kmem_cache_destroy(gfs2_bufdata_cachep);
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 6a9c847..9e4975c 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -533,15 +533,15 @@ int gfs2_quota_hold(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
struct gfs2_quota_data **qd;
int error;
- if (ip->i_res == NULL) {
+ if (ip->i_qadata == NULL) {
error = gfs2_rs_alloc(ip);
if (error)
return error;
}
- qd = ip->i_res->rs_qa_qd;
+ qd = ip->i_qadata->qa_qd;
- if (gfs2_assert_warn(sdp, !ip->i_res->rs_qa_qd_num) ||
+ if (gfs2_assert_warn(sdp, !ip->i_qadata->qa_qd_num) ||
gfs2_assert_warn(sdp, !test_bit(GIF_QD_LOCKED, &ip->i_flags)))
return -EIO;
@@ -551,13 +551,13 @@ int gfs2_quota_hold(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
error = qdsb_get(sdp, make_kqid_uid(ip->i_inode.i_uid), qd);
if (error)
goto out;
- ip->i_res->rs_qa_qd_num++;
+ ip->i_qadata->qa_qd_num++;
qd++;
error = qdsb_get(sdp, make_kqid_gid(ip->i_inode.i_gid), qd);
if (error)
goto out;
- ip->i_res->rs_qa_qd_num++;
+ ip->i_qadata->qa_qd_num++;
qd++;
if (!uid_eq(uid, NO_UID_QUOTA_CHANGE) &&
@@ -565,7 +565,7 @@ int gfs2_quota_hold(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
error = qdsb_get(sdp, make_kqid_uid(uid), qd);
if (error)
goto out;
- ip->i_res->rs_qa_qd_num++;
+ ip->i_qadata->qa_qd_num++;
qd++;
}
@@ -574,7 +574,7 @@ int gfs2_quota_hold(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
error = qdsb_get(sdp, make_kqid_gid(gid), qd);
if (error)
goto out;
- ip->i_res->rs_qa_qd_num++;
+ ip->i_qadata->qa_qd_num++;
qd++;
}
@@ -589,15 +589,15 @@ void gfs2_quota_unhold(struct gfs2_inode *ip)
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
u32 x;
- if (ip->i_res == NULL)
+ if (ip->i_qadata == NULL)
return;
gfs2_assert_warn(sdp, !test_bit(GIF_QD_LOCKED, &ip->i_flags));
- for (x = 0; x < ip->i_res->rs_qa_qd_num; x++) {
- qdsb_put(ip->i_res->rs_qa_qd[x]);
- ip->i_res->rs_qa_qd[x] = NULL;
+ for (x = 0; x < ip->i_qadata->qa_qd_num; x++) {
+ qdsb_put(ip->i_qadata->qa_qd[x]);
+ ip->i_qadata->qa_qd[x] = NULL;
}
- ip->i_res->rs_qa_qd_num = 0;
+ ip->i_qadata->qa_qd_num = 0;
}
static int sort_qd(const void *a, const void *b)
@@ -1014,12 +1014,12 @@ int gfs2_quota_lock(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
sdp->sd_args.ar_quota != GFS2_QUOTA_ON)
return 0;
- sort(ip->i_res->rs_qa_qd, ip->i_res->rs_qa_qd_num,
+ sort(ip->i_qadata->qa_qd, ip->i_qadata->qa_qd_num,
sizeof(struct gfs2_quota_data *), sort_qd, NULL);
- for (x = 0; x < ip->i_res->rs_qa_qd_num; x++) {
- qd = ip->i_res->rs_qa_qd[x];
- error = do_glock(qd, NO_FORCE, &ip->i_res->rs_qa_qd_ghs[x]);
+ for (x = 0; x < ip->i_qadata->qa_qd_num; x++) {
+ qd = ip->i_qadata->qa_qd[x];
+ error = do_glock(qd, NO_FORCE, &ip->i_qadata->qa_qd_ghs[x]);
if (error)
break;
}
@@ -1028,7 +1028,7 @@ int gfs2_quota_lock(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
set_bit(GIF_QD_LOCKED, &ip->i_flags);
else {
while (x--)
- gfs2_glock_dq_uninit(&ip->i_res->rs_qa_qd_ghs[x]);
+ gfs2_glock_dq_uninit(&ip->i_qadata->qa_qd_ghs[x]);
gfs2_quota_unhold(ip);
}
@@ -1082,14 +1082,14 @@ void gfs2_quota_unlock(struct gfs2_inode *ip)
if (!test_and_clear_bit(GIF_QD_LOCKED, &ip->i_flags))
goto out;
- for (x = 0; x < ip->i_res->rs_qa_qd_num; x++) {
+ for (x = 0; x < ip->i_qadata->qa_qd_num; x++) {
struct gfs2_quota_data *qd;
int sync;
- qd = ip->i_res->rs_qa_qd[x];
+ qd = ip->i_qadata->qa_qd[x];
sync = need_sync(qd);
- gfs2_glock_dq_uninit(&ip->i_res->rs_qa_qd_ghs[x]);
+ gfs2_glock_dq_uninit(&ip->i_qadata->qa_qd_ghs[x]);
if (!sync)
continue;
@@ -1168,8 +1168,8 @@ int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid,
if (sdp->sd_args.ar_quota != GFS2_QUOTA_ON)
return 0;
- for (x = 0; x < ip->i_res->rs_qa_qd_num; x++) {
- qd = ip->i_res->rs_qa_qd[x];
+ for (x = 0; x < ip->i_qadata->qa_qd_num; x++) {
+ qd = ip->i_qadata->qa_qd[x];
if (!(qid_eq(qd->qd_id, make_kqid_uid(uid)) ||
qid_eq(qd->qd_id, make_kqid_gid(gid))))
@@ -1223,8 +1223,8 @@ void gfs2_quota_change(struct gfs2_inode *ip, s64 change,
if (ip->i_diskflags & GFS2_DIF_SYSTEM)
return;
- for (x = 0; x < ip->i_res->rs_qa_qd_num; x++) {
- qd = ip->i_res->rs_qa_qd[x];
+ for (x = 0; x < ip->i_qadata->qa_qd_num; x++) {
+ qd = ip->i_qadata->qa_qd[x];
if (qid_eq(qd->qd_id, make_kqid_uid(uid)) ||
qid_eq(qd->qd_id, make_kqid_gid(gid))) {
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 34c42a8..7c77f11 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -597,11 +597,13 @@ void gfs2_free_clones(struct gfs2_rgrpd *rgd)
/**
* gfs2_rs_alloc - make sure we have a reservation assigned to the inode
+ * plus a quota allocations data structure, if necessary
* @ip: the inode for this reservation
*/
int gfs2_rs_alloc(struct gfs2_inode *ip)
{
int error = 0;
+ struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
down_write(&ip->i_rw_mutex);
if (ip->i_res)
@@ -614,6 +616,15 @@ int gfs2_rs_alloc(struct gfs2_inode *ip)
}
RB_CLEAR_NODE(&ip->i_res->rs_node);
+
+ if (sdp->sd_args.ar_quota == GFS2_QUOTA_ON) {
+ ip->i_qadata = kmem_cache_zalloc(gfs2_qadata_cachep, GFP_NOFS);
+ if (!ip->i_qadata) {
+ error = -ENOMEM;
+ kmem_cache_free(gfs2_rsrv_cachep, ip->i_res);
+ ip->i_res = NULL;
+ }
+ }
out:
up_write(&ip->i_rw_mutex);
return error;
@@ -691,6 +702,11 @@ void gfs2_rs_delete(struct gfs2_inode *ip, atomic_t *wcount)
BUG_ON(ip->i_res->rs_free);
kmem_cache_free(gfs2_rsrv_cachep, ip->i_res);
ip->i_res = NULL;
+
+ if (ip->i_qadata) {
+ kmem_cache_free(gfs2_qadata_cachep, ip->i_qadata);
+ ip->i_qadata = NULL;
+ }
}
up_write(&ip->i_rw_mutex);
}
diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index 86d2035..3b4819d 100644
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -27,6 +27,7 @@ struct kmem_cache *gfs2_inode_cachep __read_mostly;
struct kmem_cache *gfs2_bufdata_cachep __read_mostly;
struct kmem_cache *gfs2_rgrpd_cachep __read_mostly;
struct kmem_cache *gfs2_quotad_cachep __read_mostly;
+struct kmem_cache *gfs2_qadata_cachep __read_mostly;
struct kmem_cache *gfs2_rsrv_cachep __read_mostly;
mempool_t *gfs2_page_pool __read_mostly;
diff --git a/fs/gfs2/util.h b/fs/gfs2/util.h
index cbdcbdf..9edbcc9 100644
--- a/fs/gfs2/util.h
+++ b/fs/gfs2/util.h
@@ -149,6 +149,7 @@ extern struct kmem_cache *gfs2_inode_cachep;
extern struct kmem_cache *gfs2_bufdata_cachep;
extern struct kmem_cache *gfs2_rgrpd_cachep;
extern struct kmem_cache *gfs2_quotad_cachep;
+extern struct kmem_cache *gfs2_qadata_cachep;
extern struct kmem_cache *gfs2_rsrv_cachep;
extern mempool_t *gfs2_page_pool;
--
2.4.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Cluster-devel] [PATCH 4/4] GFS2: Make rgrp reservations part of the gfs2_inode structure
2015-10-28 14:20 [Cluster-devel] [PATCH 0/4] Fix erroneous ETXTBSY problems with GFS2 Bob Peterson
` (2 preceding siblings ...)
2015-10-28 14:20 ` [Cluster-devel] [PATCH 3/4] GFS2: Extract quota data from reservations structure (revert 5407e24) Bob Peterson
@ 2015-10-28 14:20 ` Bob Peterson
2015-10-29 17:21 ` [Cluster-devel] [PATCH 0/4] Fix erroneous ETXTBSY problems with GFS2 Andrew Price
4 siblings, 0 replies; 11+ messages in thread
From: Bob Peterson @ 2015-10-28 14:20 UTC (permalink / raw)
To: cluster-devel.redhat.com
Before this patch, multi-block reservation structures were allocated
from a special slab. This patch folds the structure into the gfs2_inode
structure. The disadvantage is that the gfs2_inode needs more memory,
even when a file is opened read-only. The advantages are: (a) we don't
need the special slab and the extra time it takes to allocate and
deallocate from it. (b) we no longer need to worry that the structure
exists for things like quota management. (c) This also allows us to
remove the calls to get_write_access and put_write_access since we
know the structure will exist.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
fs/gfs2/bmap.c | 15 +++----------
fs/gfs2/file.c | 34 ++++-------------------------
fs/gfs2/incore.h | 2 +-
fs/gfs2/inode.c | 38 ++------------------------------
fs/gfs2/main.c | 13 ++---------
fs/gfs2/quota.c | 32 ++++++++++++++++++---------
fs/gfs2/rgrp.c | 66 +++++++++++---------------------------------------------
fs/gfs2/rgrp.h | 3 +--
fs/gfs2/super.c | 7 +++---
fs/gfs2/util.c | 1 -
fs/gfs2/util.h | 1 -
11 files changed, 51 insertions(+), 161 deletions(-)
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 61296ec..917718e 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -787,8 +787,8 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
if (error)
goto out_rlist;
- if (gfs2_rs_active(ip->i_res)) /* needs to be done with the rgrp glock held */
- gfs2_rs_deltree(ip->i_res);
+ if (gfs2_rs_active(&ip->i_res)) /* needs to be done with the rgrp glock held */
+ gfs2_rs_deltree(&ip->i_res);
error = gfs2_trans_begin(sdp, rg_blocks + RES_DINODE +
RES_INDIRECT + RES_STATFS + RES_QUOTA,
@@ -1291,26 +1291,17 @@ int gfs2_setattr_size(struct inode *inode, u64 newsize)
if (ret)
return ret;
- ret = get_write_access(inode);
- if (ret)
- return ret;
-
inode_dio_wait(inode);
- ret = gfs2_rs_alloc(ip);
- if (ret)
- goto out;
-
oldsize = inode->i_size;
if (newsize >= oldsize) {
ret = do_grow(inode, newsize);
goto out;
}
- gfs2_rs_deltree(ip->i_res);
+ gfs2_rs_deltree(&ip->i_res);
ret = do_shrink(inode, oldsize, newsize);
out:
- put_write_access(inode);
return ret;
}
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 7a99270..cb23756 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -336,8 +336,8 @@ static void gfs2_size_hint(struct file *filep, loff_t offset, size_t size)
size_t blks = (size + sdp->sd_sb.sb_bsize - 1) >> sdp->sd_sb.sb_bsize_shift;
int hint = min_t(size_t, INT_MAX, blks);
- if (hint > atomic_read(&ip->i_res->rs_sizehint))
- atomic_set(&ip->i_res->rs_sizehint, hint);
+ if (hint > atomic_read(&ip->i_res.rs_sizehint))
+ atomic_set(&ip->i_res.rs_sizehint, hint);
}
/**
@@ -397,14 +397,6 @@ static int gfs2_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
/* Update file times before taking page lock */
file_update_time(vma->vm_file);
- ret = get_write_access(inode);
- if (ret)
- goto out;
-
- ret = gfs2_rs_alloc(ip);
- if (ret)
- goto out_write_access;
-
gfs2_size_hint(vma->vm_file, pos, PAGE_CACHE_SIZE);
gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh);
@@ -486,9 +478,6 @@ out_uninit:
set_page_dirty(page);
wait_for_stable_page(page);
}
-out_write_access:
- put_write_access(inode);
-out:
sb_end_pagefault(inode->i_sb);
return block_page_mkwrite_return(ret);
}
@@ -703,10 +692,6 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
struct gfs2_inode *ip = GFS2_I(file_inode(file));
int ret;
- ret = gfs2_rs_alloc(ip);
- if (ret)
- return ret;
-
gfs2_size_hint(file, iocb->ki_pos, iov_iter_count(from));
if (iocb->ki_flags & IOCB_APPEND) {
@@ -938,14 +923,10 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t le
if (ret)
goto out_unlock;
- ret = gfs2_rs_alloc(ip);
- if (ret)
- goto out_putw;
-
ret = __gfs2_fallocate(file, mode, offset, len);
if (ret)
- gfs2_rs_deltree(ip->i_res);
-out_putw:
+ gfs2_rs_deltree(&ip->i_res);
+
put_write_access(inode);
out_unlock:
gfs2_glock_dq(&gh);
@@ -959,13 +940,6 @@ static ssize_t gfs2_file_splice_write(struct pipe_inode_info *pipe,
struct file *out, loff_t *ppos,
size_t len, unsigned int flags)
{
- int error;
- struct gfs2_inode *ip = GFS2_I(out->f_mapping->host);
-
- error = gfs2_rs_alloc(ip);
- if (error)
- return (ssize_t)error;
-
gfs2_size_hint(out, *ppos, len);
return iter_file_splice_write(pipe, out, ppos, len, flags);
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 72d2a47..c8a0a54 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -400,7 +400,7 @@ struct gfs2_inode {
struct gfs2_holder i_iopen_gh;
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_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;
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index cc815ab..44c5958 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -602,10 +602,6 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
if (!name->len || name->len > GFS2_FNAMESIZE)
return -ENAMETOOLONG;
- error = gfs2_rs_alloc(dip);
- if (error)
- return error;
-
error = gfs2_rindex_update(sdp);
if (error)
return error;
@@ -654,10 +650,6 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
goto fail_free_vfs_inode;
ip = GFS2_I(inode);
- error = gfs2_rs_alloc(ip);
- if (error)
- goto fail_free_acls;
-
inode->i_mode = mode;
set_nlink(inode, S_ISDIR(mode) ? 2 : 1);
inode->i_rdev = dev;
@@ -778,7 +770,6 @@ fail_free_inode:
if (ip->i_gl)
gfs2_glock_put(ip->i_gl);
gfs2_rs_delete(ip, NULL);
-fail_free_acls:
if (default_acl)
posix_acl_release(default_acl);
if (acl)
@@ -899,10 +890,6 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir,
if (S_ISDIR(inode->i_mode))
return -EPERM;
- error = gfs2_rs_alloc(dip);
- if (error)
- return error;
-
gfs2_holder_init(dip->i_gl, LM_ST_EXCLUSIVE, 0, ghs);
gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, ghs + 1);
@@ -1372,10 +1359,6 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry,
if (error)
return error;
- error = gfs2_rs_alloc(ndip);
- if (error)
- return error;
-
if (odip != ndip) {
error = gfs2_glock_nq_init(sdp->sd_rename_gl, LM_ST_EXCLUSIVE,
0, &r_gh);
@@ -1855,14 +1838,6 @@ static int setattr_chown(struct inode *inode, struct iattr *attr)
if (!(attr->ia_valid & ATTR_GID) || gid_eq(ogid, ngid))
ogid = ngid = NO_GID_QUOTA_CHANGE;
- error = get_write_access(inode);
- if (error)
- return error;
-
- error = gfs2_rs_alloc(ip);
- if (error)
- goto out;
-
error = gfs2_rindex_update(sdp);
if (error)
goto out;
@@ -1899,7 +1874,6 @@ out_end_trans:
out_gunlock_q:
gfs2_quota_unlock(ip);
out:
- put_write_access(inode);
return error;
}
@@ -1921,10 +1895,6 @@ static int gfs2_setattr(struct dentry *dentry, struct iattr *attr)
struct gfs2_holder i_gh;
int error;
- error = gfs2_rs_alloc(ip);
- if (error)
- return error;
-
error = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &i_gh);
if (error)
return error;
@@ -2003,9 +1973,7 @@ static int gfs2_setxattr(struct dentry *dentry, const char *name,
gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh);
ret = gfs2_glock_nq(&gh);
if (ret == 0) {
- ret = gfs2_rs_alloc(ip);
- if (ret == 0)
- ret = generic_setxattr(dentry, name, data, size, flags);
+ ret = generic_setxattr(dentry, name, data, size, flags);
gfs2_glock_dq(&gh);
}
gfs2_holder_uninit(&gh);
@@ -2044,9 +2012,7 @@ static int gfs2_removexattr(struct dentry *dentry, const char *name)
gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh);
ret = gfs2_glock_nq(&gh);
if (ret == 0) {
- ret = gfs2_rs_alloc(ip);
- if (ret == 0)
- ret = generic_removexattr(dentry, name);
+ ret = generic_removexattr(dentry, name);
gfs2_glock_dq(&gh);
}
gfs2_holder_uninit(&gh);
diff --git a/fs/gfs2/main.c b/fs/gfs2/main.c
index f760aca..bb1f79b 100644
--- a/fs/gfs2/main.c
+++ b/fs/gfs2/main.c
@@ -42,7 +42,8 @@ static void gfs2_init_inode_once(void *foo)
init_rwsem(&ip->i_rw_mutex);
INIT_LIST_HEAD(&ip->i_trunc_list);
ip->i_qadata = NULL;
- ip->i_res = NULL;
+ memset(&ip->i_res, 0, sizeof(ip->i_res));
+ RB_CLEAR_NODE(&ip->i_res.rs_node);
ip->i_hash_cache = NULL;
}
@@ -142,12 +143,6 @@ static int __init init_gfs2_fs(void)
if (!gfs2_qadata_cachep)
goto fail;
- gfs2_rsrv_cachep = kmem_cache_create("gfs2_mblk",
- sizeof(struct gfs2_blkreserv),
- 0, 0, NULL);
- if (!gfs2_rsrv_cachep)
- goto fail;
-
register_shrinker(&gfs2_qd_shrinker);
error = register_filesystem(&gfs2_fs_type);
@@ -200,9 +195,6 @@ fail_lru:
unregister_shrinker(&gfs2_qd_shrinker);
gfs2_glock_exit();
- if (gfs2_rsrv_cachep)
- kmem_cache_destroy(gfs2_rsrv_cachep);
-
if (gfs2_qadata_cachep)
kmem_cache_destroy(gfs2_qadata_cachep);
@@ -248,7 +240,6 @@ static void __exit exit_gfs2_fs(void)
rcu_barrier();
mempool_destroy(gfs2_page_pool);
- kmem_cache_destroy(gfs2_rsrv_cachep);
kmem_cache_destroy(gfs2_qadata_cachep);
kmem_cache_destroy(gfs2_quotad_cachep);
kmem_cache_destroy(gfs2_rgrpd_cachep);
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 9e4975c..9caa88a 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -527,6 +527,27 @@ static void qdsb_put(struct gfs2_quota_data *qd)
qd_put(qd);
}
+/**
+ * gfs2_qa_alloc - make sure we have a quota allocations data structure,
+ * if necessary
+ * @ip: the inode for this reservation
+ */
+static int gfs2_qa_alloc(struct gfs2_inode *ip)
+{
+ int error = 0;
+ struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
+
+ if (sdp->sd_args.ar_quota != GFS2_QUOTA_ON)
+ return 0;
+
+ down_write(&ip->i_rw_mutex);
+ ip->i_qadata = kmem_cache_zalloc(gfs2_qadata_cachep, GFP_NOFS);
+ if (!ip->i_qadata)
+ error = -ENOMEM;
+ up_write(&ip->i_rw_mutex);
+ return error;
+}
+
int gfs2_quota_hold(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
@@ -534,7 +555,7 @@ int gfs2_quota_hold(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
int error;
if (ip->i_qadata == NULL) {
- error = gfs2_rs_alloc(ip);
+ error = gfs2_qa_alloc(ip);
if (error)
return error;
}
@@ -843,10 +864,6 @@ static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
unsigned int nalloc = 0, blocks;
int error;
- error = gfs2_rs_alloc(ip);
- if (error)
- return error;
-
gfs2_write_calc_reserv(ip, sizeof(struct gfs2_quota),
&data_blocks, &ind_blocks);
@@ -1635,10 +1652,6 @@ static int gfs2_set_dqblk(struct super_block *sb, struct kqid qid,
if (error)
return error;
- error = gfs2_rs_alloc(ip);
- if (error)
- goto out_put;
-
mutex_lock(&ip->i_inode.i_mutex);
error = gfs2_glock_nq_init(qd->qd_gl, LM_ST_EXCLUSIVE, 0, &q_gh);
if (error)
@@ -1705,7 +1718,6 @@ out_q:
gfs2_glock_dq_uninit(&q_gh);
out_unlockput:
mutex_unlock(&ip->i_inode.i_mutex);
-out_put:
qd_put(qd);
return error;
}
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 7c77f11..0a4ce58 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -595,41 +595,6 @@ void gfs2_free_clones(struct gfs2_rgrpd *rgd)
}
}
-/**
- * gfs2_rs_alloc - make sure we have a reservation assigned to the inode
- * plus a quota allocations data structure, if necessary
- * @ip: the inode for this reservation
- */
-int gfs2_rs_alloc(struct gfs2_inode *ip)
-{
- int error = 0;
- struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
-
- down_write(&ip->i_rw_mutex);
- if (ip->i_res)
- goto out;
-
- ip->i_res = kmem_cache_zalloc(gfs2_rsrv_cachep, GFP_NOFS);
- if (!ip->i_res) {
- error = -ENOMEM;
- goto out;
- }
-
- RB_CLEAR_NODE(&ip->i_res->rs_node);
-
- if (sdp->sd_args.ar_quota == GFS2_QUOTA_ON) {
- ip->i_qadata = kmem_cache_zalloc(gfs2_qadata_cachep, GFP_NOFS);
- if (!ip->i_qadata) {
- error = -ENOMEM;
- kmem_cache_free(gfs2_rsrv_cachep, ip->i_res);
- ip->i_res = NULL;
- }
- }
-out:
- up_write(&ip->i_rw_mutex);
- return error;
-}
-
static void dump_rs(struct seq_file *seq, const struct gfs2_blkreserv *rs)
{
gfs2_print_dbg(seq, " B: n:%llu s:%llu b:%u f:%u\n",
@@ -697,16 +662,9 @@ void gfs2_rs_deltree(struct gfs2_blkreserv *rs)
void gfs2_rs_delete(struct gfs2_inode *ip, atomic_t *wcount)
{
down_write(&ip->i_rw_mutex);
- if (ip->i_res && ((wcount == NULL) || (atomic_read(wcount) <= 1))) {
- gfs2_rs_deltree(ip->i_res);
- BUG_ON(ip->i_res->rs_free);
- kmem_cache_free(gfs2_rsrv_cachep, ip->i_res);
- ip->i_res = NULL;
-
- if (ip->i_qadata) {
- kmem_cache_free(gfs2_qadata_cachep, ip->i_qadata);
- ip->i_qadata = NULL;
- }
+ if (ip->i_qadata && ((wcount == NULL) || (atomic_read(wcount) <= 1))) {
+ kmem_cache_free(gfs2_qadata_cachep, ip->i_qadata);
+ ip->i_qadata = NULL;
}
up_write(&ip->i_rw_mutex);
}
@@ -1471,7 +1429,7 @@ static void rs_insert(struct gfs2_inode *ip)
{
struct rb_node **newn, *parent = NULL;
int rc;
- struct gfs2_blkreserv *rs = ip->i_res;
+ struct gfs2_blkreserv *rs = &ip->i_res;
struct gfs2_rgrpd *rgd = rs->rs_rbm.rgd;
u64 fsblock = gfs2_rbm_to_block(&rs->rs_rbm);
@@ -1518,7 +1476,7 @@ static void rg_mblk_search(struct gfs2_rgrpd *rgd, struct gfs2_inode *ip,
{
struct gfs2_rbm rbm = { .rgd = rgd, };
u64 goal;
- struct gfs2_blkreserv *rs = ip->i_res;
+ struct gfs2_blkreserv *rs = &ip->i_res;
u32 extlen;
u32 free_blocks = rgd->rd_free_clone - rgd->rd_reserved;
int ret;
@@ -1589,7 +1547,7 @@ static u64 gfs2_next_unreserved_block(struct gfs2_rgrpd *rgd, u64 block,
}
if (n) {
- while ((rs_cmp(block, length, rs) == 0) && (ip->i_res != rs)) {
+ while ((rs_cmp(block, length, rs) == 0) && (&ip->i_res != rs)) {
block = gfs2_rbm_to_block(&rs->rs_rbm) + rs->rs_free;
n = n->rb_right;
if (n == NULL)
@@ -1999,7 +1957,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct gfs2_rgrpd *begin = NULL;
- struct gfs2_blkreserv *rs = ip->i_res;
+ struct gfs2_blkreserv *rs = &ip->i_res;
int error = 0, rg_locked, flags = 0;
u64 last_unlinked = NO_BLOCK;
int loops = 0;
@@ -2128,7 +2086,7 @@ next_rgrp:
void gfs2_inplace_release(struct gfs2_inode *ip)
{
- struct gfs2_blkreserv *rs = ip->i_res;
+ struct gfs2_blkreserv *rs = &ip->i_res;
if (rs->rs_rgd_gh.gh_gl)
gfs2_glock_dq_uninit(&rs->rs_rgd_gh);
@@ -2282,7 +2240,7 @@ static void gfs2_rgrp_error(struct gfs2_rgrpd *rgd)
static void gfs2_adjust_reservation(struct gfs2_inode *ip,
const struct gfs2_rbm *rbm, unsigned len)
{
- struct gfs2_blkreserv *rs = ip->i_res;
+ struct gfs2_blkreserv *rs = &ip->i_res;
struct gfs2_rgrpd *rgd = rbm->rgd;
unsigned rlen;
u64 block;
@@ -2325,8 +2283,8 @@ static void gfs2_set_alloc_start(struct gfs2_rbm *rbm,
{
u64 goal;
- if (gfs2_rs_active(ip->i_res)) {
- *rbm = ip->i_res->rs_rbm;
+ if (gfs2_rs_active(&ip->i_res)) {
+ *rbm = ip->i_res.rs_rbm;
return;
}
@@ -2380,7 +2338,7 @@ int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *nblocks,
gfs2_alloc_extent(&rbm, dinode, nblocks);
block = gfs2_rbm_to_block(&rbm);
rbm.rgd->rd_last_alloc = block - rbm.rgd->rd_data0;
- if (gfs2_rs_active(ip->i_res))
+ if (gfs2_rs_active(&ip->i_res))
gfs2_adjust_reservation(ip, &rbm, *nblocks);
ndata = *nblocks;
if (dinode)
diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
index 71c1a8e..14d0d16 100644
--- a/fs/gfs2/rgrp.h
+++ b/fs/gfs2/rgrp.h
@@ -49,7 +49,6 @@ extern void gfs2_inplace_release(struct gfs2_inode *ip);
extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
bool dinode, u32 *generation);
-extern int gfs2_rs_alloc(struct gfs2_inode *ip);
extern void gfs2_rs_deltree(struct gfs2_blkreserv *rs);
extern void gfs2_rs_delete(struct gfs2_inode *ip, atomic_t *wcount);
extern void __gfs2_free_blocks(struct gfs2_inode *ip, u64 bstart, u32 blen, int meta);
@@ -78,7 +77,7 @@ extern int gfs2_rgrp_send_discards(struct gfs2_sbd *sdp, u64 offset,
extern int gfs2_fitrim(struct file *filp, void __user *argp);
/* This is how to tell if a reservation is in the rgrp tree: */
-static inline bool gfs2_rs_active(struct gfs2_blkreserv *rs)
+static inline bool gfs2_rs_active(const struct gfs2_blkreserv *rs)
{
return rs && !RB_EMPTY_NODE(&rs->rs_node);
}
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 928be2f..d692f01 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -1593,8 +1593,8 @@ out_truncate:
out_unlock:
/* Error path for case 1 */
- if (gfs2_rs_active(ip->i_res))
- gfs2_rs_deltree(ip->i_res);
+ if (gfs2_rs_active(&ip->i_res))
+ gfs2_rs_deltree(&ip->i_res);
if (test_bit(HIF_HOLDER, &ip->i_iopen_gh.gh_iflags)) {
ip->i_iopen_gh.gh_flags |= GL_NOCACHE;
@@ -1632,7 +1632,8 @@ static struct inode *gfs2_alloc_inode(struct super_block *sb)
ip->i_flags = 0;
ip->i_gl = NULL;
ip->i_rgd = NULL;
- ip->i_res = NULL;
+ memset(&ip->i_res, 0, sizeof(ip->i_res));
+ RB_CLEAR_NODE(&ip->i_res.rs_node);
}
return &ip->i_inode;
}
diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index 3b4819d..cf64583 100644
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -28,7 +28,6 @@ struct kmem_cache *gfs2_bufdata_cachep __read_mostly;
struct kmem_cache *gfs2_rgrpd_cachep __read_mostly;
struct kmem_cache *gfs2_quotad_cachep __read_mostly;
struct kmem_cache *gfs2_qadata_cachep __read_mostly;
-struct kmem_cache *gfs2_rsrv_cachep __read_mostly;
mempool_t *gfs2_page_pool __read_mostly;
void gfs2_assert_i(struct gfs2_sbd *sdp)
diff --git a/fs/gfs2/util.h b/fs/gfs2/util.h
index 9edbcc9..c81295f 100644
--- a/fs/gfs2/util.h
+++ b/fs/gfs2/util.h
@@ -150,7 +150,6 @@ extern struct kmem_cache *gfs2_bufdata_cachep;
extern struct kmem_cache *gfs2_rgrpd_cachep;
extern struct kmem_cache *gfs2_quotad_cachep;
extern struct kmem_cache *gfs2_qadata_cachep;
-extern struct kmem_cache *gfs2_rsrv_cachep;
extern mempool_t *gfs2_page_pool;
static inline unsigned int gfs2_tune_get_i(struct gfs2_tune *gt,
--
2.4.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Cluster-devel] [PATCH 0/4] Fix erroneous ETXTBSY problems with GFS2
2015-10-28 14:20 [Cluster-devel] [PATCH 0/4] Fix erroneous ETXTBSY problems with GFS2 Bob Peterson
` (3 preceding siblings ...)
2015-10-28 14:20 ` [Cluster-devel] [PATCH 4/4] GFS2: Make rgrp reservations part of the gfs2_inode structure Bob Peterson
@ 2015-10-29 17:21 ` Andrew Price
4 siblings, 0 replies; 11+ messages in thread
From: Andrew Price @ 2015-10-29 17:21 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 28/10/15 14:20, Bob Peterson wrote:
> Commit 2b3dcf3 (GFS2: Increase i_writecount during gfs2_setattr_size)
> added a bunch of calls to get_write_access() in order to ensure file close
> could not delete an inode's multi-block reservation while the function
> was running. For example, close was interfering with setattr_size.
> The patch worked as expected, but there was an unintended consequence:
> If a GFS2 file is open, you can't do these functions (like chown) or
> you'll get ETXTBSY (Text file busy). For example, if you're running a
> program and try to do chown or chattr while it's running, it will fail.
>
> This problem is easily fixed by reverting that patch and including the
> multi-block reservation structure inside the gfs2 inode. The problem is,
> the gfs2 inode then grows to a much bigger size.
>
> This patch set takes some measures to reduce the size of the gfs2 inode
> and then making the reservations part of the inode.
>
> Before the patch set, GFS2 inodes were 880 bytes in length. Adding just
> the reservations structure bumped it up to 1304, which is not good.
> With all these patches in place, the size is 1008, which is comparable
> to ext4 (1048) and xfs (1088). Further savings are possible (and I've
> prototyped a few) but I don't want to get too extreme here.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> Bob Peterson (4):
> GFS2: Reduce size of incore inode
> GFS2: Reduce inode size by using 32-bit i_generation
> GFS2: Extract quota data from reservations structure (revert 5407e24)
> GFS2: Make rgrp reservations part of the gfs2_inode structure
Other than my comments on patch 2:
Reviewed-by: Andrew Price <anprice@redhat.com>
for the series.
Cheers,
Andy
^ permalink raw reply [flat|nested] 11+ messages in thread