xfs: Add a new transaction for changing ag state

This a new tranaction which would be use for Changing AG state via ioctl(2)

Signed-off-by Jie Liu <[email protected]>

---
 fs/xfs/xfs_mount.h |    1 +
 fs/xfs/xfs_trans.c |   12 ++++++++++++
 fs/xfs/xfs_trans.h |    8 +++++---
 3 files changed, 18 insertions(+), 3 deletions(-)

fs/xfs/xfs_mount.h +1 -0

index deee09e..4fe2232 100644
@@ -40,6 +40,7 @@ typedef struct xfs_trans_reservations {
 	uint	tr_growrtalloc;	/* grow realtime allocations */
 	uint	tr_growrtzero;	/* grow realtime zeroing */
 	uint	tr_growrtfree;	/* grow realtime freeing */
+	uint	tr_setagstate;	/* set a.g. state trans */
 } xfs_trans_reservations_t;
 
 #ifndef __KERNEL__

fs/xfs/xfs_trans.c +0 -0

index 06ed520..5a5c4d8 100644
@@ -531,6 +531,17 @@ xfs_calc_clear_agi_bucket_reservation(
 }
 

Quoted 28 lines · Jeff Liu

This a new tranaction which would be use for Changing AG state via ioctl(2)

Signed-off-by Jie Liu <[email protected]>

---
 fs/xfs/xfs_mount.h |    1 +
 fs/xfs/xfs_trans.c |   12 ++++++++++++
 fs/xfs/xfs_trans.h |    8 +++++---
 3 files changed, 18 insertions(+), 3 deletions(-)

fs/xfs/xfs_mount.h +1 -0

index deee09e..4fe2232 100644
@@ -40,6 +40,7 @@ typedef struct xfs_trans_reservations {
 	uint	tr_growrtalloc;	/* grow realtime allocations */
 	uint	tr_growrtzero;	/* grow realtime zeroing */
 	uint	tr_growrtfree;	/* grow realtime freeing */
+	uint	tr_setagstate;	/* set a.g. state trans */
 } xfs_trans_reservations_t;
 
 #ifndef __KERNEL__

fs/xfs/xfs_trans.c +0 -0

index 06ed520..5a5c4d8 100644
@@ -531,6 +531,17 @@ xfs_calc_clear_agi_bucket_reservation(
 }
 

fs/xfs/xfs_trans.c xfs_calc_clear_agi_bucket_reservation( · line 542

Quoted 10 lines

fs/xfs/xfs_trans.c +9 -0

 /*
+ * Setting the state of an allocation group.
+ *	agf of the ag: sector size
+ */
+STATIC uint
+xfs_calc_set_agstate_reservation(
+	struct xfs_mount	*mp)
+{
+	return mp->m_sb.sb_sectsize + 128;
+}

I know you are just copying other code,

but what we really need to
do here is get rid of all these magic "128" numbers in the transaction reservations.

What the 128 is actually reserving is space for the struct xfs_buf_log_format that gets written into the log for every buffer. i.e. the space needed for this structure:

struct xfs_buf_log_format {

        short unsigned int         blf_type;             /*     0     2 */
        short unsigned int         blf_size;             /*     2     2 */
        ushort                     blf_flags;            /*     4     2 */
        ushort                     blf_len;              /*     6     2 */
        __int64_t                  blf_blkno;            /*     8     8 */
        unsigned int               blf_map_size;         /*    16     4 */
        unsigned int               blf_data_map[16];     /*    20    64 */
        /* --- cacheline 1 boundary (64 bytes) was 20 bytes ago --- */

        /* size: 88, cachelines: 2, members: 7 */
        /* padding: 4 */
        /* last cacheline: 24 bytes */
};

It's rounded up to give a little bit of extra space because there's it also needs a log opheader:

struct xlog_op_header {

        __be32                     oh_tid;               /*     0     4 */
        __be32                     oh_len;               /*     4     4 */
        __u8                       oh_clientid;          /*     8     1 */
        __u8                       oh_flags;             /*     9     1 */
        __u16                      oh_res2;              /*    10     2 */

        /* size: 12, cachelines: 1, members: 5 */
        /* last cacheline: 12 bytes */
};

So there's at least 100 bytes of extra information per buffer that needs to be recorded with the buffer itself. Rounding it up to 128 bytes is probably just fine, though it should probably be done programatically rather than hand coded.

As such I'd like to see this sort of thing encoded in a macro or inline function so the above code becomes something like:

	return mp->m_sb.sb_sectsize + xfs_buf_log_overhead(mp);

and

/*
* A buffer has a format structure overhead in the log in addition
* to the data, so we need to take this into account when reserving
* space in a transaction for a buffer. Round the space required up
* to a multiple of 128 bytes so that we don't change the historical
* reservation that has ben used for this overhead.
*/
static inline int
xfs_buf_log_overhead()

{
	return round_up(sizeof(struct xlog_op_header) +
			sizeof(struct xfs_buf_log_format), 128);
}

Quoted 17 lines · Dave Chinner

As such I'd like to see this sort of thing encoded in a macro or inline function so the above code becomes something like:

	return mp->m_sb.sb_sectsize + xfs_buf_log_overhead(mp);

and

/*
* A buffer has a format structure overhead in the log in addition
* to the data, so we need to take this into account when reserving
* space in a transaction for a buffer. Round the space required up
* to a multiple of 128 bytes so that we don't change the historical
* reservation that has ben used for this overhead.
*/
static inline int
xfs_buf_log_overhead()

Quoted 4 lines

{
	return round_up(sizeof(struct xlog_op_header) +
			sizeof(struct xfs_buf_log_format), 128);
}

Thanks for your teaching!

We hard-coded '128' in almost all of those transaction reservations, is it time to get rid of it according to your comments above?

I'd like to do it if you have no time to deal with such trivial things.

Quoted 27 lines · Jeff Liu

Quoted 17 lines · Dave Chinner

As such I'd like to see this sort of thing encoded in a macro or inline function so the above code becomes something like:

	return mp->m_sb.sb_sectsize + xfs_buf_log_overhead(mp);

and

/*
* A buffer has a format structure overhead in the log in addition
* to the data, so we need to take this into account when reserving
* space in a transaction for a buffer. Round the space required up
* to a multiple of 128 bytes so that we don't change the historical
* reservation that has ben used for this overhead.
*/
static inline int
xfs_buf_log_overhead()

Quoted 4 lines

{
	return round_up(sizeof(struct xlog_op_header) +
			sizeof(struct xfs_buf_log_format), 128);
}

Thanks for your teaching!

We hard-coded '128' in almost all of those transaction reservations, is it time to get rid of it according to your comments above?

Quoted 1 line

I'd like to do it if you have no time to deal with such trivial things.

Yes please!

It's been on my "clean-ups for a rainy day" list for a long time, but I've never managed to get around to it. So if you want to do this, it would make me very happy :)