Re: [PATCH] ceph: minor coding style tweaks

2021-02-04 Thread Jeff Layton
On Thu, 2021-02-04 at 15:54 +0800, Zhiyuan Dai wrote:
> Fixed some coding style issues, improve code reading.
> 1. Move the pointer location
> 2. Move brace position
> 3. Alignment to open parenthesis
> 
> This patch adds whitespace to clearly separate the parameters.
> 
> Signed-off-by: Zhiyuan Dai 
> ---
>  fs/ceph/addr.c   |  3 +--
>  fs/ceph/cache.c  | 14 +++---
>  fs/ceph/caps.c   |  2 +-
>  fs/ceph/dir.c|  6 +++---
>  fs/ceph/export.c |  2 +-
>  fs/ceph/inode.c  |  6 +++---
>  fs/ceph/locks.c  |  2 +-
>  fs/ceph/mds_client.c |  8 
>  fs/ceph/mdsmap.c |  4 ++--
>  fs/ceph/snap.c   |  6 +++---
>  fs/ceph/xattr.c  |  4 ++--
>  11 files changed, 28 insertions(+), 29 deletions(-)
> 


While I appreciate the effort, it has been my experience that coding-
style cleanup patches tend to be more trouble than they are worth. They
can make backporting hard by introducing trivial merge conflicts that
have to be resolved.

If you happen to be doing substantive work in an area, then by all
means, go ahead and clean up the coding style in that area, but this
sort of patch isn't terribly helpful. I'm going to plan to not merge
this unless you can articulate a very good reason to do so.

Thanks,
Jeff

> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 9505529..94ce73e 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -493,8 +493,7 @@ static int ceph_readpages(struct file *file, struct 
> address_space *mapping,
>   return rc;
>  }
>  
> 
> 
> 
> -struct ceph_writeback_ctl
> -{
> +struct ceph_writeback_ctl {
>   loff_t i_size;
>   u64 truncate_size;
>   u32 truncate_seq;
> diff --git a/fs/ceph/cache.c b/fs/ceph/cache.c
> index 2f5cb6b..795523e 100644
> --- a/fs/ceph/cache.c
> +++ b/fs/ceph/cache.c
> @@ -50,7 +50,7 @@ void ceph_fscache_unregister(void)
>   fscache_unregister_netfs(_cache_netfs);
>  }
>  
> 
> 
> 
> -int ceph_fscache_register_fs(struct ceph_fs_client* fsc, struct fs_context 
> *fc)
> +int ceph_fscache_register_fs(struct ceph_fs_client *fsc, struct fs_context 
> *fc)
>  {
>   const struct ceph_fsid *fsid = >client->fsid;
>   const char *fscache_uniq = fsc->mount_options->fscache_uniq;
> @@ -110,8 +110,8 @@ static enum fscache_checkaux ceph_fscache_inode_check_aux(
>   loff_t object_size)
>  {
>   struct ceph_aux_inode aux;
> - struct ceph_inode_info* ci = cookie_netfs_data;
> - struct inode* inode = >vfs_inode;
> + struct ceph_inode_info *ci = cookie_netfs_data;
> + struct inode *inode = >vfs_inode;
>  
> 
> 
> 
>   if (dlen != sizeof(aux) ||
>   i_size_read(inode) != object_size)
> @@ -164,9 +164,9 @@ void ceph_fscache_register_inode_cookie(struct inode 
> *inode)
>   inode_unlock(inode);
>  }
>  
> 
> 
> 
> -void ceph_fscache_unregister_inode_cookie(struct ceph_inode_info* ci)
> +void ceph_fscache_unregister_inode_cookie(struct ceph_inode_info *ci)
>  {
> - struct fscache_cookie* cookie;
> + struct fscache_cookie *cookie;
>  
> 
> 
> 
>   if ((cookie = ci->fscache) == NULL)
>   return;
> @@ -296,7 +296,7 @@ void ceph_readpage_to_fscache(struct inode *inode, struct 
> page *page)
>    fscache_uncache_page(ci->fscache, page);
>  }
>  
> 
> 
> 
> -void ceph_invalidate_fscache_page(struct inode* inode, struct page *page)
> +void ceph_invalidate_fscache_page(struct inode *inode, struct page *page)
>  {
>   struct ceph_inode_info *ci = ceph_inode(inode);
>  
> 
> 
> 
> @@ -307,7 +307,7 @@ void ceph_invalidate_fscache_page(struct inode* inode, 
> struct page *page)
>   fscache_uncache_page(ci->fscache, page);
>  }
>  
> 
> 
> 
> -void ceph_fscache_unregister_fs(struct ceph_fs_client* fsc)
> +void ceph_fscache_unregister_fs(struct ceph_fs_client *fsc)
>  {
>   if (fscache_cookie_valid(fsc->fscache)) {
>   struct ceph_fscache_entry *ent;
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 255a512..39fe5d8 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -2270,7 +2270,7 @@ static int caps_are_flushed(struct inode *inode, u64 
> flush_tid)
>  
> 
> 
> 
>   spin_lock(>i_ceph_lock);
>   if (!list_empty(>i_cap_flush_list)) {
> - struct ceph_cap_flush * cf =
> + struct ceph_cap_flush *cf =
>   list_first_entry(>i_cap_flush_list,
>    struct ceph_cap_flush, i_list);
>   if (cf->tid <= flush_tid)
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 858ee73..2b7edd00 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1269,7 +1269,7 @@ void __ceph_dentry_lease_touch(struct ceph_dentry_info 
> *di)
>   spin_unlock(>dentry_list_lock);
>  }
>  
> 
> 
> 
> -static void __dentry_dir_lease_touch(struct ceph_mds_client* mdsc,
> +static void __dentry_dir_lease_touch(struct ceph_mds_client *mdsc,
>    struct ceph_dentry_info *di)
>  {
>   di->flags &= ~(CEPH_DENTRY_LEASE_LIST | 

[PATCH] ceph: minor coding style tweaks

2021-02-03 Thread Zhiyuan Dai
Fixed some coding style issues, improve code reading.
1. Move the pointer location
2. Move brace position
3. Alignment to open parenthesis

This patch adds whitespace to clearly separate the parameters.

Signed-off-by: Zhiyuan Dai 
---
 fs/ceph/addr.c   |  3 +--
 fs/ceph/cache.c  | 14 +++---
 fs/ceph/caps.c   |  2 +-
 fs/ceph/dir.c|  6 +++---
 fs/ceph/export.c |  2 +-
 fs/ceph/inode.c  |  6 +++---
 fs/ceph/locks.c  |  2 +-
 fs/ceph/mds_client.c |  8 
 fs/ceph/mdsmap.c |  4 ++--
 fs/ceph/snap.c   |  6 +++---
 fs/ceph/xattr.c  |  4 ++--
 11 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 9505529..94ce73e 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -493,8 +493,7 @@ static int ceph_readpages(struct file *file, struct 
address_space *mapping,
return rc;
 }
 
-struct ceph_writeback_ctl
-{
+struct ceph_writeback_ctl {
loff_t i_size;
u64 truncate_size;
u32 truncate_seq;
diff --git a/fs/ceph/cache.c b/fs/ceph/cache.c
index 2f5cb6b..795523e 100644
--- a/fs/ceph/cache.c
+++ b/fs/ceph/cache.c
@@ -50,7 +50,7 @@ void ceph_fscache_unregister(void)
fscache_unregister_netfs(_cache_netfs);
 }
 
-int ceph_fscache_register_fs(struct ceph_fs_client* fsc, struct fs_context *fc)
+int ceph_fscache_register_fs(struct ceph_fs_client *fsc, struct fs_context *fc)
 {
const struct ceph_fsid *fsid = >client->fsid;
const char *fscache_uniq = fsc->mount_options->fscache_uniq;
@@ -110,8 +110,8 @@ static enum fscache_checkaux ceph_fscache_inode_check_aux(
loff_t object_size)
 {
struct ceph_aux_inode aux;
-   struct ceph_inode_info* ci = cookie_netfs_data;
-   struct inode* inode = >vfs_inode;
+   struct ceph_inode_info *ci = cookie_netfs_data;
+   struct inode *inode = >vfs_inode;
 
if (dlen != sizeof(aux) ||
i_size_read(inode) != object_size)
@@ -164,9 +164,9 @@ void ceph_fscache_register_inode_cookie(struct inode *inode)
inode_unlock(inode);
 }
 
-void ceph_fscache_unregister_inode_cookie(struct ceph_inode_info* ci)
+void ceph_fscache_unregister_inode_cookie(struct ceph_inode_info *ci)
 {
-   struct fscache_cookie* cookie;
+   struct fscache_cookie *cookie;
 
if ((cookie = ci->fscache) == NULL)
return;
@@ -296,7 +296,7 @@ void ceph_readpage_to_fscache(struct inode *inode, struct 
page *page)
 fscache_uncache_page(ci->fscache, page);
 }
 
-void ceph_invalidate_fscache_page(struct inode* inode, struct page *page)
+void ceph_invalidate_fscache_page(struct inode *inode, struct page *page)
 {
struct ceph_inode_info *ci = ceph_inode(inode);
 
@@ -307,7 +307,7 @@ void ceph_invalidate_fscache_page(struct inode* inode, 
struct page *page)
fscache_uncache_page(ci->fscache, page);
 }
 
-void ceph_fscache_unregister_fs(struct ceph_fs_client* fsc)
+void ceph_fscache_unregister_fs(struct ceph_fs_client *fsc)
 {
if (fscache_cookie_valid(fsc->fscache)) {
struct ceph_fscache_entry *ent;
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 255a512..39fe5d8 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -2270,7 +2270,7 @@ static int caps_are_flushed(struct inode *inode, u64 
flush_tid)
 
spin_lock(>i_ceph_lock);
if (!list_empty(>i_cap_flush_list)) {
-   struct ceph_cap_flush * cf =
+   struct ceph_cap_flush *cf =
list_first_entry(>i_cap_flush_list,
 struct ceph_cap_flush, i_list);
if (cf->tid <= flush_tid)
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 858ee73..2b7edd00 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1269,7 +1269,7 @@ void __ceph_dentry_lease_touch(struct ceph_dentry_info 
*di)
spin_unlock(>dentry_list_lock);
 }
 
-static void __dentry_dir_lease_touch(struct ceph_mds_client* mdsc,
+static void __dentry_dir_lease_touch(struct ceph_mds_client *mdsc,
 struct ceph_dentry_info *di)
 {
di->flags &= ~(CEPH_DENTRY_LEASE_LIST | CEPH_DENTRY_REFERENCED);
@@ -1351,7 +1351,7 @@ struct ceph_lease_walk_control {
 {
struct ceph_dentry_info *di, *tmp;
struct dentry *dentry, *last = NULL;
-   struct list_head* list;
+   struct list_head *list;
 LIST_HEAD(dispose);
unsigned long freed = 0;
int ret = 0;
@@ -1498,7 +1498,7 @@ int ceph_trim_dentries(struct ceph_mds_client *mdsc)
lwc.dir_lease = true;
lwc.expire_dir_lease = freed < count;
lwc.dir_lease_ttl = mdsc->fsc->mount_options->caps_wanted_delay_max * 
HZ;
-   freed +=__dentry_leases_walk(mdsc, , __dir_lease_check);
+   freed += __dentry_leases_walk(mdsc, , __dir_lease_check);
if (!lwc.nr_to_scan) /* more to check */
return -EAGAIN;
 
diff --git a/fs/ceph/export.c b/fs/ceph/export.c
index e088843..83ec43f 

Re: [PATCH] ceph: minor coding style tweaks

2021-02-03 Thread Joe Perches
On Thu, 2021-02-04 at 14:32 +0800, Zhiyuan Dai wrote:
> Fixed some coding style issues, improve code reading.

Might describe the patch does 3 things:

o Move the pointer location
struct foo* bar; -> struct foo *bar;
o Move brace position
  from
struct foo
{
  to
struct foo {
o Alignment to open parenthesis

And one more comment:

> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
[]
> @@ -309,8 +309,8 @@ static int ceph_fill_dirfrag(struct inode *inode,
>  
>  static int frag_tree_split_cmp(const void *l, const void *r)
>  {
> - struct ceph_frag_tree_split *ls = (struct ceph_frag_tree_split*)l;
> - struct ceph_frag_tree_split *rs = (struct ceph_frag_tree_split*)r;
> + struct ceph_frag_tree_split *ls = (struct ceph_frag_tree_split *)l;
> + struct ceph_frag_tree_split *rs = (struct ceph_frag_tree_split *)r;

It's unnecessary to cast void pointers and it's bad form to lose
the const qualifier.

const struct ceph_frag_tree_split *ls = l;
const struct ceph_frag_tree_split *rs = r;

>   return ceph_frag_compare(le32_to_cpu(ls->frag),
>    le32_to_cpu(rs->frag));
>  }




[PATCH] ceph: minor coding style tweaks

2021-02-03 Thread Zhiyuan Dai
Fixed some coding style issues, improve code reading.

Signed-off-by: Zhiyuan Dai 
---
 fs/ceph/addr.c   |  3 +--
 fs/ceph/cache.c  | 14 +++---
 fs/ceph/caps.c   |  2 +-
 fs/ceph/dir.c|  6 +++---
 fs/ceph/export.c |  2 +-
 fs/ceph/inode.c  |  6 +++---
 fs/ceph/locks.c  |  2 +-
 fs/ceph/mds_client.c |  8 
 fs/ceph/mdsmap.c |  4 ++--
 fs/ceph/snap.c   |  6 +++---
 fs/ceph/xattr.c  |  4 ++--
 11 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 9505529..94ce73e 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -493,8 +493,7 @@ static int ceph_readpages(struct file *file, struct 
address_space *mapping,
return rc;
 }
 
-struct ceph_writeback_ctl
-{
+struct ceph_writeback_ctl {
loff_t i_size;
u64 truncate_size;
u32 truncate_seq;
diff --git a/fs/ceph/cache.c b/fs/ceph/cache.c
index 2f5cb6b..795523e 100644
--- a/fs/ceph/cache.c
+++ b/fs/ceph/cache.c
@@ -50,7 +50,7 @@ void ceph_fscache_unregister(void)
fscache_unregister_netfs(_cache_netfs);
 }
 
-int ceph_fscache_register_fs(struct ceph_fs_client* fsc, struct fs_context *fc)
+int ceph_fscache_register_fs(struct ceph_fs_client *fsc, struct fs_context *fc)
 {
const struct ceph_fsid *fsid = >client->fsid;
const char *fscache_uniq = fsc->mount_options->fscache_uniq;
@@ -110,8 +110,8 @@ static enum fscache_checkaux ceph_fscache_inode_check_aux(
loff_t object_size)
 {
struct ceph_aux_inode aux;
-   struct ceph_inode_info* ci = cookie_netfs_data;
-   struct inode* inode = >vfs_inode;
+   struct ceph_inode_info *ci = cookie_netfs_data;
+   struct inode *inode = >vfs_inode;
 
if (dlen != sizeof(aux) ||
i_size_read(inode) != object_size)
@@ -164,9 +164,9 @@ void ceph_fscache_register_inode_cookie(struct inode *inode)
inode_unlock(inode);
 }
 
-void ceph_fscache_unregister_inode_cookie(struct ceph_inode_info* ci)
+void ceph_fscache_unregister_inode_cookie(struct ceph_inode_info *ci)
 {
-   struct fscache_cookie* cookie;
+   struct fscache_cookie *cookie;
 
if ((cookie = ci->fscache) == NULL)
return;
@@ -296,7 +296,7 @@ void ceph_readpage_to_fscache(struct inode *inode, struct 
page *page)
 fscache_uncache_page(ci->fscache, page);
 }
 
-void ceph_invalidate_fscache_page(struct inode* inode, struct page *page)
+void ceph_invalidate_fscache_page(struct inode *inode, struct page *page)
 {
struct ceph_inode_info *ci = ceph_inode(inode);
 
@@ -307,7 +307,7 @@ void ceph_invalidate_fscache_page(struct inode* inode, 
struct page *page)
fscache_uncache_page(ci->fscache, page);
 }
 
-void ceph_fscache_unregister_fs(struct ceph_fs_client* fsc)
+void ceph_fscache_unregister_fs(struct ceph_fs_client *fsc)
 {
if (fscache_cookie_valid(fsc->fscache)) {
struct ceph_fscache_entry *ent;
diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
index 255a512..39fe5d8 100644
--- a/fs/ceph/caps.c
+++ b/fs/ceph/caps.c
@@ -2270,7 +2270,7 @@ static int caps_are_flushed(struct inode *inode, u64 
flush_tid)
 
spin_lock(>i_ceph_lock);
if (!list_empty(>i_cap_flush_list)) {
-   struct ceph_cap_flush * cf =
+   struct ceph_cap_flush *cf =
list_first_entry(>i_cap_flush_list,
 struct ceph_cap_flush, i_list);
if (cf->tid <= flush_tid)
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 858ee73..2b7edd00 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1269,7 +1269,7 @@ void __ceph_dentry_lease_touch(struct ceph_dentry_info 
*di)
spin_unlock(>dentry_list_lock);
 }
 
-static void __dentry_dir_lease_touch(struct ceph_mds_client* mdsc,
+static void __dentry_dir_lease_touch(struct ceph_mds_client *mdsc,
 struct ceph_dentry_info *di)
 {
di->flags &= ~(CEPH_DENTRY_LEASE_LIST | CEPH_DENTRY_REFERENCED);
@@ -1351,7 +1351,7 @@ struct ceph_lease_walk_control {
 {
struct ceph_dentry_info *di, *tmp;
struct dentry *dentry, *last = NULL;
-   struct list_head* list;
+   struct list_head *list;
 LIST_HEAD(dispose);
unsigned long freed = 0;
int ret = 0;
@@ -1498,7 +1498,7 @@ int ceph_trim_dentries(struct ceph_mds_client *mdsc)
lwc.dir_lease = true;
lwc.expire_dir_lease = freed < count;
lwc.dir_lease_ttl = mdsc->fsc->mount_options->caps_wanted_delay_max * 
HZ;
-   freed +=__dentry_leases_walk(mdsc, , __dir_lease_check);
+   freed += __dentry_leases_walk(mdsc, , __dir_lease_check);
if (!lwc.nr_to_scan) /* more to check */
return -EAGAIN;
 
diff --git a/fs/ceph/export.c b/fs/ceph/export.c
index e088843..83ec43f 100644
--- a/fs/ceph/export.c
+++ b/fs/ceph/export.c
@@ -343,7 +343,7 @@ static struct dentry *ceph_get_parent(struct dentry *child)
struct