2009-03-16 17:40:55

by Aneesh Kumar K.V

[permalink] [raw]
Subject: [PATCH] ext4: Rename pa_linear to pa_type

Impact: code cleanup

This patch rename pa_linear to pa_type and add MB_INODE_PA
and MB_GROUP_PA to indicate inode and group prealloc space.

Signed-off-by: Aneesh Kumar K.V <[email protected]>

---
fs/ext4/mballoc.c | 14 +++++++-------
fs/ext4/mballoc.h | 7 +++++--
2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 4415bee..72331ce 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3696,7 +3696,7 @@ ext4_mb_new_inode_pa(struct ext4_allocation_context *ac)
INIT_LIST_HEAD(&pa->pa_inode_list);
INIT_LIST_HEAD(&pa->pa_group_list);
pa->pa_deleted = 0;
- pa->pa_linear = 0;
+ pa->pa_type = MB_INODE_PA;

mb_debug("new inode pa %p: %llu/%u for %u\n", pa,
pa->pa_pstart, pa->pa_len, pa->pa_lstart);
@@ -3759,7 +3759,7 @@ ext4_mb_new_group_pa(struct ext4_allocation_context *ac)
INIT_LIST_HEAD(&pa->pa_inode_list);
INIT_LIST_HEAD(&pa->pa_group_list);
pa->pa_deleted = 0;
- pa->pa_linear = 1;
+ pa->pa_type = MB_GROUP_PA;

mb_debug("new group pa %p: %llu/%u for %u\n", pa,
pa->pa_pstart, pa->pa_len, pa->pa_lstart);
@@ -4013,7 +4013,7 @@ repeat:
list_del_rcu(&pa->pa_inode_list);
spin_unlock(pa->pa_obj_lock);

- if (pa->pa_linear)
+ if (pa->pa_type == MB_GROUP_PA)
ext4_mb_release_group_pa(&e4b, pa, ac);
else
ext4_mb_release_inode_pa(&e4b, bitmap_bh, pa, ac);
@@ -4113,7 +4113,7 @@ repeat:
spin_unlock(&ei->i_prealloc_lock);

list_for_each_entry_safe(pa, tmp, &list, u.pa_tmp_list) {
- BUG_ON(pa->pa_linear != 0);
+ BUG_ON(pa->pa_type != MB_INODE_PA);
ext4_get_group_no_and_offset(sb, pa->pa_pstart, &group, NULL);

err = ext4_mb_load_buddy(sb, group, &e4b);
@@ -4365,7 +4365,7 @@ ext4_mb_discard_lg_preallocations(struct super_block *sb,
continue;
}
/* only lg prealloc space */
- BUG_ON(!pa->pa_linear);
+ BUG_ON(pa->pa_type != MB_GROUP_PA);

/* seems this one can be freed ... */
pa->pa_deleted = 1;
@@ -4471,7 +4471,7 @@ static int ext4_mb_release_context(struct ext4_allocation_context *ac)
{
struct ext4_prealloc_space *pa = ac->ac_pa;
if (pa) {
- if (pa->pa_linear) {
+ if (pa->pa_type == MB_GROUP_PA) {
/* see comment in ext4_mb_use_group_pa() */
spin_lock(&pa->pa_lock);
pa->pa_pstart += ac->ac_b_ex.fe_len;
@@ -4491,7 +4491,7 @@ static int ext4_mb_release_context(struct ext4_allocation_context *ac)
* doesn't grow big. We need to release
* alloc_semp before calling ext4_mb_add_n_trim()
*/
- if (pa->pa_linear && likely(pa->pa_free)) {
+ if ((pa->pa_type == MB_GROUP_PA) && likely(pa->pa_free)) {
spin_lock(pa->pa_obj_lock);
list_del_rcu(&pa->pa_inode_list);
spin_unlock(pa->pa_obj_lock);
diff --git a/fs/ext4/mballoc.h b/fs/ext4/mballoc.h
index 10a2921..340cc4b 100644
--- a/fs/ext4/mballoc.h
+++ b/fs/ext4/mballoc.h
@@ -132,12 +132,15 @@ struct ext4_prealloc_space {
ext4_lblk_t pa_lstart; /* log. block */
unsigned short pa_len; /* len of preallocated chunk */
unsigned short pa_free; /* how many blocks are free */
- unsigned short pa_linear; /* consumed in one direction
- * strictly, for grp prealloc */
+ unsigned short pa_type; /* pa type. inode or group */
spinlock_t *pa_obj_lock;
struct inode *pa_inode; /* hack, for history only */
};

+enum {
+ MB_INODE_PA = 0,
+ MB_GROUP_PA = 1
+};

struct ext4_free_extent {
ext4_lblk_t fe_logical;
--
tg: (5bee17f..) pa_linear_rename (depends on: master)


2009-03-16 17:54:50

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH] ext4: Rename pa_linear to pa_type

Aneesh Kumar K.V wrote:
> Impact: code cleanup
>
> This patch rename pa_linear to pa_type and add MB_INODE_PA
> and MB_GROUP_PA to indicate inode and group prealloc space.
>
> Signed-off-by: Aneesh Kumar K.V <[email protected]>

Seems reasonable to me. Though I might not put "inode or group" in the
pa_type comments just because that might possibly change later. Not a
big deal though.

Especially this part:

- if (pa->pa_linear)
+ if (pa->pa_type == MB_GROUP_PA)
ext4_mb_release_group_pa(&e4b, pa, ac);

is a lot clearer; other tests for linear did have a bit more to do with
linear use but that's not abstracted enough; it really is inode or group
use. It might be worth keeping a comment somewhere though that the
group prealloc is still used in a linear fashion?

but those are nitpicks...

Reviewed-by: Eric Sandeen <[email protected]>

> ---
> fs/ext4/mballoc.c | 14 +++++++-------
> fs/ext4/mballoc.h | 7 +++++--
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 4415bee..72331ce 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -3696,7 +3696,7 @@ ext4_mb_new_inode_pa(struct ext4_allocation_context *ac)
> INIT_LIST_HEAD(&pa->pa_inode_list);
> INIT_LIST_HEAD(&pa->pa_group_list);
> pa->pa_deleted = 0;
> - pa->pa_linear = 0;
> + pa->pa_type = MB_INODE_PA;
>
> mb_debug("new inode pa %p: %llu/%u for %u\n", pa,
> pa->pa_pstart, pa->pa_len, pa->pa_lstart);
> @@ -3759,7 +3759,7 @@ ext4_mb_new_group_pa(struct ext4_allocation_context *ac)
> INIT_LIST_HEAD(&pa->pa_inode_list);
> INIT_LIST_HEAD(&pa->pa_group_list);
> pa->pa_deleted = 0;
> - pa->pa_linear = 1;
> + pa->pa_type = MB_GROUP_PA;
>
> mb_debug("new group pa %p: %llu/%u for %u\n", pa,
> pa->pa_pstart, pa->pa_len, pa->pa_lstart);
> @@ -4013,7 +4013,7 @@ repeat:
> list_del_rcu(&pa->pa_inode_list);
> spin_unlock(pa->pa_obj_lock);
>
> - if (pa->pa_linear)
> + if (pa->pa_type == MB_GROUP_PA)
> ext4_mb_release_group_pa(&e4b, pa, ac);
> else
> ext4_mb_release_inode_pa(&e4b, bitmap_bh, pa, ac);
> @@ -4113,7 +4113,7 @@ repeat:
> spin_unlock(&ei->i_prealloc_lock);
>
> list_for_each_entry_safe(pa, tmp, &list, u.pa_tmp_list) {
> - BUG_ON(pa->pa_linear != 0);
> + BUG_ON(pa->pa_type != MB_INODE_PA);
> ext4_get_group_no_and_offset(sb, pa->pa_pstart, &group, NULL);
>
> err = ext4_mb_load_buddy(sb, group, &e4b);
> @@ -4365,7 +4365,7 @@ ext4_mb_discard_lg_preallocations(struct super_block *sb,
> continue;
> }
> /* only lg prealloc space */
> - BUG_ON(!pa->pa_linear);
> + BUG_ON(pa->pa_type != MB_GROUP_PA);
>
> /* seems this one can be freed ... */
> pa->pa_deleted = 1;
> @@ -4471,7 +4471,7 @@ static int ext4_mb_release_context(struct ext4_allocation_context *ac)
> {
> struct ext4_prealloc_space *pa = ac->ac_pa;
> if (pa) {
> - if (pa->pa_linear) {
> + if (pa->pa_type == MB_GROUP_PA) {
> /* see comment in ext4_mb_use_group_pa() */
> spin_lock(&pa->pa_lock);
> pa->pa_pstart += ac->ac_b_ex.fe_len;
> @@ -4491,7 +4491,7 @@ static int ext4_mb_release_context(struct ext4_allocation_context *ac)
> * doesn't grow big. We need to release
> * alloc_semp before calling ext4_mb_add_n_trim()
> */
> - if (pa->pa_linear && likely(pa->pa_free)) {
> + if ((pa->pa_type == MB_GROUP_PA) && likely(pa->pa_free)) {
> spin_lock(pa->pa_obj_lock);
> list_del_rcu(&pa->pa_inode_list);
> spin_unlock(pa->pa_obj_lock);
> diff --git a/fs/ext4/mballoc.h b/fs/ext4/mballoc.h
> index 10a2921..340cc4b 100644
> --- a/fs/ext4/mballoc.h
> +++ b/fs/ext4/mballoc.h
> @@ -132,12 +132,15 @@ struct ext4_prealloc_space {
> ext4_lblk_t pa_lstart; /* log. block */
> unsigned short pa_len; /* len of preallocated chunk */
> unsigned short pa_free; /* how many blocks are free */
> - unsigned short pa_linear; /* consumed in one direction
> - * strictly, for grp prealloc */
> + unsigned short pa_type; /* pa type. inode or group */
> spinlock_t *pa_obj_lock;
> struct inode *pa_inode; /* hack, for history only */
> };
>
> +enum {
> + MB_INODE_PA = 0,
> + MB_GROUP_PA = 1
> +};
>
> struct ext4_free_extent {
> ext4_lblk_t fe_logical;


2009-03-27 21:11:34

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: Rename pa_linear to pa_type

I've added this patch to the ext4 patch queue.

- Ted