Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1281242 > unrolled thread

[lustre cleanups 0/6] Patch series to make lustre safe(r) for W=1 compiles

Started byValdis Kletnieks <Valdis.Kletnieks@vt.edu>
First post2015-12-01 23:10 +0100
Last post2015-12-04 07:30 +0100
Articles 7 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [lustre cleanups 0/6] Patch series to make lustre safe(r) for W=1 compiles Valdis Kletnieks <Valdis.Kletnieks@vt.edu> - 2015-12-01 23:10 +0100
    [lustre cleanups 4/6] Fix another C compiler whine: Valdis Kletnieks <Valdis.Kletnieks@vt.edu> - 2015-12-01 23:10 +0100
    [lustre cleanups 1/6] Swap inline/const to kill 272 warnings Valdis Kletnieks <Valdis.Kletnieks@vt.edu> - 2015-12-01 23:10 +0100
    [lustre cleanups 3/6] Clean up another C warnining: Valdis Kletnieks <Valdis.Kletnieks@vt.edu> - 2015-12-01 23:10 +0100
    [lustre cleanups 6/6] Nuke another unsigned >= 0 assert Valdis Kletnieks <Valdis.Kletnieks@vt.edu> - 2015-12-01 23:10 +0100
    [lustre cleanups 5/6] Nuke an unsigned >= 0 assert Valdis Kletnieks <Valdis.Kletnieks@vt.edu> - 2015-12-01 23:10 +0100
    Re: [lustre cleanups 0/6] Patch series to make lustre safe(r) for  W=1 compiles "Dilger, Andreas" <andreas.dilger@intel.com> - 2015-12-04 07:30 +0100

#1281242 — [lustre cleanups 0/6] Patch series to make lustre safe(r) for W=1 compiles

FromValdis Kletnieks <Valdis.Kletnieks@vt.edu>
Date2015-12-01 23:10 +0100
Subject[lustre cleanups 0/6] Patch series to make lustre safe(r) for W=1 compiles
Message-ID<qB1hw-73C-9@gated-at.bofh.it>
Start of a batch series to clean up the Lustre tree. Other people have
done  some sparse and checkpatch cleanups, but I found a bunch of
stuff building with W=1.

Valdis Kletnieks (6):

  Swap inline/const to kill 272 warnings
  Fix set-but-unused whinge.
  Clean up another C warnining:
  Fix another C compiler whine:
  Nuke an unsigned >= 0 assert
  Nuke another unsigned >= 0 assert

 drivers/staging/lustre/lustre/fid/lproc_fid.c      |  1 +
 drivers/staging/lustre/lustre/include/lu_object.h  |  2 +-
 drivers/staging/lustre/lustre/include/lustre_cfg.h |  4 --
 drivers/staging/lustre/lustre/libcfs/module.c      | 15 ++++----
 drivers/staging/lustre/lustre/llite/rw.c           |  1 -
 drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c    |  1 -
 7 files changed, 26 insertions(+), 41 deletions(-)

-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1281243 — [lustre cleanups 4/6] Fix another C compiler whine:

FromValdis Kletnieks <Valdis.Kletnieks@vt.edu>
Date2015-12-01 23:10 +0100
Subject[lustre cleanups 4/6] Fix another C compiler whine:
Message-ID<qB1hx-73C-15@gated-at.bofh.it>
In reply to#1281242
  CC [M]  drivers/staging/lustre/lustre/libcfs/module.o
drivers/staging/lustre/lustre/libcfs/module.c: In function 'lustre_insert_debugfs':
drivers/staging/lustre/lustre/libcfs/module.c:670:17: warning: variable 'entry' set but not used [-Wunused-but-set-variable]
  struct dentry *entry;
                     ^
Just ignore the dentry returned, and add a comment that we *know*
we're not really leaking the dentry because something else will be able
to reap it via recursion.

Signed-off-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
---
:100644 100644 96d9d4651a51... 4438dc426b54... M	drivers/staging/lustre/lustre/libcfs/module.c
 drivers/staging/lustre/lustre/libcfs/module.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/lustre/lustre/libcfs/module.c b/drivers/staging/lustre/lustre/libcfs/module.c
index 96d9d4651a51..4438dc426b54 100644
--- a/drivers/staging/lustre/lustre/libcfs/module.c
+++ b/drivers/staging/lustre/lustre/libcfs/module.c
@@ -667,8 +667,6 @@ static const struct file_operations lnet_debugfs_file_operations = {
 void lustre_insert_debugfs(struct ctl_table *table,
 			   const struct lnet_debugfs_symlink_def *symlinks)
 {
-	struct dentry *entry;
-
 	if (lnet_debugfs_root == NULL)
 		lnet_debugfs_root = debugfs_create_dir("lnet", NULL);
 
@@ -676,15 +676,18 @@ void lustre_insert_debugfs(struct ctl_table *table,
 	if (IS_ERR_OR_NULL(lnet_debugfs_root))
 		return;
 
+	/* We don't save the dentry returned in next two calls, because
+	 * we don't call debugfs_remove() but rather remove_recursive()
+	 */
 	for (; table->procname; table++)
-		entry = debugfs_create_file(table->procname, table->mode,
-					    lnet_debugfs_root, table,
-					    &lnet_debugfs_file_operations);
+		 debugfs_create_file(table->procname, table->mode,
+				     lnet_debugfs_root, table,
+				     &lnet_debugfs_file_operations);
 
 	for (; symlinks && symlinks->name; symlinks++)
-		entry = debugfs_create_symlink(symlinks->name,
-					       lnet_debugfs_root,
-					       symlinks->target);
+		debugfs_create_symlink(symlinks->name,
+				       lnet_debugfs_root,
+				       symlinks->target);
 
 }
 EXPORT_SYMBOL_GPL(lustre_insert_debugfs);
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281245 — [lustre cleanups 1/6] Swap inline/const to kill 272 warnings

FromValdis Kletnieks <Valdis.Kletnieks@vt.edu>
Date2015-12-01 23:10 +0100
Subject[lustre cleanups 1/6] Swap inline/const to kill 272 warnings
Message-ID<qB1hx-73C-19@gated-at.bofh.it>
In reply to#1281242
Start of a batch series to clean up the Lustre tree. Other people have
done some sparse and checkpatch cleanups, but I found a bunch of stuff
building with W=1.

Low-hanging fruit first:

  CC [M]  drivers/staging/lustre/lustre/fid/fid_request.o
In file included from drivers/staging/lustre/lustre/fid/../include/lustre_net.h:66:0,
                 from drivers/staging/lustre/lustre/fid/../include/lustre_lib.h:64,
                 from drivers/staging/lustre/lustre/fid/../include/obd.h:52,
                 from drivers/staging/lustre/lustre/fid/fid_request.c:48:
drivers/staging/lustre/lustre/fid/../include/lu_object.h:765:1: warning: 'inline' is not at beginning of declaration [-Wold-style-
declaration]
 static const inline struct lu_device_operations *
 ^

So we just swap inline and const.  272 warnings gone. :)

Signed-off-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
---
diff --git a/drivers/staging/lustre/lustre/include/lu_object.h b/drivers/staging/lustre/lustre/include/lu_object.h
index fa78689748a9..176724f60c1b 100644
--- a/drivers/staging/lustre/lustre/include/lu_object.h
+++ b/drivers/staging/lustre/lustre/include/lu_object.h
@@ -756,7 +756,7 @@ static inline const struct lu_fid *lu_object_fid(const struct lu_object *o)
 /**
  * return device operations vector for this object
  */
-static const inline struct lu_device_operations *
+static inline const struct lu_device_operations *
 lu_object_ops(const struct lu_object *o)
 {
 	return o->lo_dev->ld_ops;
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281246 — [lustre cleanups 3/6] Clean up another C warnining:

FromValdis Kletnieks <Valdis.Kletnieks@vt.edu>
Date2015-12-01 23:10 +0100
Subject[lustre cleanups 3/6] Clean up another C warnining:
Message-ID<qB1hx-73C-21@gated-at.bofh.it>
In reply to#1281242
drivers/staging/lustre/lustre/fid/../include/lustre_cfg.h: In function 'lustre_cfg_free':
drivers/staging/lustre/lustre/fid/../include/lustre_cfg.h:253:6: warning: variable 'len' set but not used [-Wunused-but-set-variable]
  int len;

Yep, we're just gonna call kfree, no need to calculate len. Bye-bye.

Signed-off-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
---
diff --git a/drivers/staging/lustre/lustre/include/lustre_cfg.h b/drivers/staging/lustre/lustre/include/lustre_cfg.h
index eb6b292b7b25..d30d8b054c92 100644
--- a/drivers/staging/lustre/lustre/include/lustre_cfg.h
+++ b/drivers/staging/lustre/lustre/include/lustre_cfg.h
@@ -252,10 +252,6 @@ static inline struct lustre_cfg *lustre_cfg_new(int cmd,
 
 static inline void lustre_cfg_free(struct lustre_cfg *lcfg)
 {
-	int len;
-
-	len = lustre_cfg_len(lcfg->lcfg_bufcount, lcfg->lcfg_buflens);
-
 	kfree(lcfg);
 	return;
 }
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281247 — [lustre cleanups 6/6] Nuke another unsigned >= 0 assert

FromValdis Kletnieks <Valdis.Kletnieks@vt.edu>
Date2015-12-01 23:10 +0100
Subject[lustre cleanups 6/6] Nuke another unsigned >= 0 assert
Message-ID<qB1hy-73C-27@gated-at.bofh.it>
In reply to#1281242
Clean up another case of the compiler remininding the programmer they
are an idiot:

drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c:308:34: warning: comparison of u
nsigned expression >= 0 is always true [-Wtype-limits]
  LASSERT(page_pools.epp_waitqlen >= 0);

Just lose the assert, and save a page of compiler spew.

Signed-off-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
---
diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
index cd8a9987f7ac..1f326673f089 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c
@@ -304,7 +304,6 @@ static unsigned long enc_pools_cleanup(struct page ***pools, int npools)
 static inline void enc_pools_wakeup(void)
 {
 	assert_spin_locked(&page_pools.epp_lock);
-	LASSERT(page_pools.epp_waitqlen >= 0);
 
 	if (unlikely(page_pools.epp_waitqlen)) {
 		LASSERT(waitqueue_active(&page_pools.epp_waitq));
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281249 — [lustre cleanups 5/6] Nuke an unsigned >= 0 assert

FromValdis Kletnieks <Valdis.Kletnieks@vt.edu>
Date2015-12-01 23:10 +0100
Subject[lustre cleanups 5/6] Nuke an unsigned >= 0 assert
Message-ID<qB1hy-73C-31@gated-at.bofh.it>
In reply to#1281242
Writing asserts for almost-never-can-happen things can be valuable.
Writing an assert that tests that an "unsigned int" hasn't gone negative
isn't.

And it generates an *ugly* message:

drivers/staging/lustre/lustre/llite/rw.c:763:20: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
  LASSERTF(reserved >= 0, "reserved %lu\n", reserved);
                    ^
include/linux/compiler.h:137:45: note: in definition of macro 'unlikely'
 #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                             ^
drivers/staging/lustre/lustre/llite/rw.c:763:2: note: in expansion of macro 'LASSERTF'
  LASSERTF(reserved >= 0, "reserved %lu\n", reserved);
  ^
drivers/staging/lustre/lustre/llite/rw.c:763:20: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
  LASSERTF(reserved >= 0, "reserved %lu\n", reserved);
                    ^
include/linux/compiler.h:137:53: note: in definition of macro 'unlikely'
 #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                     ^
drivers/staging/lustre/lustre/llite/rw.c:763:2: note: in expansion of macro 'LASSERTF'
  LASSERTF(reserved >= 0, "reserved %lu\n", reserved);
  ^
drivers/staging/lustre/lustre/llite/rw.c:763:20: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
  LASSERTF(reserved >= 0, "reserved %lu\n", reserved);
                    ^
include/linux/compiler.h:110:47: note: in definition of macro 'likely_notrace'
 #define likely_notrace(x) __builtin_expect(!!(x), 1)
                                               ^
include/linux/compiler.h:137:58: note: in expansion of macro '__branch_check__'
 #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                          ^
drivers/staging/lustre/lustre/llite/../include/linux/../../../include/linux/libcfs/libcfs_private.h:58:6: note: in expansion of macro 'unlikely'
  if (unlikely(!(cond))) {     \
      ^
drivers/staging/lustre/lustre/llite/rw.c:763:2: note: in expansion of macro 'LASSERTF'
  LASSERTF(reserved >= 0, "reserved %lu\n", reserved);
  ^

Umm, thank you, GCC.  We'll delete the problem line so ne never see that spew again.

Signed-off-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
---
diff --git a/drivers/staging/lustre/lustre/llite/rw.c b/drivers/staging/lustre/lustre/llite/rw.c
index f79193fa2fb7..39390aab9da2 100644
--- a/drivers/staging/lustre/lustre/llite/rw.c
+++ b/drivers/staging/lustre/lustre/llite/rw.c
@@ -764,7 +764,6 @@ int ll_readahead(const struct lu_env *env, struct cl_io *io,
 	ret = ll_read_ahead_pages(env, io, queue,
 				  ria, &reserved, mapping, &ra_end);
 
-	LASSERTF(reserved >= 0, "reserved %lu\n", reserved);
 	if (reserved != 0)
 		ll_ra_count_put(ll_i2sbi(inode), reserved);
 
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283595 — Re: [lustre cleanups 0/6] Patch series to make lustre safe(r) for W=1 compiles

From"Dilger, Andreas" <andreas.dilger@intel.com>
Date2015-12-04 07:30 +0100
SubjectRe: [lustre cleanups 0/6] Patch series to make lustre safe(r) for W=1 compiles
Message-ID<qBS2u-7wi-9@gated-at.bofh.it>
In reply to#1281242
On 2015/12/01, 15:05, "Valdis Kletnieks" <Valdis.Kletnieks@vt.edu> wrote:

>Start of a batch series to clean up the Lustre tree. Other people have
>done  some sparse and checkpatch cleanups, but I found a bunch of
>stuff building with W=1.

Hello Valdis,
thanks for these patches.  Strictly speaking, they should also be sent
to "Greg Kroah-Hartman <gregkh@linuxfoundation.org>" and also
devel@driverdev.osuosl.org because Lustre is still in the staging tree.

Also, for the patch subject line, it is standard to use something like:

    [PATCH 1/6] staging/lustre: Swap inline/const to kill 272 warnings

as generated by "git format-patch".

Could you please resubmit your patch series so that Greg sees your
patches.

Cheers, Andreas

>Valdis Kletnieks (6):
>
>  Swap inline/const to kill 272 warnings
>  Fix set-but-unused whinge.
>  Clean up another C warnining:
>  Fix another C compiler whine:
>  Nuke an unsigned >= 0 assert
>  Nuke another unsigned >= 0 assert
>
> drivers/staging/lustre/lustre/fid/lproc_fid.c      |  1 +
> drivers/staging/lustre/lustre/include/lu_object.h  |  2 +-
> drivers/staging/lustre/lustre/include/lustre_cfg.h |  4 --
> drivers/staging/lustre/lustre/libcfs/module.c      | 15 ++++----
> drivers/staging/lustre/lustre/llite/rw.c           |  1 -
> drivers/staging/lustre/lustre/ptlrpc/sec_bulk.c    |  1 -
> 7 files changed, 26 insertions(+), 41 deletions(-)
>
>-- 
>2.6.3
>
>


Cheers, Andreas
-- 
Andreas Dilger

Lustre Principal Architect
Intel High Performance Data Division


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web