All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] patch add-ext4_es_store_pblock_status
@ 2014-02-20  0:38 Theodore Ts'o
  2014-02-20  0:42 ` Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2014-02-20  0:38 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

---
 fs/ext4/extents_status.c | 15 +++++++--------
 fs/ext4/extents_status.h |  9 +++++++++
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 3981ff7..0578b6b 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -658,8 +658,7 @@ int ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 
 	newes.es_lblk = lblk;
 	newes.es_len = len;
-	ext4_es_store_pblock(&newes, pblk);
-	ext4_es_store_status(&newes, status);
+	ext4_es_store_pblock_status(&newes, status, pblk);
 	trace_ext4_es_insert_extent(inode, &newes);
 
 	ext4_es_insert_extent_check(inode, &newes);
@@ -699,8 +698,7 @@ void ext4_es_cache_extent(struct inode *inode, ext4_lblk_t lblk,
 
 	newes.es_lblk = lblk;
 	newes.es_len = len;
-	ext4_es_store_pblock(&newes, pblk);
-	ext4_es_store_status(&newes, status);
+	ext4_es_store_pblock_status(&newes, pblk, status);
 	trace_ext4_es_cache_extent(inode, &newes);
 
 	if (!len)
@@ -812,13 +810,14 @@ retry:
 
 			newes.es_lblk = end + 1;
 			newes.es_len = len2;
+			block = 0x7FDEADBEEF;
 			if (ext4_es_is_written(&orig_es) ||
-			    ext4_es_is_unwritten(&orig_es)) {
+			    ext4_es_is_unwritten(&orig_es))
 				block = ext4_es_pblock(&orig_es) +
 					orig_es.es_len - len2;
-				ext4_es_store_pblock(&newes, block);
-			}
-			ext4_es_store_status(&newes, ext4_es_status(&orig_es));
+			ext4_es_store_pblock_status(&newes,
+						    ext4_es_status(&orig_es),
+						    block);
 			err = __es_insert_extent(inode, &newes);
 			if (err) {
 				es->es_lblk = orig_es.es_lblk;
diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
index 167f4ab8..f1b62a4 100644
--- a/fs/ext4/extents_status.h
+++ b/fs/ext4/extents_status.h
@@ -129,6 +129,15 @@ static inline void ext4_es_store_status(struct extent_status *es,
 		       (es->es_pblk & ~ES_MASK));
 }
 
+static inline void ext4_es_store_pblock_status(struct extent_status *es,
+					       ext4_fsblk_t pb,
+					       unsigned int status)
+{
+	es->es_pblk = (((ext4_fsblk_t)
+			(status & EXTENT_STATUS_FLAGS) << ES_SHIFT) |
+		       (pb & ~ES_MASK));
+}
+
 extern void ext4_es_register_shrinker(struct ext4_sb_info *sbi);
 extern void ext4_es_unregister_shrinker(struct ext4_sb_info *sbi);
 extern void ext4_es_lru_add(struct inode *inode);
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] patch add-ext4_es_store_pblock_status
  2014-02-20  0:38 [PATCH] patch add-ext4_es_store_pblock_status Theodore Ts'o
@ 2014-02-20  0:42 ` Darrick J. Wong
  2014-02-20  1:26   ` Theodore Ts'o
  0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2014-02-20  0:42 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Ext4 Developers List

On Wed, Feb 19, 2014 at 07:38:37PM -0500, Theodore Ts'o wrote:
> ---

A changelog would be helpful...

--D

>  fs/ext4/extents_status.c | 15 +++++++--------
>  fs/ext4/extents_status.h |  9 +++++++++
>  2 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 3981ff7..0578b6b 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -658,8 +658,7 @@ int ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>  
>  	newes.es_lblk = lblk;
>  	newes.es_len = len;
> -	ext4_es_store_pblock(&newes, pblk);
> -	ext4_es_store_status(&newes, status);
> +	ext4_es_store_pblock_status(&newes, status, pblk);
>  	trace_ext4_es_insert_extent(inode, &newes);
>  
>  	ext4_es_insert_extent_check(inode, &newes);
> @@ -699,8 +698,7 @@ void ext4_es_cache_extent(struct inode *inode, ext4_lblk_t lblk,
>  
>  	newes.es_lblk = lblk;
>  	newes.es_len = len;
> -	ext4_es_store_pblock(&newes, pblk);
> -	ext4_es_store_status(&newes, status);
> +	ext4_es_store_pblock_status(&newes, pblk, status);
>  	trace_ext4_es_cache_extent(inode, &newes);
>  
>  	if (!len)
> @@ -812,13 +810,14 @@ retry:
>  
>  			newes.es_lblk = end + 1;
>  			newes.es_len = len2;
> +			block = 0x7FDEADBEEF;
>  			if (ext4_es_is_written(&orig_es) ||
> -			    ext4_es_is_unwritten(&orig_es)) {
> +			    ext4_es_is_unwritten(&orig_es))
>  				block = ext4_es_pblock(&orig_es) +
>  					orig_es.es_len - len2;
> -				ext4_es_store_pblock(&newes, block);
> -			}
> -			ext4_es_store_status(&newes, ext4_es_status(&orig_es));
> +			ext4_es_store_pblock_status(&newes,
> +						    ext4_es_status(&orig_es),
> +						    block);
>  			err = __es_insert_extent(inode, &newes);
>  			if (err) {
>  				es->es_lblk = orig_es.es_lblk;
> diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
> index 167f4ab8..f1b62a4 100644
> --- a/fs/ext4/extents_status.h
> +++ b/fs/ext4/extents_status.h
> @@ -129,6 +129,15 @@ static inline void ext4_es_store_status(struct extent_status *es,
>  		       (es->es_pblk & ~ES_MASK));
>  }
>  
> +static inline void ext4_es_store_pblock_status(struct extent_status *es,
> +					       ext4_fsblk_t pb,
> +					       unsigned int status)
> +{
> +	es->es_pblk = (((ext4_fsblk_t)
> +			(status & EXTENT_STATUS_FLAGS) << ES_SHIFT) |
> +		       (pb & ~ES_MASK));
> +}
> +
>  extern void ext4_es_register_shrinker(struct ext4_sb_info *sbi);
>  extern void ext4_es_unregister_shrinker(struct ext4_sb_info *sbi);
>  extern void ext4_es_lru_add(struct inode *inode);
> -- 
> 1.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] patch add-ext4_es_store_pblock_status
  2014-02-20  0:42 ` Darrick J. Wong
@ 2014-02-20  1:26   ` Theodore Ts'o
  2014-02-20  1:28     ` [PATCH] ext4: add ext4_es_store_pblock_status() Theodore Ts'o
  0 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2014-02-20  1:26 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Ext4 Developers List

On Wed, Feb 19, 2014 at 04:42:14PM -0800, Darrick J. Wong wrote:
> On Wed, Feb 19, 2014 at 07:38:37PM -0500, Theodore Ts'o wrote:
> > ---
> 
> A changelog would be helpful...

Whoops, I forgot to run "guilt refresh" before sending out the patch.
Resending....

							- Ted

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] ext4: add ext4_es_store_pblock_status()
  2014-02-20  1:26   ` Theodore Ts'o
@ 2014-02-20  1:28     ` Theodore Ts'o
  2014-02-21  1:59       ` [PATCH -v3] " Theodore Ts'o
  0 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2014-02-20  1:28 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Avoid false positives by static code analysis tools such as sparse and
coverity caused by the fact that we set the physical block, and then
the status in the extent_status structure.  It is also more efficient
to set both of these values at once.

Addresses-Coverity-Id: #989077
Addresses-Coverity-Id: #989078
Addresses-Coverity-Id: #1080722

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/extents_status.c | 15 +++++++--------
 fs/ext4/extents_status.h |  9 +++++++++
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 3981ff7..0578b6b 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -658,8 +658,7 @@ int ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 
 	newes.es_lblk = lblk;
 	newes.es_len = len;
-	ext4_es_store_pblock(&newes, pblk);
-	ext4_es_store_status(&newes, status);
+	ext4_es_store_pblock_status(&newes, status, pblk);
 	trace_ext4_es_insert_extent(inode, &newes);
 
 	ext4_es_insert_extent_check(inode, &newes);
@@ -699,8 +698,7 @@ void ext4_es_cache_extent(struct inode *inode, ext4_lblk_t lblk,
 
 	newes.es_lblk = lblk;
 	newes.es_len = len;
-	ext4_es_store_pblock(&newes, pblk);
-	ext4_es_store_status(&newes, status);
+	ext4_es_store_pblock_status(&newes, pblk, status);
 	trace_ext4_es_cache_extent(inode, &newes);
 
 	if (!len)
@@ -812,13 +810,14 @@ retry:
 
 			newes.es_lblk = end + 1;
 			newes.es_len = len2;
+			block = 0x7FDEADBEEF;
 			if (ext4_es_is_written(&orig_es) ||
-			    ext4_es_is_unwritten(&orig_es)) {
+			    ext4_es_is_unwritten(&orig_es))
 				block = ext4_es_pblock(&orig_es) +
 					orig_es.es_len - len2;
-				ext4_es_store_pblock(&newes, block);
-			}
-			ext4_es_store_status(&newes, ext4_es_status(&orig_es));
+			ext4_es_store_pblock_status(&newes,
+						    ext4_es_status(&orig_es),
+						    block);
 			err = __es_insert_extent(inode, &newes);
 			if (err) {
 				es->es_lblk = orig_es.es_lblk;
diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
index 167f4ab8..f1b62a4 100644
--- a/fs/ext4/extents_status.h
+++ b/fs/ext4/extents_status.h
@@ -129,6 +129,15 @@ static inline void ext4_es_store_status(struct extent_status *es,
 		       (es->es_pblk & ~ES_MASK));
 }
 
+static inline void ext4_es_store_pblock_status(struct extent_status *es,
+					       ext4_fsblk_t pb,
+					       unsigned int status)
+{
+	es->es_pblk = (((ext4_fsblk_t)
+			(status & EXTENT_STATUS_FLAGS) << ES_SHIFT) |
+		       (pb & ~ES_MASK));
+}
+
 extern void ext4_es_register_shrinker(struct ext4_sb_info *sbi);
 extern void ext4_es_unregister_shrinker(struct ext4_sb_info *sbi);
 extern void ext4_es_lru_add(struct inode *inode);
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH -v3] ext4: add ext4_es_store_pblock_status()
  2014-02-20  1:28     ` [PATCH] ext4: add ext4_es_store_pblock_status() Theodore Ts'o
@ 2014-02-21  1:59       ` Theodore Ts'o
  2014-02-21  8:10         ` Zheng Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2014-02-21  1:59 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Avoid false positives by static code analysis tools such as sparse and
coverity caused by the fact that we set the physical block, and then
the status in the extent_status structure.  It is also more efficient
to set both of these values at once.

Addresses-Coverity-Id: #989077
Addresses-Coverity-Id: #989078
Addresses-Coverity-Id: #1080722

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/extents_status.c | 14 ++++++--------
 fs/ext4/extents_status.h |  9 +++++++++
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 3981ff7..a900004 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -658,8 +658,7 @@ int ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 
 	newes.es_lblk = lblk;
 	newes.es_len = len;
-	ext4_es_store_pblock(&newes, pblk);
-	ext4_es_store_status(&newes, status);
+	ext4_es_store_pblock_status(&newes, pblk, status);
 	trace_ext4_es_insert_extent(inode, &newes);
 
 	ext4_es_insert_extent_check(inode, &newes);
@@ -699,8 +698,7 @@ void ext4_es_cache_extent(struct inode *inode, ext4_lblk_t lblk,
 
 	newes.es_lblk = lblk;
 	newes.es_len = len;
-	ext4_es_store_pblock(&newes, pblk);
-	ext4_es_store_status(&newes, status);
+	ext4_es_store_pblock_status(&newes, pblk, status);
 	trace_ext4_es_cache_extent(inode, &newes);
 
 	if (!len)
@@ -812,13 +810,13 @@ retry:
 
 			newes.es_lblk = end + 1;
 			newes.es_len = len2;
+			block = 0x7FDEADBEEF;
 			if (ext4_es_is_written(&orig_es) ||
-			    ext4_es_is_unwritten(&orig_es)) {
+			    ext4_es_is_unwritten(&orig_es))
 				block = ext4_es_pblock(&orig_es) +
 					orig_es.es_len - len2;
-				ext4_es_store_pblock(&newes, block);
-			}
-			ext4_es_store_status(&newes, ext4_es_status(&orig_es));
+			ext4_es_store_pblock_status(&newes, block,
+						    ext4_es_status(&orig_es));
 			err = __es_insert_extent(inode, &newes);
 			if (err) {
 				es->es_lblk = orig_es.es_lblk;
diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
index 167f4ab8..f1b62a4 100644
--- a/fs/ext4/extents_status.h
+++ b/fs/ext4/extents_status.h
@@ -129,6 +129,15 @@ static inline void ext4_es_store_status(struct extent_status *es,
 		       (es->es_pblk & ~ES_MASK));
 }
 
+static inline void ext4_es_store_pblock_status(struct extent_status *es,
+					       ext4_fsblk_t pb,
+					       unsigned int status)
+{
+	es->es_pblk = (((ext4_fsblk_t)
+			(status & EXTENT_STATUS_FLAGS) << ES_SHIFT) |
+		       (pb & ~ES_MASK));
+}
+
 extern void ext4_es_register_shrinker(struct ext4_sb_info *sbi);
 extern void ext4_es_unregister_shrinker(struct ext4_sb_info *sbi);
 extern void ext4_es_lru_add(struct inode *inode);
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH -v3] ext4: add ext4_es_store_pblock_status()
  2014-02-21  1:59       ` [PATCH -v3] " Theodore Ts'o
@ 2014-02-21  8:10         ` Zheng Liu
  0 siblings, 0 replies; 6+ messages in thread
From: Zheng Liu @ 2014-02-21  8:10 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Ext4 Developers List

On Thu, Feb 20, 2014 at 08:59:56PM -0500, Theodore Ts'o wrote:
> Avoid false positives by static code analysis tools such as sparse and
> coverity caused by the fact that we set the physical block, and then
> the status in the extent_status structure.  It is also more efficient
> to set both of these values at once.
> 
> Addresses-Coverity-Id: #989077
> Addresses-Coverity-Id: #989078
> Addresses-Coverity-Id: #1080722
> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

Thanks for fixing this.  It looks good to me.
Reviewed-by: Zheng Liu <wenqing.lz@taobao.com>

                                                - Zheng

> ---
>  fs/ext4/extents_status.c | 14 ++++++--------
>  fs/ext4/extents_status.h |  9 +++++++++
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 3981ff7..a900004 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -658,8 +658,7 @@ int ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>  
>  	newes.es_lblk = lblk;
>  	newes.es_len = len;
> -	ext4_es_store_pblock(&newes, pblk);
> -	ext4_es_store_status(&newes, status);
> +	ext4_es_store_pblock_status(&newes, pblk, status);
>  	trace_ext4_es_insert_extent(inode, &newes);
>  
>  	ext4_es_insert_extent_check(inode, &newes);
> @@ -699,8 +698,7 @@ void ext4_es_cache_extent(struct inode *inode, ext4_lblk_t lblk,
>  
>  	newes.es_lblk = lblk;
>  	newes.es_len = len;
> -	ext4_es_store_pblock(&newes, pblk);
> -	ext4_es_store_status(&newes, status);
> +	ext4_es_store_pblock_status(&newes, pblk, status);
>  	trace_ext4_es_cache_extent(inode, &newes);
>  
>  	if (!len)
> @@ -812,13 +810,13 @@ retry:
>  
>  			newes.es_lblk = end + 1;
>  			newes.es_len = len2;
> +			block = 0x7FDEADBEEF;
>  			if (ext4_es_is_written(&orig_es) ||
> -			    ext4_es_is_unwritten(&orig_es)) {
> +			    ext4_es_is_unwritten(&orig_es))
>  				block = ext4_es_pblock(&orig_es) +
>  					orig_es.es_len - len2;
> -				ext4_es_store_pblock(&newes, block);
> -			}
> -			ext4_es_store_status(&newes, ext4_es_status(&orig_es));
> +			ext4_es_store_pblock_status(&newes, block,
> +						    ext4_es_status(&orig_es));
>  			err = __es_insert_extent(inode, &newes);
>  			if (err) {
>  				es->es_lblk = orig_es.es_lblk;
> diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
> index 167f4ab8..f1b62a4 100644
> --- a/fs/ext4/extents_status.h
> +++ b/fs/ext4/extents_status.h
> @@ -129,6 +129,15 @@ static inline void ext4_es_store_status(struct extent_status *es,
>  		       (es->es_pblk & ~ES_MASK));
>  }
>  
> +static inline void ext4_es_store_pblock_status(struct extent_status *es,
> +					       ext4_fsblk_t pb,
> +					       unsigned int status)
> +{
> +	es->es_pblk = (((ext4_fsblk_t)
> +			(status & EXTENT_STATUS_FLAGS) << ES_SHIFT) |
> +		       (pb & ~ES_MASK));
> +}
> +
>  extern void ext4_es_register_shrinker(struct ext4_sb_info *sbi);
>  extern void ext4_es_unregister_shrinker(struct ext4_sb_info *sbi);
>  extern void ext4_es_lru_add(struct inode *inode);
> -- 
> 1.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-21  8:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-20  0:38 [PATCH] patch add-ext4_es_store_pblock_status Theodore Ts'o
2014-02-20  0:42 ` Darrick J. Wong
2014-02-20  1:26   ` Theodore Ts'o
2014-02-20  1:28     ` [PATCH] ext4: add ext4_es_store_pblock_status() Theodore Ts'o
2014-02-21  1:59       ` [PATCH -v3] " Theodore Ts'o
2014-02-21  8:10         ` Zheng Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.