[PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and blocks_count fields in nilfs_root struct

[PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and blocks_count fields in nilfs_root struct

[Vyacheslav Dubeyko]

From: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
Subject: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and
 blocks_count fields in nilfs_root struct

The cp_inodes_count and cp_blocks_count are represented as
__le64 type in on-disk structure (struct nilfs_checkpoint).
But analogous fields in in-core structure (struct nilfs_root)
are represented by atomic_t type.

This patch replaces atomic_t on atomic_long_t type in representation
of inodes_count and blocks_count fields in struct nilfs_root.

Signed-off-by: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
CC: Ryusuke Konishi <konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
---
 fs/nilfs2/ifile.c     |    2 +-
 fs/nilfs2/inode.c     |    8 ++++----
 fs/nilfs2/segment.c   |    4 ++--
 fs/nilfs2/super.c     |    8 +++++---
 fs/nilfs2/the_nilfs.c |    4 ++--
 fs/nilfs2/the_nilfs.h |    4 ++--
 6 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/fs/nilfs2/ifile.c b/fs/nilfs2/ifile.c
index d788a59..d540a42 100644
--- a/fs/nilfs2/ifile.c
+++ b/fs/nilfs2/ifile.c
@@ -174,7 +174,7 @@ int nilfs_ifile_count_free_inodes(struct inode *ifile,
 	*nmaxinodes = 0;
 	*nfreeinodes = 0;
 
-	nused = atomic_read(&NILFS_I(ifile)->i_root->inodes_count);
+	nused = atomic_long_read(&NILFS_I(ifile)->i_root->inodes_count);
 	err = nilfs_palloc_count_max_entries(ifile, nused, nmaxinodes);
 	if (likely(!err))
 		*nfreeinodes = *nmaxinodes - nused;
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 689fb60..9e13255 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -54,7 +54,7 @@ void nilfs_inode_add_blocks(struct inode *inode, int n)
 
 	inode_add_bytes(inode, (1 << inode->i_blkbits) * n);
 	if (root)
-		atomic_add(n, &root->blocks_count);
+		atomic_long_add(n, &root->blocks_count);
 }
 
 void nilfs_inode_sub_blocks(struct inode *inode, int n)
@@ -63,7 +63,7 @@ void nilfs_inode_sub_blocks(struct inode *inode, int n)
 
 	inode_sub_bytes(inode, (1 << inode->i_blkbits) * n);
 	if (root)
-		atomic_sub(n, &root->blocks_count);
+		atomic_long_sub(n, &root->blocks_count);
 }
 
 /**
@@ -350,7 +350,7 @@ struct inode *nilfs_new_inode(struct inode *dir, umode_t mode)
 		goto failed_ifile_create_inode;
 	/* reference count of i_bh inherits from nilfs_mdt_read_block() */
 
-	atomic_inc(&root->inodes_count);
+	atomic_long_inc(&root->inodes_count);
 	inode_init_owner(inode, dir, mode);
 	inode->i_ino = ino;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
@@ -782,7 +782,7 @@ void nilfs_evict_inode(struct inode *inode)
 
 	ret = nilfs_ifile_delete_inode(ii->i_root->ifile, inode->i_ino);
 	if (!ret)
-		atomic_dec(&ii->i_root->inodes_count);
+		atomic_long_dec(&ii->i_root->inodes_count);
 
 	nilfs_clear_inode(inode);
 
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index a5752a58..051f67b 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -835,9 +835,9 @@ static int nilfs_segctor_fill_in_checkpoint(struct nilfs_sc_info *sci)
 	raw_cp->cp_snapshot_list.ssl_next = 0;
 	raw_cp->cp_snapshot_list.ssl_prev = 0;
 	raw_cp->cp_inodes_count =
-		cpu_to_le64(atomic_read(&sci->sc_root->inodes_count));
+		cpu_to_le64(atomic_long_read(&sci->sc_root->inodes_count));
 	raw_cp->cp_blocks_count =
-		cpu_to_le64(atomic_read(&sci->sc_root->blocks_count));
+		cpu_to_le64(atomic_long_read(&sci->sc_root->blocks_count));
 	raw_cp->cp_nblk_inc =
 		cpu_to_le64(sci->sc_nblk_inc + sci->sc_nblk_this_inc);
 	raw_cp->cp_create = cpu_to_le64(sci->sc_seg_ctime);
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index acd110f..f1ce6d4 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -554,8 +554,10 @@ int nilfs_attach_checkpoint(struct super_block *sb, __u64 cno, int curr_mnt,
 	if (err)
 		goto failed_bh;
 
-	atomic_set(&root->inodes_count, le64_to_cpu(raw_cp->cp_inodes_count));
-	atomic_set(&root->blocks_count, le64_to_cpu(raw_cp->cp_blocks_count));
+	atomic_long_set(&root->inodes_count,
+			le64_to_cpu(raw_cp->cp_inodes_count));
+	atomic_long_set(&root->blocks_count,
+			le64_to_cpu(raw_cp->cp_blocks_count));
 
 	nilfs_cpfile_put_checkpoint(nilfs->ns_cpfile, cno, bh_cp);
 
@@ -647,7 +649,7 @@ static int nilfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 			 * curent inodes count as maximum possible and
 			 * zero as free inodes value.
 			 */
-			nmaxinodes = atomic_read(&root->inodes_count);
+			nmaxinodes = atomic_long_read(&root->inodes_count);
 			nfreeinodes = 0;
 			err = 0;
 		} else
diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
index 41e6a04..d005510 100644
--- a/fs/nilfs2/the_nilfs.c
+++ b/fs/nilfs2/the_nilfs.c
@@ -764,8 +764,8 @@ nilfs_find_or_create_root(struct the_nilfs *nilfs, __u64 cno)
 	new->ifile = NULL;
 	new->nilfs = nilfs;
 	atomic_set(&new->count, 1);
-	atomic_set(&new->inodes_count, 0);
-	atomic_set(&new->blocks_count, 0);
+	atomic_long_set(&new->inodes_count, 0);
+	atomic_long_set(&new->blocks_count, 0);
 
 	rb_link_node(&new->rb_node, parent, p);
 	rb_insert_color(&new->rb_node, &nilfs->ns_cptree);
diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
index be1267a..e37a246 100644
--- a/fs/nilfs2/the_nilfs.h
+++ b/fs/nilfs2/the_nilfs.h
@@ -241,8 +241,8 @@ struct nilfs_root {
 	struct the_nilfs *nilfs;
 	struct inode *ifile;
 
-	atomic_t inodes_count;
-	atomic_t blocks_count;
+	atomic_long_t inodes_count;
+	atomic_long_t blocks_count;
 };
 
 /* Special checkpoint number */
-- 
1.7.9.5



--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and blocks_count fields in nilfs_root struct

[Ryusuke Konishi]

On Fri, 24 May 2013 17:32:52 +0400, Vyacheslav Dubeyko wrote:
> From: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
> Subject: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and
>  blocks_count fields in nilfs_root struct
> 
> The cp_inodes_count and cp_blocks_count are represented as
> __le64 type in on-disk structure (struct nilfs_checkpoint).
> But analogous fields in in-core structure (struct nilfs_root)
> are represented by atomic_t type.
> 
> This patch replaces atomic_t on atomic_long_t type in representation
> of inodes_count and blocks_count fields in struct nilfs_root.
> 
> Signed-off-by: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org>
> CC: Ryusuke Konishi <konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>

Acked-by: Ryusuke Konishi <konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>


Thanks,
Ryusuke Konishi

> ---
>  fs/nilfs2/ifile.c     |    2 +-
>  fs/nilfs2/inode.c     |    8 ++++----
>  fs/nilfs2/segment.c   |    4 ++--
>  fs/nilfs2/super.c     |    8 +++++---
>  fs/nilfs2/the_nilfs.c |    4 ++--
>  fs/nilfs2/the_nilfs.h |    4 ++--
>  6 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/nilfs2/ifile.c b/fs/nilfs2/ifile.c
> index d788a59..d540a42 100644
> --- a/fs/nilfs2/ifile.c
> +++ b/fs/nilfs2/ifile.c
> @@ -174,7 +174,7 @@ int nilfs_ifile_count_free_inodes(struct inode *ifile,
>  	*nmaxinodes = 0;
>  	*nfreeinodes = 0;
>  
> -	nused = atomic_read(&NILFS_I(ifile)->i_root->inodes_count);
> +	nused = atomic_long_read(&NILFS_I(ifile)->i_root->inodes_count);
>  	err = nilfs_palloc_count_max_entries(ifile, nused, nmaxinodes);
>  	if (likely(!err))
>  		*nfreeinodes = *nmaxinodes - nused;
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index 689fb60..9e13255 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -54,7 +54,7 @@ void nilfs_inode_add_blocks(struct inode *inode, int n)
>  
>  	inode_add_bytes(inode, (1 << inode->i_blkbits) * n);
>  	if (root)
> -		atomic_add(n, &root->blocks_count);
> +		atomic_long_add(n, &root->blocks_count);
>  }
>  
>  void nilfs_inode_sub_blocks(struct inode *inode, int n)
> @@ -63,7 +63,7 @@ void nilfs_inode_sub_blocks(struct inode *inode, int n)
>  
>  	inode_sub_bytes(inode, (1 << inode->i_blkbits) * n);
>  	if (root)
> -		atomic_sub(n, &root->blocks_count);
> +		atomic_long_sub(n, &root->blocks_count);
>  }
>  
>  /**
> @@ -350,7 +350,7 @@ struct inode *nilfs_new_inode(struct inode *dir, umode_t mode)
>  		goto failed_ifile_create_inode;
>  	/* reference count of i_bh inherits from nilfs_mdt_read_block() */
>  
> -	atomic_inc(&root->inodes_count);
> +	atomic_long_inc(&root->inodes_count);
>  	inode_init_owner(inode, dir, mode);
>  	inode->i_ino = ino;
>  	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
> @@ -782,7 +782,7 @@ void nilfs_evict_inode(struct inode *inode)
>  
>  	ret = nilfs_ifile_delete_inode(ii->i_root->ifile, inode->i_ino);
>  	if (!ret)
> -		atomic_dec(&ii->i_root->inodes_count);
> +		atomic_long_dec(&ii->i_root->inodes_count);
>  
>  	nilfs_clear_inode(inode);
>  
> diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
> index a5752a58..051f67b 100644
> --- a/fs/nilfs2/segment.c
> +++ b/fs/nilfs2/segment.c
> @@ -835,9 +835,9 @@ static int nilfs_segctor_fill_in_checkpoint(struct nilfs_sc_info *sci)
>  	raw_cp->cp_snapshot_list.ssl_next = 0;
>  	raw_cp->cp_snapshot_list.ssl_prev = 0;
>  	raw_cp->cp_inodes_count =
> -		cpu_to_le64(atomic_read(&sci->sc_root->inodes_count));
> +		cpu_to_le64(atomic_long_read(&sci->sc_root->inodes_count));
>  	raw_cp->cp_blocks_count =
> -		cpu_to_le64(atomic_read(&sci->sc_root->blocks_count));
> +		cpu_to_le64(atomic_long_read(&sci->sc_root->blocks_count));
>  	raw_cp->cp_nblk_inc =
>  		cpu_to_le64(sci->sc_nblk_inc + sci->sc_nblk_this_inc);
>  	raw_cp->cp_create = cpu_to_le64(sci->sc_seg_ctime);
> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> index acd110f..f1ce6d4 100644
> --- a/fs/nilfs2/super.c
> +++ b/fs/nilfs2/super.c
> @@ -554,8 +554,10 @@ int nilfs_attach_checkpoint(struct super_block *sb, __u64 cno, int curr_mnt,
>  	if (err)
>  		goto failed_bh;
>  
> -	atomic_set(&root->inodes_count, le64_to_cpu(raw_cp->cp_inodes_count));
> -	atomic_set(&root->blocks_count, le64_to_cpu(raw_cp->cp_blocks_count));
> +	atomic_long_set(&root->inodes_count,
> +			le64_to_cpu(raw_cp->cp_inodes_count));
> +	atomic_long_set(&root->blocks_count,
> +			le64_to_cpu(raw_cp->cp_blocks_count));
>  
>  	nilfs_cpfile_put_checkpoint(nilfs->ns_cpfile, cno, bh_cp);
>  
> @@ -647,7 +649,7 @@ static int nilfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>  			 * curent inodes count as maximum possible and
>  			 * zero as free inodes value.
>  			 */
> -			nmaxinodes = atomic_read(&root->inodes_count);
> +			nmaxinodes = atomic_long_read(&root->inodes_count);
>  			nfreeinodes = 0;
>  			err = 0;
>  		} else
> diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
> index 41e6a04..d005510 100644
> --- a/fs/nilfs2/the_nilfs.c
> +++ b/fs/nilfs2/the_nilfs.c
> @@ -764,8 +764,8 @@ nilfs_find_or_create_root(struct the_nilfs *nilfs, __u64 cno)
>  	new->ifile = NULL;
>  	new->nilfs = nilfs;
>  	atomic_set(&new->count, 1);
> -	atomic_set(&new->inodes_count, 0);
> -	atomic_set(&new->blocks_count, 0);
> +	atomic_long_set(&new->inodes_count, 0);
> +	atomic_long_set(&new->blocks_count, 0);
>  
>  	rb_link_node(&new->rb_node, parent, p);
>  	rb_insert_color(&new->rb_node, &nilfs->ns_cptree);
> diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
> index be1267a..e37a246 100644
> --- a/fs/nilfs2/the_nilfs.h
> +++ b/fs/nilfs2/the_nilfs.h
> @@ -241,8 +241,8 @@ struct nilfs_root {
>  	struct the_nilfs *nilfs;
>  	struct inode *ifile;
>  
> -	atomic_t inodes_count;
> -	atomic_t blocks_count;
> +	atomic_long_t inodes_count;
> +	atomic_long_t blocks_count;
>  };
>  
>  /* Special checkpoint number */
> -- 
> 1.7.9.5
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and blocks_count fields in nilfs_root struct

[Jörn Engel]

On Fri, 24 May 2013 17:32:52 +0400, Vyacheslav Dubeyko wrote:
>
> Subject: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count
...
> The cp_inodes_count and cp_blocks_count are represented as
> __le64 type in on-disk structure (struct nilfs_checkpoint).

Isn't atomic_long_t defined to be 32bit on 32bit architectures?

Jörn

--
What one programmer can do in one month, two programmers can do in two months.
-- Fred Brooks
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and blocks_count fields in nilfs_root struct

[Vyacheslav Dubeyko]

On May 24, 2013, at 9:54 PM, Jörn Engel wrote:

> On Fri, 24 May 2013 17:32:52 +0400, Vyacheslav Dubeyko wrote:
>> 
>> Subject: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count
> ...
>> The cp_inodes_count and cp_blocks_count are represented as
>> __le64 type in on-disk structure (struct nilfs_checkpoint).
> 
> Isn't atomic_long_t defined to be 32bit on 32bit architectures?
> 

As I understand, yes. Please, see link:
http://lxr.free-electrons.com/source/include/asm-generic/atomic-long.h#L141

With the best regards,
Vyacheslav Dubeyko.

> Jörn
> 
> --
> What one programmer can do in one month, two programmers can do in two months.
> -- Fred Brooks
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and blocks_count fields in nilfs_root struct

[Jörn Engel]

On Fri, 24 May 2013 23:30:10 +0400, Vyacheslav Dubeyko wrote:
> On May 24, 2013, at 9:54 PM, Jörn Engel wrote:
> > On Fri, 24 May 2013 17:32:52 +0400, Vyacheslav Dubeyko wrote:
> >> 
> >> Subject: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count
> > ...
> >> The cp_inodes_count and cp_blocks_count are represented as
> >> __le64 type in on-disk structure (struct nilfs_checkpoint).
> > 
> > Isn't atomic_long_t defined to be 32bit on 32bit architectures?
> 
> As I understand, yes.

And it doesn't concern you to use a 32bit memory structure to
represent a 64bit on-disk structure? ;)

Jörn

--
Measure. Don't tune for speed until you've measured, and even then
don't unless one part of the code overwhelms the rest.
-- Rob Pike
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and blocks_count fields in nilfs_root struct

[Vyacheslav Dubeyko]

On May 24, 2013, at 11:01 PM, Jörn Engel wrote:

> On Fri, 24 May 2013 23:30:10 +0400, Vyacheslav Dubeyko wrote:
>> On May 24, 2013, at 9:54 PM, Jörn Engel wrote:
>>> On Fri, 24 May 2013 17:32:52 +0400, Vyacheslav Dubeyko wrote:
>>>> 
>>>> Subject: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count
>>> ...
>>>> The cp_inodes_count and cp_blocks_count are represented as
>>>> __le64 type in on-disk structure (struct nilfs_checkpoint).
>>> 
>>> Isn't atomic_long_t defined to be 32bit on 32bit architectures?
>> 
>> As I understand, yes.
> 
> And it doesn't concern you to use a 32bit memory structure to
> represent a 64bit on-disk structure? ;)
> 

I suppose that you mean possibility to mount NILFS2 volume under 32 bit architecture
after working with this volume under 64 bit architecture. Am I correct?

Have you any concrete remarks about code of the patch? Feel free to offer your vision.

Thanks,
Vyacheslav Dubeyko.

> Jörn
> 
> --
> Measure. Don't tune for speed until you've measured, and even then
> don't unless one part of the code overwhelms the rest.
> -- Rob Pike
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and blocks_count fields in nilfs_root struct

[Jörn Engel]

On Sat, 25 May 2013 02:01:04 +0400, Vyacheslav Dubeyko wrote:
> On May 24, 2013, at 11:01 PM, Jörn Engel wrote:
> > On Fri, 24 May 2013 23:30:10 +0400, Vyacheslav Dubeyko wrote:
> >> On May 24, 2013, at 9:54 PM, Jörn Engel wrote:
> >>> On Fri, 24 May 2013 17:32:52 +0400, Vyacheslav Dubeyko wrote:
> >>>> 
> >>>> Subject: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count
> >>> ...
> >>>> The cp_inodes_count and cp_blocks_count are represented as
> >>>> __le64 type in on-disk structure (struct nilfs_checkpoint).
> >>> 
> >>> Isn't atomic_long_t defined to be 32bit on 32bit architectures?
> >> 
> >> As I understand, yes.
> > 
> > And it doesn't concern you to use a 32bit memory structure to
> > represent a 64bit on-disk structure? ;)
> 
> I suppose that you mean possibility to mount NILFS2 volume under 32 bit architecture
> after working with this volume under 64 bit architecture. Am I correct?
> 
> Have you any concrete remarks about code of the patch? Feel free to offer your vision.

Replace atomic_long_t with atomic64_t and replace the various accessor
functions.  The on-disk data structure is 64bit, so of course you use
a 64bit in-memory structure.

Does it really take a vision to come up with this idea?

Jörn

--
People will accept your ideas much more readily if you tell them
that Benjamin Franklin said it first.
-- unknown
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and blocks_count fields in nilfs_root struct

[Ryusuke Konishi]

On Fri, 24 May 2013 17:37:12 -0400, Jörn Engel wrote:
> On Sat, 25 May 2013 02:01:04 +0400, Vyacheslav Dubeyko wrote:
>> On May 24, 2013, at 11:01 PM, Jörn Engel wrote:
>> > On Fri, 24 May 2013 23:30:10 +0400, Vyacheslav Dubeyko wrote:
>> >> On May 24, 2013, at 9:54 PM, Jörn Engel wrote:
>> >>> On Fri, 24 May 2013 17:32:52 +0400, Vyacheslav Dubeyko wrote:
>> >>>> 
>> >>>> Subject: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count
>> >>> ...
>> >>>> The cp_inodes_count and cp_blocks_count are represented as
>> >>>> __le64 type in on-disk structure (struct nilfs_checkpoint).
>> >>> 
>> >>> Isn't atomic_long_t defined to be 32bit on 32bit architectures?
>> >> 
>> >> As I understand, yes.
>> > 
>> > And it doesn't concern you to use a 32bit memory structure to
>> > represent a 64bit on-disk structure? ;)
>> 
>> I suppose that you mean possibility to mount NILFS2 volume under 32 bit architecture
>> after working with this volume under 64 bit architecture. Am I correct?
>> 
>> Have you any concrete remarks about code of the patch? Feel free to offer your vision.
> 
> Replace atomic_long_t with atomic64_t and replace the various accessor
> functions.  The on-disk data structure is 64bit, so of course you use
> a 64bit in-memory structure.
> 
> Does it really take a vision to come up with this idea?

Things are not so simple.  It is not guaranteed that 32-bit
architectures can handle 64-bit on-disk values.  (Think ino_t, page
index, or nrpages.  These are defined with "unsigned long".)

However, these "inodes_count" and "blocks_count" are directly
corresponding to their on-disk counterparts, so using atomic64_t looks
more intuitive for most people; using atomic_long_t for these was
confusing.

Ok, I agree.

Vyacheslav, could you please consider taking in Jörn's comment ?


With regards,
Ryusuke Konishi
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and blocks_count fields in nilfs_root struct

[Vyacheslav Dubeyko]

Hi Ryusuke,

On May 25, 2013, at 7:33 AM, Ryusuke Konishi wrote:

> On Fri, 24 May 2013 17:37:12 -0400, Jörn Engel wrote:
>> On Sat, 25 May 2013 02:01:04 +0400, Vyacheslav Dubeyko wrote:
>>> On May 24, 2013, at 11:01 PM, Jörn Engel wrote:
>>>> On Fri, 24 May 2013 23:30:10 +0400, Vyacheslav Dubeyko wrote:
>>>>> On May 24, 2013, at 9:54 PM, Jörn Engel wrote:
>>>>>> On Fri, 24 May 2013 17:32:52 +0400, Vyacheslav Dubeyko wrote:
>>>>>>> 
>>>>>>> Subject: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count
>>>>>> ...
>>>>>>> The cp_inodes_count and cp_blocks_count are represented as
>>>>>>> __le64 type in on-disk structure (struct nilfs_checkpoint).
>>>>>> 
>>>>>> Isn't atomic_long_t defined to be 32bit on 32bit architectures?
>>>>> 
>>>>> As I understand, yes.
>>>> 
>>>> And it doesn't concern you to use a 32bit memory structure to
>>>> represent a 64bit on-disk structure? ;)
>>> 
>>> I suppose that you mean possibility to mount NILFS2 volume under 32 bit architecture
>>> after working with this volume under 64 bit architecture. Am I correct?
>>> 
>>> Have you any concrete remarks about code of the patch? Feel free to offer your vision.
>> 
>> Replace atomic_long_t with atomic64_t and replace the various accessor
>> functions.  The on-disk data structure is 64bit, so of course you use
>> a 64bit in-memory structure.
>> 
>> Does it really take a vision to come up with this idea?
> 
> Things are not so simple.  It is not guaranteed that 32-bit
> architectures can handle 64-bit on-disk values.  (Think ino_t, page
> index, or nrpages.  These are defined with "unsigned long".)
> 
> However, these "inodes_count" and "blocks_count" are directly
> corresponding to their on-disk counterparts, so using atomic64_t looks
> more intuitive for most people; using atomic_long_t for these was
> confusing.
> 
> Ok, I agree.
> 
> Vyacheslav, could you please consider taking in Jörn's comment ?
> 

So, I have such understanding. Please, correct me if I'm wrong.

The atomic_long_t and atomic64_t types are 64 bits on 64-bit architecture.
The atomic_long_t type is 32 bits on 32-bit architecture. So, If we want to have
64 bits atomic counter on 32-bit architecture then we need to use generic atomic64_t
implementation (asm-generic/atomic64.t). But generic 64-bit atomic support
depends from CONFIG_GENERIC_ATOMIC64 configuration option
(http://lxr.free-electrons.com/source/include/linux/atomic.h#L127). As a result,
NILFS2 driver needs to have dependance from CONFIG_GENERIC_ATOMIC64
configuration option on 32-bit architecture configuration.

With the best regards,
Vyacheslav Dubeyko.

> 
> With regards,
> Ryusuke Konishi

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count and blocks_count fields in nilfs_root struct

[Ryusuke Konishi]

On Sun, 26 May 2013 16:18:20 +0400, Vyacheslav Dubeyko wrote:
> Hi Ryusuke,
> 
> On May 25, 2013, at 7:33 AM, Ryusuke Konishi wrote:
> 
>> On Fri, 24 May 2013 17:37:12 -0400, Jörn Engel wrote:
>>> On Sat, 25 May 2013 02:01:04 +0400, Vyacheslav Dubeyko wrote:
>>>> On May 24, 2013, at 11:01 PM, Jörn Engel wrote:
>>>>> On Fri, 24 May 2013 23:30:10 +0400, Vyacheslav Dubeyko wrote:
>>>>>> On May 24, 2013, at 9:54 PM, Jörn Engel wrote:
>>>>>>> On Fri, 24 May 2013 17:32:52 +0400, Vyacheslav Dubeyko wrote:
>>>>>>>> 
>>>>>>>> Subject: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count
>>>>>>> ...
>>>>>>>> The cp_inodes_count and cp_blocks_count are represented as
>>>>>>>> __le64 type in on-disk structure (struct nilfs_checkpoint).
>>>>>>> 
>>>>>>> Isn't atomic_long_t defined to be 32bit on 32bit architectures?
>>>>>> 
>>>>>> As I understand, yes.
>>>>> 
>>>>> And it doesn't concern you to use a 32bit memory structure to
>>>>> represent a 64bit on-disk structure? ;)
>>>> 
>>>> I suppose that you mean possibility to mount NILFS2 volume under 32 bit architecture
>>>> after working with this volume under 64 bit architecture. Am I correct?
>>>> 
>>>> Have you any concrete remarks about code of the patch? Feel free to offer your vision.
>>> 
>>> Replace atomic_long_t with atomic64_t and replace the various accessor
>>> functions.  The on-disk data structure is 64bit, so of course you use
>>> a 64bit in-memory structure.
>>> 
>>> Does it really take a vision to come up with this idea?
>> 
>> Things are not so simple.  It is not guaranteed that 32-bit
>> architectures can handle 64-bit on-disk values.  (Think ino_t, page
>> index, or nrpages.  These are defined with "unsigned long".)
>> 
>> However, these "inodes_count" and "blocks_count" are directly
>> corresponding to their on-disk counterparts, so using atomic64_t looks
>> more intuitive for most people; using atomic_long_t for these was
>> confusing.
>> 
>> Ok, I agree.
>> 
>> Vyacheslav, could you please consider taking in Jörn's comment ?
>> 
> 
> So, I have such understanding. Please, correct me if I'm wrong.
> 
> The atomic_long_t and atomic64_t types are 64 bits on 64-bit architecture.
> The atomic_long_t type is 32 bits on 32-bit architecture. So, If we want to have
> 64 bits atomic counter on 32-bit architecture then we need to use generic atomic64_t
> implementation (asm-generic/atomic64.t). But generic 64-bit atomic support
> depends from CONFIG_GENERIC_ATOMIC64 configuration option
> (http://lxr.free-electrons.com/source/include/linux/atomic.h#L127). As a result,
> NILFS2 driver needs to have dependance from CONFIG_GENERIC_ATOMIC64
> configuration option on 32-bit architecture configuration.

You don't have to add denpendency to CONFIG_GENERIC_ATOMIC64.

In my understanding, the generic atomic64 operations (i.e. routines in
lib/atomic64.c) are only used when selected architecture doesn't have
own implementation.

For instance, sparc32 uses the generic implementation, but x86_64
architecture uses arch/x86/include/asm/atomic64_64.h, and x86_32 uses
arch/x86/include/asm/atomic64_32.h and
arch/x86/lib/atomic64_{386,cx8}_32.S for 32-bit.

CONFIG_GENERIC_ATOMIC64 is selected by arch/*/Kconfig in the former
case.  It is not intended to be selected directly by users.

With regards,
Ryusuke Konishi
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html