* [PATCH 01/15] xfs: Add a new transaction for changing ag state
@ 2012-11-16 6:44 Jeff Liu
2012-12-03 1:24 ` Dave Chinner
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Liu @ 2012-11-16 6:44 UTC (permalink / raw)
To: xfs
This a new tranaction which would be use for Changing AG state via ioctl(2)
Signed-off-by: Jie Liu <jeff.liu@oracle.com>
---
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(-)
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index deee09e..4fe2232 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -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__
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 06ed520..5a5c4d8 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -531,6 +531,17 @@ xfs_calc_clear_agi_bucket_reservation(
}
/*
+ * 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;
+}
+
+/*
* Initialize the precomputed transaction reservation values
* in the mount structure.
*/
@@ -561,6 +572,7 @@ xfs_trans_init(
resp->tr_growrtalloc = xfs_calc_growrtalloc_reservation(mp);
resp->tr_growrtzero = xfs_calc_growrtzero_reservation(mp);
resp->tr_growrtfree = xfs_calc_growrtfree_reservation(mp);
+ resp->tr_setagstate = xfs_calc_set_agstate_reservation(mp);
}
/*
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index db05654..a4f4092 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -107,7 +107,8 @@ typedef struct xfs_trans_header {
#define XFS_TRANS_SWAPEXT 40
#define XFS_TRANS_SB_COUNT 41
#define XFS_TRANS_CHECKPOINT 42
-#define XFS_TRANS_TYPE_MAX 42
+#define XFS_TRANS_SET_AGSTATE 43
+#define XFS_TRANS_TYPE_MAX 43
/* new transaction types need to be reflected in xfs_logprint(8) */
#define XFS_TRANS_TYPES \
@@ -152,7 +153,8 @@ typedef struct xfs_trans_header {
{ XFS_TRANS_CHECKPOINT, "CHECKPOINT" }, \
{ XFS_TRANS_DUMMY1, "DUMMY1" }, \
{ XFS_TRANS_DUMMY2, "DUMMY2" }, \
- { XLOG_UNMOUNT_REC_TYPE, "UNMOUNT" }
+ { XLOG_UNMOUNT_REC_TYPE, "UNMOUNT" }, \
+ { XFS_TRANS_SET_AGSTATE, "SET_AGSTATE" }
/*
* This structure is used to track log items associated with
@@ -262,7 +264,7 @@ struct xfs_log_item_desc {
(128 * (ext + (ext * XFS_BM_MAXLEVELS(mp, XFS_ATTR_FORK)))))
#define XFS_ATTRRM_LOG_RES(mp) ((mp)->m_reservations.tr_attrrm)
#define XFS_CLEAR_AGI_BUCKET_LOG_RES(mp) ((mp)->m_reservations.tr_clearagi)
-
+#define XFS_SETAGSTATE_LOG_RES ((mp)->m_reservations.tr_setagstate)
/*
* Various log count values.
--
1.7.4.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 01/15] xfs: Add a new transaction for changing ag state
2012-11-16 6:44 [PATCH 01/15] xfs: Add a new transaction for changing ag state Jeff Liu
@ 2012-12-03 1:24 ` Dave Chinner
2012-12-03 2:54 ` Jeff Liu
0 siblings, 1 reply; 5+ messages in thread
From: Dave Chinner @ 2012-12-03 1:24 UTC (permalink / raw)
To: Jeff Liu; +Cc: xfs
On Fri, Nov 16, 2012 at 02:44:55PM +0800, Jeff Liu wrote:
> This a new tranaction which would be use for Changing AG state via ioctl(2)
>
> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
> ---
> 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(-)
>
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index deee09e..4fe2232 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -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__
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 06ed520..5a5c4d8 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -531,6 +531,17 @@ xfs_calc_clear_agi_bucket_reservation(
> }
>
> /*
> + * 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);
}
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 01/15] xfs: Add a new transaction for changing ag state
2012-12-03 1:24 ` Dave Chinner
@ 2012-12-03 2:54 ` Jeff Liu
2012-12-03 21:28 ` Dave Chinner
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Liu @ 2012-12-03 2:54 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
On 12/03/2012 09:24 AM, Dave Chinner wrote:
> On Fri, Nov 16, 2012 at 02:44:55PM +0800, Jeff Liu wrote:
>> This a new tranaction which would be use for Changing AG state via ioctl(2)
>>
>> Signed-off-by: Jie Liu <jeff.liu@oracle.com>
>> ---
>> 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(-)
>>
>> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
>> index deee09e..4fe2232 100644
>> --- a/fs/xfs/xfs_mount.h
>> +++ b/fs/xfs/xfs_mount.h
>> @@ -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__
>> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
>> index 06ed520..5a5c4d8 100644
>> --- a/fs/xfs/xfs_trans.c
>> +++ b/fs/xfs/xfs_trans.c
>> @@ -531,6 +531,17 @@ xfs_calc_clear_agi_bucket_reservation(
>> }
>>
>> /*
>> + * 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,
Yes, I read the comments of what's the meaning of 128 at xfs_trans.c,
then just copied over xfs_calc_clear_agi_bucket_reservation() to it.
> 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);
> }
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.
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 01/15] xfs: Add a new transaction for changing ag state
2012-12-03 2:54 ` Jeff Liu
@ 2012-12-03 21:28 ` Dave Chinner
2012-12-04 11:56 ` Jeff Liu
0 siblings, 1 reply; 5+ messages in thread
From: Dave Chinner @ 2012-12-03 21:28 UTC (permalink / raw)
To: Jeff Liu; +Cc: xfs
On Mon, Dec 03, 2012 at 10:54:16AM +0800, Jeff Liu wrote:
> On 12/03/2012 09:24 AM, Dave Chinner wrote:
> > 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);
> > }
> 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.
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 :)
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 01/15] xfs: Add a new transaction for changing ag state
2012-12-03 21:28 ` Dave Chinner
@ 2012-12-04 11:56 ` Jeff Liu
0 siblings, 0 replies; 5+ messages in thread
From: Jeff Liu @ 2012-12-04 11:56 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
On 12/04/2012 05:28 AM, Dave Chinner wrote:
> On Mon, Dec 03, 2012 at 10:54:16AM +0800, Jeff Liu wrote:
>> On 12/03/2012 09:24 AM, Dave Chinner wrote:
>>> 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);
>>> }
>> 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.
>
> 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 :)
Just done the changes, I will continue to run a comprehensive testing
tomorrow before posting those patches. :)
Thanks,
-Jeff
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-12-04 21:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-16 6:44 [PATCH 01/15] xfs: Add a new transaction for changing ag state Jeff Liu
2012-12-03 1:24 ` Dave Chinner
2012-12-03 2:54 ` Jeff Liu
2012-12-03 21:28 ` Dave Chinner
2012-12-04 11:56 ` Jeff Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox