byteorder: introduce le32_add_cpu & friends to core
Subsystems (2)
RFC PATCH 1/1 byteorder: introduce le32_add_cpu & friends to core
There are many places where these functions would be useful.
(just look at: grep -r 'cpu_to_[ble12346]*([ble12346]*_to_cpu.*[-+]' linux-src/) What do you think?
ps: this patch depends on http://lkml.org/lkml/2007/12/25/35
--
add inline functions which add native byte order variable to little/big endian variable to core header and as an example convert ext3 to use them
le16_add_cpu(__le16 *var, u16 val)le32_add_cpu(__le32 *var, u32 val)le64_add_cpu(__le64 *var, u64 val)be16_add_cpu(__be16 *var, u16 val)be32_add_cpu(__be32 *var, u32 val)be64_add_cpu(__be64 *var, u64 val)
--- fs/ext3/balloc.c | 7 +---- fs/ext3/ialloc.c | 12 +++------ fs/ext3/resize.c | 12 +++------ fs/ext3/super.c | 2 +- fs/ext3/xattr.c | 6 +--- fs/ocfs2/cluster/endian.h | 30 ------------------------ fs/ocfs2/cluster/nodemanager.c | 1 - fs/ocfs2/dlm/dlmast.c | 1 - fs/ocfs2/endian.h | 45 ------------------------------------- fs/ocfs2/ocfs2.h | 1 - include/linux/byteorder/generic.h | 30 ++++++++++++++++++++++++ 11 files changed, 43 insertions(+), 104 deletions(-) delete mode 100644 fs/ocfs2/cluster/endian.h delete mode 100644 fs/ocfs2/endian.h
fs/ext3/balloc.c +2 -5
@@ -566,9 +566,7 @@ do_more:jbd_unlock_bh_state (bitmap_bh);spin_lock (sb_bgl_lock (sbi, block_group)); - desc->bg_free_blocks_count = -cpu_to_le16 (le16_to_cpu (desc->bg_free_blocks_count) + - group_freed); +le16_add_cpu (&desc->bg_free_blocks_count, group_freed);spin_unlock (sb_bgl_lock (sbi, block_group));percpu_counter_add (&sbi->s_freeblocks_counter, count);
@@ -1630,8 +1628,7 @@ allocated: ret_block, goal_hits, goal_attempts);spin_lock (sb_bgl_lock (sbi, group_no)); - gdp->bg_free_blocks_count = -cpu_to_le16 (le16_to_cpu (gdp->bg_free_blocks_count)-num); +le16_add_cpu (&gdp->bg_free_blocks_count, -num);spin_unlock (sb_bgl_lock (sbi, group_no));percpu_counter_sub (&sbi->s_freeblocks_counter, num);
fs/ext3/ialloc.c +4 -8
@@ -164,11 +164,9 @@ void ext3_free_inode (handle_t *handle, struct inode * inode)if (gdp) {spin_lock (sb_bgl_lock (sbi, block_group)); - gdp->bg_free_inodes_count =cpu_to_le16 ( -le16_to_cpu (gdp->bg_free_inodes_count) +1 ); +le16_add_cpu (&gdp->bg_free_inodes_count,1 );if (is_directory) - gdp->bg_used_dirs_count =cpu_to_le16 ( -le16_to_cpu (gdp->bg_used_dirs_count) -1 ); +le16_add_cpu (&gdp->bg_used_dirs_count,-1 );spin_unlock (sb_bgl_lock (sbi, block_group));percpu_counter_inc (&sbi->s_freeinodes_counter);if (is_directory)
@@ -527,11 +525,9 @@ got: err =ext3_journal_get_write_access (handle, bh2);if (err) goto fail;spin_lock (sb_bgl_lock (sbi, group)); - gdp->bg_free_inodes_count = -cpu_to_le16 (le16_to_cpu (gdp->bg_free_inodes_count) -1 ); +le16_add_cpu (&gdp->bg_free_inodes_count,-1 );if (S_ISDIR (mode)) { - gdp->bg_used_dirs_count = -cpu_to_le16 (le16_to_cpu (gdp->bg_used_dirs_count) +1 ); +le16_add_cpu (&gdp->bg_used_dirs_count,1 ); }spin_unlock (sb_bgl_lock (sbi, group));BUFFER_TRACE (bh2,"call ext3_journal_dirty_metadata" );
fs/ext3/resize.c +4 -8
@@ -518,8 +518,7 @@ static int add_new_gdb(handle_t *handle, struct inode *inode,EXT3_SB (sb)->s_gdb_count++;kfree (o_group_desc); - es->s_reserved_gdt_blocks = -cpu_to_le16 (le16_to_cpu (es->s_reserved_gdt_blocks) -1 ); +le16_add_cpu (&es->s_reserved_gdt_blocks,-1 );ext3_journal_dirty_metadata (handle,EXT3_SB (sb)->s_sbh);return 0 ;
@@ -891,10 +890,8 @@ int ext3_group_add(struct super_block *sb, struct ext3_new_group_data *input)* blocks/inodes before the group is live won't actually let us * allocate the new space yet. */ - es->s_blocks_count =cpu_to_le32 (le32_to_cpu (es->s_blocks_count) + - input->blocks_count); - es->s_inodes_count =cpu_to_le32 (le32_to_cpu (es->s_inodes_count) + -EXT3_INODES_PER_GROUP (sb)); +le32_add_cpu (&es->s_blocks_count, input->blocks_count); +le32_add_cpu (&es->s_inodes_count,EXT3_INODES_PER_GROUP (sb));/* * We need to protect s_groups_count against other CPUs seeing
@@ -927,8 +924,7 @@ int ext3_group_add(struct super_block *sb, struct ext3_new_group_data *input)/* Update the reserved block counts only once the new group is * active. */ - es->s_r_blocks_count =cpu_to_le32 (le32_to_cpu (es->s_r_blocks_count) + - input->reserved_blocks); +le32_add_cpu (&es->s_r_blocks_count, input->reserved_blocks);/* Update the free space counts */ percpu_counter_add(&sbi->s_freeblocks_counter,
fs/ext3/super.c +1 -1
@@ -1224,7 +1224,7 @@ static int ext3_setup_super(struct super_block *sb, struct ext3_super_block *es,#endif if (!(__s16 )le16_to_cpu (es->s_max_mnt_count)) es->s_max_mnt_count =cpu_to_le16 (EXT3_DFL_MAX_MNT_COUNT ); - es->s_mnt_count=cpu_to_le16 (le16_to_cpu (es->s_mnt_count) +1 ); +le16_add_cpu (&es->s_mnt_count,1 ); es->s_mtime =cpu_to_le32 (get_seconds ());ext3_update_dynamic_rev (sb);EXT3_SET_INCOMPAT_FEATURE (sb,EXT3_FEATURE_INCOMPAT_RECOVER );
fs/ext3/xattr.c +2 -4
@@ -492,8 +492,7 @@ ext3_xattr_release_block(handle_t *handle, struct inode *inode,get_bh (bh);ext3_forget (handle,1 , inode, bh, bh->b_blocknr); }else { -BHDR (bh)->h_refcount =cpu_to_le32 ( -le32_to_cpu (BHDR (bh)->h_refcount) -1 ); +le32_add_cpu (&BHDR (bh)->h_refcount,-1 ); error =ext3_journal_dirty_metadata (handle, bh);if (IS_SYNC (inode)) handle->h_sync =1 ;
@@ -780,8 +779,7 @@ inserted:if (error) goto cleanup_dquot;lock_buffer (new_bh); -BHDR (new_bh)->h_refcount =cpu_to_le32 (1 + -le32_to_cpu (BHDR (new_bh)->h_refcount)); +le32_add_cpu (&BHDR (new_bh)->h_refcount,1 );ea_bdebug (new_bh,"reusing; refcount now=%d" ,le32_to_cpu (BHDR (new_bh)->h_refcount));unlock_buffer (new_bh);
fs/ocfs2/cluster/endian.h deleted +0 -30
@@ -1,30 +0,0 @@ -/* -*- mode: c; c-basic-offset: 8; -*- -* vim: noexpandtab sw=8 ts=8 sts=0: -* -* Copyright (C) 2005 Oracle. All rights reserved. -* -* This program is free software; you can redistribute it and/or -* modify it under the terms of the GNU General Public -* License as published by the Free Software Foundation; either -* version 2 of the License, or (at your option) any later version. -* -* This program is distributed in the hope that it will be useful, -* but WITHOUT ANY WARRANTY; without even the implied warranty of -* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -* General Public License for more details. -* -* You should have received a copy of the GNU General Public -* License along with this program; if not, write to the -* Free Software Foundation, Inc., 59 Temple Place - Suite 330, -* Boston, MA 021110-1307, USA. -*/ - -#ifndef OCFS2_CLUSTER_ENDIAN_H -#define OCFS2_CLUSTER_ENDIAN_H - -static inline void be32_add_cpu (__be32 *var,u32 val) -{ - *var =cpu_to_be32 (be32_to_cpu (*var) + val); -} - -#endif /* OCFS2_CLUSTER_ENDIAN_H */
fs/ocfs2/cluster/nodemanager.c +0 -1
@@ -24,7 +24,6 @@#include <linux/sysctl.h> #include <linux/configfs.h> -#include "endian.h" #include "tcp.h" #include "nodemanager.h" #include "heartbeat.h"
fs/ocfs2/dlm/dlmast.c +0 -1
@@ -43,7 +43,6 @@#include "cluster/heartbeat.h" #include "cluster/nodemanager.h" #include "cluster/tcp.h" -#include "cluster/endian.h" #include "dlmapi.h" #include "dlmcommon.h"
fs/ocfs2/endian.h deleted +0 -45
45 lines not on this page · raw message
fs/ocfs2/ocfs2.h +0 -1
@@ -43,7 +43,6 @@#include "dlm/dlmapi.h" #include "ocfs2_fs.h" -#include "endian.h" #include "ocfs2_lockid.h" /* Most user visible OCFS2 inodes will have very few pieces of
include/linux/byteorder/generic.h +30 -0
@@ -146,6 +146,36 @@#define htons (x) ___htons(x)#define ntohs (x) ___ntohs(x) +static inline void le16_add_cpu (__le16 *var,u16 val) +{ + *var =cpu_to_le16 (le16_to_cpu (*var) + val); +} + +static inline void le32_add_cpu (__le32 *var,u32 val) +{ + *var =cpu_to_le32 (le32_to_cpu (*var) + val); +} + +static inline void le64_add_cpu (__le64 *var,u64 val) +{ + *var =cpu_to_le64 (le64_to_cpu (*var) + val); +} + +static inline void be16_add_cpu (__be16 *var,u16 val) +{ + *var =cpu_to_be16 (be16_to_cpu (*var) + val); +} + +static inline void be32_add_cpu (__be32 *var,u32 val) +{ + *var =cpu_to_be32 (be32_to_cpu (*var) + val); +} + +static inline void be64_add_cpu (__be64 *var,u64 val) +{ + *var =cpu_to_be64 (be64_to_cpu (*var) + val); +} +#endif /* KERNEL */ #endif /* _LINUX_BYTEORDER_GENERIC_H */
Signature · 1 line
1.5.3.7
Quoted 10 lines · Marcin Slusarz
There are many places where these functions would be useful.
(just look at: grep -r 'cpu_to_[ble12346]*([ble12346]*_to_cpu.*[-+]' linux-src/) What do you think?ps: this patch depends on http://lkml.org/lkml/2007/12/25/35
--add inline functions which add native byte order variable to little/big endian variable to core header and as an example convert ext3 to use them
Various places already have this as be*_add / le*_add, so it might be more useful to keep those names already in use.
Quoted 8 lines · Christoph Hellwig
Quoted 7 lines · Marcin Slusarz
There are many places where these functions would be useful.
(just look at: grep -r 'cpu_to_[ble12346]*([ble12346]*_to_cpu.*[-+]' linux-src/) What do you think?ps: this patch depends on http://lkml.org/lkml/2007/12/25/35
--
Quoted 6 lines
Quoted 3 lines
add inline functions which add native byte order variable to little/big endian variable to core header and as an example convert ext3 to use them
Various places already have this as be*_add / le*_add, so it might be more useful to keep those names already in use.
I found it in XFS only. Did I miss something?be32_add is shorter than be32_add_cpu but I think it's not clear whether second parameter is in native byte order or not.
Marcin
Quoted 1 line · Marcin Slusarz
I found it in XFS only. Did I miss something?
These helpers come from ocfs2 which has both be and le variants.
Quoted 2 lines
be32_addis shorter thanbe32_add_cpubut I think it's not clear whether second parameter is in native byte order or not.
Then again adding foreing endian values doesn't make much sense.
If you insist on your naming please make sure to convert ocfs and xfs to your naming.
Quoted 7 lines · Christoph Hellwig
Quoted 2 lines
be32_addis shorter thanbe32_add_cpubut I think it's not clear whether second parameter is in native byte order or not.Then again adding foreing endian values doesn't make much sense.
If you insist on your naming please make sure to convert ocfs and xfs to your naming.
Personally, I'd rather stick with the (IMHO) more descriptive name - btw, the ones he picked don't require any extra conversion in Ocfs2.
--Mark
--
Mark Fasheh
Principal Software Developer, Oracle
[email protected]
Quoted 10 lines · Marcin Slusarz
There are many places where these functions would be useful.
(just look at: grep -r 'cpu_to_[ble12346]*([ble12346]*_to_cpu.*[-+]' linux-src/) What do you think?ps: this patch depends on http://lkml.org/lkml/2007/12/25/35
--add inline functions which add native byte order variable to little/big endian variable to core header and as an example convert ext3 to use them
You might want to note where you got these functions from in your message.
Also, it would be easier to follow these changes if you had broken things up into two patches - one which moved stuff from ocfs2 into generic helpers and a 2nd one to convert ext3.
Looking into my crystal ball, I see a bunch of "convert to using byteorder math macros" patches coming down the pipe. Since we're talking about disk fields where a mistake could be costly, I suggest that any patch more than a couple of lines should be tested by the submitter with sparse. A statement that sparse didn't produce any new warnings (with the patch applied) could be included in the description.
--Mark
--
Mark Fasheh
Principal Software Developer, Oracle
[email protected]
Quoted 5 lines · Mark Fasheh
Quoted 4 lines · Marcin Slusarz
There are many places where these functions would be useful.
(just look at: grep -r 'cpu_to_[ble12346]*([ble12346]*_to_cpu.*[-+]' linux-src/) What do you think?
Quoted 8 lines
Quoted 6 lines
ps: this patch depends on http://lkml.org/lkml/2007/12/25/35
--add inline functions which add native byte order variable to little/big endian variable to core header and as an example convert ext3 to use them
You might want to note where you got these functions from in your message.
Yes, my mistake, sorry about that.
Quoted 3 lines
Also, it would be easier to follow these changes if you had broken things up into two patches - one which moved stuff from ocfs2 into generic helpers and a 2nd one to convert ext3.
Will do (tomorrow).
Quoted 6 lines
Looking into my crystal ball, I see a bunch of "convert to using byteorder math macros" patches coming down the pipe. Since we're talking about disk fields where a mistake could be costly, I suggest that any patch more than a couple of lines should be tested by the submitter with sparse. A statement that sparse didn't produce any new warnings (with the patch applied) could be included in the description.
Good idea. Thanks.
Marcin