2016-12-07 22:42:23

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 0/8] Sparse warning fixes in Lustre.

This set of fixes aims at sparse warnings.
Most of the patches are just moving declarations around
to deal with the
warning: symbol 'xxx' was not declared. Should it be static?
kind of messages.

Also a screwup with root_squash sysfs control is fixed.

Oleg Drokin (8):
staging/lustre/llite: move root_squash from sysfs to debugfs
staging/lustre/ldlm: Correct itree_overlap_cb return type
staging/lustre/llite: mark ll_io_init() static
staging/lustre/lov: make lov_lsm_alloc() static
staging/lustre/osc: extern declare osc_caches in a header
staging/lustre: Declare lu_context/session_tags_default
staging/lustre: Move lov_read_and_clear_async_rc declaration
staging/lustre/ptlrpc: Move nrs_conf_fifo extern to a header

drivers/staging/lustre/lustre/include/lu_object.h | 3 +++
drivers/staging/lustre/lustre/include/obd.h | 3 +++
drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 2 +-
drivers/staging/lustre/lustre/llite/file.c | 2 +-
drivers/staging/lustre/lustre/llite/lproc_llite.c | 27 ++++++++++++----------
drivers/staging/lustre/lustre/llite/vvp_internal.h | 2 --
drivers/staging/lustre/lustre/lov/lov_pack.c | 3 ++-
drivers/staging/lustre/lustre/obdclass/cl_object.c | 3 +--
drivers/staging/lustre/lustre/osc/osc_internal.h | 2 ++
drivers/staging/lustre/lustre/osc/osc_request.c | 2 --
drivers/staging/lustre/lustre/ptlrpc/nrs.c | 3 ---
.../staging/lustre/lustre/ptlrpc/ptlrpc_internal.h | 3 +++
12 files changed, 31 insertions(+), 24 deletions(-)

--
2.7.4


2016-12-07 22:42:28

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 6/8] staging/lustre: Declare lu_context/session_tags_default

Make the declaration in a header, not as an extern in a C file,
that is frowned upon.
This also makes sparse a little bit more happy.

Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/lustre/include/lu_object.h | 3 +++
drivers/staging/lustre/lustre/obdclass/cl_object.c | 3 +--
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lu_object.h b/drivers/staging/lustre/lustre/include/lu_object.h
index 260643e..69b2812 100644
--- a/drivers/staging/lustre/lustre/include/lu_object.h
+++ b/drivers/staging/lustre/lustre/include/lu_object.h
@@ -1326,5 +1326,8 @@ void lu_buf_realloc(struct lu_buf *buf, size_t size);
int lu_buf_check_and_grow(struct lu_buf *buf, size_t len);
struct lu_buf *lu_buf_check_and_alloc(struct lu_buf *buf, size_t len);

+extern __u32 lu_context_tags_default;
+extern __u32 lu_session_tags_default;
+
/** @} lu */
#endif /* __LUSTRE_LU_OBJECT_H */
diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
index f5d4e23..703cb67 100644
--- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
+++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
@@ -54,6 +54,7 @@
#include <linux/list.h>
#include "../../include/linux/libcfs/libcfs_hash.h" /* for cfs_hash stuff */
#include "../include/cl_object.h"
+#include "../include/lu_object.h"
#include "cl_internal.h"

static struct kmem_cache *cl_env_kmem;
@@ -61,8 +62,6 @@ static struct kmem_cache *cl_env_kmem;
/** Lock class of cl_object_header::coh_attr_guard */
static struct lock_class_key cl_attr_guard_class;

-extern __u32 lu_context_tags_default;
-extern __u32 lu_session_tags_default;
/**
* Initialize cl_object_header.
*/
--
2.7.4

2016-12-07 22:42:27

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 8/8] staging/lustre/ptlrpc: Move nrs_conf_fifo extern to a header

This avoids having an extern definition in a C file which is bad,
and also silences sparse complaint as well.

Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/lustre/ptlrpc/nrs.c | 3 ---
drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h | 3 +++
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/lustre/ptlrpc/nrs.c b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
index 7b6ffb1..ef19dbe 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/nrs.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
@@ -1559,9 +1559,6 @@ int ptlrpc_nrs_policy_control(const struct ptlrpc_service *svc,
return rc;
}

-/* ptlrpc/nrs_fifo.c */
-extern struct ptlrpc_nrs_pol_conf nrs_conf_fifo;
-
/**
* Adds all policies that ship with the ptlrpc module, to NRS core's list of
* policies \e nrs_core.nrs_policies.
diff --git a/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h b/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
index e0f859c..8e6a805 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
+++ b/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
@@ -226,6 +226,9 @@ struct ptlrpc_nrs_policy *nrs_request_policy(struct ptlrpc_nrs_request *nrq)
sizeof(NRS_LPROCFS_QUANTUM_NAME_REG __stringify(LPROCFS_NRS_QUANTUM_MAX) " " \
NRS_LPROCFS_QUANTUM_NAME_HP __stringify(LPROCFS_NRS_QUANTUM_MAX))

+/* ptlrpc/nrs_fifo.c */
+extern struct ptlrpc_nrs_pol_conf nrs_conf_fifo;
+
/* recovd_thread.c */

int ptlrpc_expire_one_request(struct ptlrpc_request *req, int async_unlink);
--
2.7.4

2016-12-07 22:42:26

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 7/8] staging/lustre: Move lov_read_and_clear_async_rc declaration

Move it to obd.h, so that it's included from both the users and
the actual definition, making sure they never get out of sync.
This also silences a sparse warning.

Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/lustre/include/obd.h | 3 +++
drivers/staging/lustre/lustre/llite/vvp_internal.h | 2 --
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
index 0f48e9c..1839f4f 100644
--- a/drivers/staging/lustre/lustre/include/obd.h
+++ b/drivers/staging/lustre/lustre/include/obd.h
@@ -43,6 +43,7 @@
#include "lustre_fld.h"
#include "lustre_handles.h"
#include "lustre_intent.h"
+#include "cl_object.h"

#define MAX_OBD_DEVICES 8192

@@ -76,6 +77,8 @@ static inline void loi_init(struct lov_oinfo *loi)
struct lov_stripe_md;
struct obd_info;

+int lov_read_and_clear_async_rc(struct cl_object *clob);
+
typedef int (*obd_enqueue_update_f)(void *cookie, int rc);

/* obd info for a particular level (lov, osc). */
diff --git a/drivers/staging/lustre/lustre/llite/vvp_internal.h b/drivers/staging/lustre/lustre/llite/vvp_internal.h
index c60d041..f40fd7f 100644
--- a/drivers/staging/lustre/lustre/llite/vvp_internal.h
+++ b/drivers/staging/lustre/lustre/llite/vvp_internal.h
@@ -301,8 +301,6 @@ static inline struct vvp_lock *cl2vvp_lock(const struct cl_lock_slice *slice)
# define CLOBINVRNT(env, clob, expr) \
((void)sizeof(env), (void)sizeof(clob), (void)sizeof(!!(expr)))

-int lov_read_and_clear_async_rc(struct cl_object *clob);
-
int vvp_io_init(const struct lu_env *env, struct cl_object *obj,
struct cl_io *io);
int vvp_io_write_commit(const struct lu_env *env, struct cl_io *io);
--
2.7.4

2016-12-07 22:42:22

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 2/8] staging/lustre/ldlm: Correct itree_overlap_cb return type

As per interval_search() prototype, the callback should return
enum, not int.
This fixes correspondign sparse warning.

Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
index a4a291a..f4cbc89 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
@@ -1148,7 +1148,7 @@ static int lock_matches(struct ldlm_lock *lock, struct lock_match_data *data)
return INTERVAL_ITER_STOP;
}

-static unsigned int itree_overlap_cb(struct interval_node *in, void *args)
+static enum interval_iter itree_overlap_cb(struct interval_node *in, void *args)
{
struct ldlm_interval *node = to_ldlm_interval(in);
struct lock_match_data *data = args;
--
2.7.4

2016-12-07 22:43:10

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 3/8] staging/lustre/llite: mark ll_io_init() static

It's not used anywhere out of this file.
Highlighted by sparse.

Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/lustre/llite/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index f634c11..d93f06a 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -1016,7 +1016,7 @@ static bool file_is_noatime(const struct file *file)
return false;
}

-void ll_io_init(struct cl_io *io, const struct file *file, int write)
+static void ll_io_init(struct cl_io *io, const struct file *file, int write)
{
struct inode *inode = file_inode(file);

--
2.7.4

2016-12-07 22:42:21

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 5/8] staging/lustre/osc: extern declare osc_caches in a header

This avoids frowned upon extern in the C file, and also
shuts down a sparse warning of
drivers/staging/lustre/lustre/osc/osc_dev.c:55:22: warning: symbol 'osc_caches' was not declared. Should it be static?

Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/lustre/osc/osc_internal.h | 2 ++
drivers/staging/lustre/lustre/osc/osc_request.c | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/osc/osc_internal.h b/drivers/staging/lustre/lustre/osc/osc_internal.h
index 688783d..5cce82b 100644
--- a/drivers/staging/lustre/lustre/osc/osc_internal.h
+++ b/drivers/staging/lustre/lustre/osc/osc_internal.h
@@ -181,6 +181,8 @@ static inline struct osc_device *obd2osc_dev(const struct obd_device *d)
return container_of0(d->obd_lu_dev, struct osc_device, od_cl.cd_lu_dev);
}

+extern struct lu_kmem_descr osc_caches[];
+
extern struct kmem_cache *osc_quota_kmem;
struct osc_quota_info {
/** linkage for quota hash table */
diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
index 7143564..f691297 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -2766,8 +2766,6 @@ static struct obd_ops osc_obd_ops = {
.quotactl = osc_quotactl,
};

-extern struct lu_kmem_descr osc_caches[];
-
static int __init osc_init(void)
{
struct lprocfs_static_vars lvars = { NULL };
--
2.7.4

2016-12-07 22:42:20

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 1/8] staging/lustre/llite: move root_squash from sysfs to debugfs

root_squash control got accidentally moved to sysfs instead of
debugfs, and the write side of it was also broken expecting a
userspace buffer.
It contains both uid and gid values in a single file, so debugfs
is a clear place for it.

Reported-by: Al Viro <[email protected]>
Fixes: c948390f10ccc "fix inconsistencies of root squash feature"
Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/lustre/llite/lproc_llite.c | 27 +++++++++++++----------
1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/lproc_llite.c b/drivers/staging/lustre/lustre/llite/lproc_llite.c
index 03682c1..f3ee584 100644
--- a/drivers/staging/lustre/lustre/llite/lproc_llite.c
+++ b/drivers/staging/lustre/lustre/llite/lproc_llite.c
@@ -924,27 +924,29 @@ static ssize_t ll_unstable_stats_seq_write(struct file *file,
}
LPROC_SEQ_FOPS(ll_unstable_stats);

-static ssize_t root_squash_show(struct kobject *kobj, struct attribute *attr,
- char *buf)
+static int ll_root_squash_seq_show(struct seq_file *m, void *v)
{
- struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info,
- ll_kobj);
+ struct super_block *sb = m->private;
+ struct ll_sb_info *sbi = ll_s2sbi(sb);
struct root_squash_info *squash = &sbi->ll_squash;

- return sprintf(buf, "%u:%u\n", squash->rsi_uid, squash->rsi_gid);
+ seq_printf(m, "%u:%u\n", squash->rsi_uid, squash->rsi_gid);
+ return 0;
}

-static ssize_t root_squash_store(struct kobject *kobj, struct attribute *attr,
- const char *buffer, size_t count)
+static ssize_t ll_root_squash_seq_write(struct file *file,
+ const char __user *buffer,
+ size_t count, loff_t *off)
{
- struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info,
- ll_kobj);
+ struct seq_file *m = file->private_data;
+ struct super_block *sb = m->private;
+ struct ll_sb_info *sbi = ll_s2sbi(sb);
struct root_squash_info *squash = &sbi->ll_squash;

return lprocfs_wr_root_squash(buffer, count, squash,
- ll_get_fsname(sbi->ll_sb, NULL, 0));
+ ll_get_fsname(sb, NULL, 0));
}
-LUSTRE_RW_ATTR(root_squash);
+LPROC_SEQ_FOPS(ll_root_squash);

static int ll_nosquash_nids_seq_show(struct seq_file *m, void *v)
{
@@ -997,6 +999,8 @@ static struct lprocfs_vars lprocfs_llite_obd_vars[] = {
{ "statahead_stats", &ll_statahead_stats_fops, NULL, 0 },
{ "unstable_stats", &ll_unstable_stats_fops, NULL },
{ "sbi_flags", &ll_sbi_flags_fops, NULL, 0 },
+ { .name = "root_squash",
+ .fops = &ll_root_squash_fops },
{ .name = "nosquash_nids",
.fops = &ll_nosquash_nids_fops },
{ NULL }
@@ -1027,7 +1031,6 @@ static struct attribute *llite_attrs[] = {
&lustre_attr_max_easize.attr,
&lustre_attr_default_easize.attr,
&lustre_attr_xattr_cache.attr,
- &lustre_attr_root_squash.attr,
NULL,
};

--
2.7.4

2016-12-07 22:43:54

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 4/8] staging/lustre/lov: make lov_lsm_alloc() static

It's not used anywhere outside of this file.
Highlighted by sparse.

Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/lustre/lov/lov_pack.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/lov/lov_pack.c b/drivers/staging/lustre/lustre/lov/lov_pack.c
index 6c93d18..68fa2de 100644
--- a/drivers/staging/lustre/lustre/lov/lov_pack.c
+++ b/drivers/staging/lustre/lustre/lov/lov_pack.c
@@ -198,7 +198,8 @@ static int lov_verify_lmm(void *lmm, int lmm_bytes, __u16 *stripe_count)
return rc;
}

-struct lov_stripe_md *lov_lsm_alloc(u16 stripe_count, u32 pattern, u32 magic)
+static struct lov_stripe_md *lov_lsm_alloc(u16 stripe_count, u32 pattern,
+ u32 magic)
{
struct lov_stripe_md *lsm;
unsigned int i;
--
2.7.4

2016-12-07 23:46:53

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 0/8] Sparse warning fixes in Lustre.

On Wed, Dec 07, 2016 at 05:41:26PM -0500, Oleg Drokin wrote:
> This set of fixes aims at sparse warnings.

Speaking of the stuff sparse catches there: class_process_proc_param().
I've tried to describe what I think of that Fine Piece Of Software
several times, but I had to give up - my command of obscenity is not
up to the task, neither in English nor in Russian. Please, take it
out. Preferably - along with the ->ldo_process_config()/->process_config()
thing.

2016-12-08 00:16:53

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 2/8] staging/lustre/ldlm: Correct itree_overlap_cb return type


> As per interval_search() prototype, the callback should return
> enum, not int.
> This fixes correspondign sparse warning.
>
> Signed-off-by: Oleg Drokin <[email protected]>

Reviewed-by: James Simmons <[email protected]>

> ---
> drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> index a4a291a..f4cbc89 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> @@ -1148,7 +1148,7 @@ static int lock_matches(struct ldlm_lock *lock, struct lock_match_data *data)
> return INTERVAL_ITER_STOP;
> }
>
> -static unsigned int itree_overlap_cb(struct interval_node *in, void *args)
> +static enum interval_iter itree_overlap_cb(struct interval_node *in, void *args)
> {
> struct ldlm_interval *node = to_ldlm_interval(in);
> struct lock_match_data *data = args;
> --
> 2.7.4
>
>

2016-12-08 00:21:22

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 2/8] staging/lustre/ldlm: Correct itree_overlap_cb return type


> As per interval_search() prototype, the callback should return
> enum, not int.
> This fixes correspondign sparse warning.
>
> Signed-off-by: Oleg Drokin <[email protected]>

Reviewed-by: James Simmons <[email protected]>

> ---
> drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> index a4a291a..f4cbc89 100644
> --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c
> @@ -1148,7 +1148,7 @@ static int lock_matches(struct ldlm_lock *lock, struct lock_match_data *data)
> return INTERVAL_ITER_STOP;
> }
>
> -static unsigned int itree_overlap_cb(struct interval_node *in, void *args)
> +static enum interval_iter itree_overlap_cb(struct interval_node *in, void *args)
> {
> struct ldlm_interval *node = to_ldlm_interval(in);
> struct lock_match_data *data = args;
> --
> 2.7.4
>
>

2016-12-08 00:22:08

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 5/8] staging/lustre/osc: extern declare osc_caches in a header


> This avoids frowned upon extern in the C file, and also
> shuts down a sparse warning of
> drivers/staging/lustre/lustre/osc/osc_dev.c:55:22: warning: symbol 'osc_caches' was not declared. Should it be static?

Reviewed-by: James Simmons <[email protected]>

> Signed-off-by: Oleg Drokin <[email protected]>
> ---
> drivers/staging/lustre/lustre/osc/osc_internal.h | 2 ++
> drivers/staging/lustre/lustre/osc/osc_request.c | 2 --
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/osc/osc_internal.h b/drivers/staging/lustre/lustre/osc/osc_internal.h
> index 688783d..5cce82b 100644
> --- a/drivers/staging/lustre/lustre/osc/osc_internal.h
> +++ b/drivers/staging/lustre/lustre/osc/osc_internal.h
> @@ -181,6 +181,8 @@ static inline struct osc_device *obd2osc_dev(const struct obd_device *d)
> return container_of0(d->obd_lu_dev, struct osc_device, od_cl.cd_lu_dev);
> }
>
> +extern struct lu_kmem_descr osc_caches[];
> +
> extern struct kmem_cache *osc_quota_kmem;
> struct osc_quota_info {
> /** linkage for quota hash table */
> diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
> index 7143564..f691297 100644
> --- a/drivers/staging/lustre/lustre/osc/osc_request.c
> +++ b/drivers/staging/lustre/lustre/osc/osc_request.c
> @@ -2766,8 +2766,6 @@ static struct obd_ops osc_obd_ops = {
> .quotactl = osc_quotactl,
> };
>
> -extern struct lu_kmem_descr osc_caches[];
> -
> static int __init osc_init(void)
> {
> struct lprocfs_static_vars lvars = { NULL };
> --
> 2.7.4
>
>

2016-12-08 00:23:21

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 5/8] staging/lustre/osc: extern declare osc_caches in a header


> This avoids frowned upon extern in the C file, and also
> shuts down a sparse warning of
> drivers/staging/lustre/lustre/osc/osc_dev.c:55:22: warning: symbol 'osc_caches' was not declared. Should it be static?

Reviewed-by: James Simmons <[email protected]>

> Signed-off-by: Oleg Drokin <[email protected]>
> ---
> drivers/staging/lustre/lustre/osc/osc_internal.h | 2 ++
> drivers/staging/lustre/lustre/osc/osc_request.c | 2 --
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/osc/osc_internal.h b/drivers/staging/lustre/lustre/osc/osc_internal.h
> index 688783d..5cce82b 100644
> --- a/drivers/staging/lustre/lustre/osc/osc_internal.h
> +++ b/drivers/staging/lustre/lustre/osc/osc_internal.h
> @@ -181,6 +181,8 @@ static inline struct osc_device *obd2osc_dev(const struct obd_device *d)
> return container_of0(d->obd_lu_dev, struct osc_device, od_cl.cd_lu_dev);
> }
>
> +extern struct lu_kmem_descr osc_caches[];
> +
> extern struct kmem_cache *osc_quota_kmem;
> struct osc_quota_info {
> /** linkage for quota hash table */
> diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
> index 7143564..f691297 100644
> --- a/drivers/staging/lustre/lustre/osc/osc_request.c
> +++ b/drivers/staging/lustre/lustre/osc/osc_request.c
> @@ -2766,8 +2766,6 @@ static struct obd_ops osc_obd_ops = {
> .quotactl = osc_quotactl,
> };
>
> -extern struct lu_kmem_descr osc_caches[];
> -
> static int __init osc_init(void)
> {
> struct lprocfs_static_vars lvars = { NULL };
> --
> 2.7.4
>
>

2016-12-08 00:24:01

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 4/8] staging/lustre/lov: make lov_lsm_alloc() static


> It's not used anywhere outside of this file.
> Highlighted by sparse.

Reviewed-by: James Simmons <[email protected]>

> Signed-off-by: Oleg Drokin <[email protected]>
> ---
> drivers/staging/lustre/lustre/lov/lov_pack.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/lustre/lustre/lov/lov_pack.c b/drivers/staging/lustre/lustre/lov/lov_pack.c
> index 6c93d18..68fa2de 100644
> --- a/drivers/staging/lustre/lustre/lov/lov_pack.c
> +++ b/drivers/staging/lustre/lustre/lov/lov_pack.c
> @@ -198,7 +198,8 @@ static int lov_verify_lmm(void *lmm, int lmm_bytes, __u16 *stripe_count)
> return rc;
> }
>
> -struct lov_stripe_md *lov_lsm_alloc(u16 stripe_count, u32 pattern, u32 magic)
> +static struct lov_stripe_md *lov_lsm_alloc(u16 stripe_count, u32 pattern,
> + u32 magic)
> {
> struct lov_stripe_md *lsm;
> unsigned int i;
> --
> 2.7.4
>
>

2016-12-08 00:25:39

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 3/8] staging/lustre/llite: mark ll_io_init() static

> It's not used anywhere out of this file.
> Highlighted by sparse.

Reviewed-by: James Simmons <[email protected]>

> Signed-off-by: Oleg Drokin <[email protected]>
> ---
> drivers/staging/lustre/lustre/llite/file.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
> index f634c11..d93f06a 100644
> --- a/drivers/staging/lustre/lustre/llite/file.c
> +++ b/drivers/staging/lustre/lustre/llite/file.c
> @@ -1016,7 +1016,7 @@ static bool file_is_noatime(const struct file *file)
> return false;
> }
>
> -void ll_io_init(struct cl_io *io, const struct file *file, int write)
> +static void ll_io_init(struct cl_io *io, const struct file *file, int write)
> {
> struct inode *inode = file_inode(file);
>
> --
> 2.7.4
>
>

2016-12-08 00:25:37

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 7/8] staging/lustre: Move lov_read_and_clear_async_rc declaration


> Move it to obd.h, so that it's included from both the users and
> the actual definition, making sure they never get out of sync.
> This also silences a sparse warning.

Reviewed-by: James Simmons <[email protected]>

> Signed-off-by: Oleg Drokin <[email protected]>
> ---
> drivers/staging/lustre/lustre/include/obd.h | 3 +++
> drivers/staging/lustre/lustre/llite/vvp_internal.h | 2 --
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
> index 0f48e9c..1839f4f 100644
> --- a/drivers/staging/lustre/lustre/include/obd.h
> +++ b/drivers/staging/lustre/lustre/include/obd.h
> @@ -43,6 +43,7 @@
> #include "lustre_fld.h"
> #include "lustre_handles.h"
> #include "lustre_intent.h"
> +#include "cl_object.h"
>
> #define MAX_OBD_DEVICES 8192
>
> @@ -76,6 +77,8 @@ static inline void loi_init(struct lov_oinfo *loi)
> struct lov_stripe_md;
> struct obd_info;
>
> +int lov_read_and_clear_async_rc(struct cl_object *clob);
> +
> typedef int (*obd_enqueue_update_f)(void *cookie, int rc);
>
> /* obd info for a particular level (lov, osc). */
> diff --git a/drivers/staging/lustre/lustre/llite/vvp_internal.h b/drivers/staging/lustre/lustre/llite/vvp_internal.h
> index c60d041..f40fd7f 100644
> --- a/drivers/staging/lustre/lustre/llite/vvp_internal.h
> +++ b/drivers/staging/lustre/lustre/llite/vvp_internal.h
> @@ -301,8 +301,6 @@ static inline struct vvp_lock *cl2vvp_lock(const struct cl_lock_slice *slice)
> # define CLOBINVRNT(env, clob, expr) \
> ((void)sizeof(env), (void)sizeof(clob), (void)sizeof(!!(expr)))
>
> -int lov_read_and_clear_async_rc(struct cl_object *clob);
> -
> int vvp_io_init(const struct lu_env *env, struct cl_object *obj,
> struct cl_io *io);
> int vvp_io_write_commit(const struct lu_env *env, struct cl_io *io);
> --
> 2.7.4
>
>

2016-12-08 00:27:34

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 8/8] staging/lustre/ptlrpc: Move nrs_conf_fifo extern to a header

> This avoids having an extern definition in a C file which is bad,
> and also silences sparse complaint as well.
>
> Signed-off-by: Oleg Drokin <[email protected]>

Reviewed-by: James Simmons <[email protected]>

> ---
> drivers/staging/lustre/lustre/ptlrpc/nrs.c | 3 ---
> drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h | 3 +++
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/nrs.c b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
> index 7b6ffb1..ef19dbe 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/nrs.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
> @@ -1559,9 +1559,6 @@ int ptlrpc_nrs_policy_control(const struct ptlrpc_service *svc,
> return rc;
> }
>
> -/* ptlrpc/nrs_fifo.c */
> -extern struct ptlrpc_nrs_pol_conf nrs_conf_fifo;
> -
> /**
> * Adds all policies that ship with the ptlrpc module, to NRS core's list of
> * policies \e nrs_core.nrs_policies.
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h b/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
> index e0f859c..8e6a805 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
> +++ b/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
> @@ -226,6 +226,9 @@ struct ptlrpc_nrs_policy *nrs_request_policy(struct ptlrpc_nrs_request *nrq)
> sizeof(NRS_LPROCFS_QUANTUM_NAME_REG __stringify(LPROCFS_NRS_QUANTUM_MAX) " " \
> NRS_LPROCFS_QUANTUM_NAME_HP __stringify(LPROCFS_NRS_QUANTUM_MAX))
>
> +/* ptlrpc/nrs_fifo.c */
> +extern struct ptlrpc_nrs_pol_conf nrs_conf_fifo;
> +
> /* recovd_thread.c */
>
> int ptlrpc_expire_one_request(struct ptlrpc_request *req, int async_unlink);
> --
> 2.7.4
>
>

2016-12-08 00:28:31

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 6/8] staging/lustre: Declare lu_context/session_tags_default


> Make the declaration in a header, not as an extern in a C file,
> that is frowned upon.
> This also makes sparse a little bit more happy.
>

Reviewed-by: James Simmons <[email protected]>

> Signed-off-by: Oleg Drokin <[email protected]>
> ---
> drivers/staging/lustre/lustre/include/lu_object.h | 3 +++
> drivers/staging/lustre/lustre/obdclass/cl_object.c | 3 +--
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/include/lu_object.h b/drivers/staging/lustre/lustre/include/lu_object.h
> index 260643e..69b2812 100644
> --- a/drivers/staging/lustre/lustre/include/lu_object.h
> +++ b/drivers/staging/lustre/lustre/include/lu_object.h
> @@ -1326,5 +1326,8 @@ void lu_buf_realloc(struct lu_buf *buf, size_t size);
> int lu_buf_check_and_grow(struct lu_buf *buf, size_t len);
> struct lu_buf *lu_buf_check_and_alloc(struct lu_buf *buf, size_t len);
>
> +extern __u32 lu_context_tags_default;
> +extern __u32 lu_session_tags_default;
> +
> /** @} lu */
> #endif /* __LUSTRE_LU_OBJECT_H */
> diff --git a/drivers/staging/lustre/lustre/obdclass/cl_object.c b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> index f5d4e23..703cb67 100644
> --- a/drivers/staging/lustre/lustre/obdclass/cl_object.c
> +++ b/drivers/staging/lustre/lustre/obdclass/cl_object.c
> @@ -54,6 +54,7 @@
> #include <linux/list.h>
> #include "../../include/linux/libcfs/libcfs_hash.h" /* for cfs_hash stuff */
> #include "../include/cl_object.h"
> +#include "../include/lu_object.h"
> #include "cl_internal.h"
>
> static struct kmem_cache *cl_env_kmem;
> @@ -61,8 +62,6 @@ static struct kmem_cache *cl_env_kmem;
> /** Lock class of cl_object_header::coh_attr_guard */
> static struct lock_class_key cl_attr_guard_class;
>
> -extern __u32 lu_context_tags_default;
> -extern __u32 lu_session_tags_default;
> /**
> * Initialize cl_object_header.
> */
> --
> 2.7.4
>
>

2016-12-08 00:33:18

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 8/8] staging/lustre/ptlrpc: Move nrs_conf_fifo extern to a header


> This avoids having an extern definition in a C file which is bad,
> and also silences sparse complaint as well.
>
> Signed-off-by: Oleg Drokin <[email protected]>

Reviewed-by: James Simmons <[email protected]>

> ---
> drivers/staging/lustre/lustre/ptlrpc/nrs.c | 3 ---
> drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h | 3 +++
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/nrs.c b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
> index 7b6ffb1..ef19dbe 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/nrs.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/nrs.c
> @@ -1559,9 +1559,6 @@ int ptlrpc_nrs_policy_control(const struct ptlrpc_service *svc,
> return rc;
> }
>
> -/* ptlrpc/nrs_fifo.c */
> -extern struct ptlrpc_nrs_pol_conf nrs_conf_fifo;
> -
> /**
> * Adds all policies that ship with the ptlrpc module, to NRS core's list of
> * policies \e nrs_core.nrs_policies.
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h b/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
> index e0f859c..8e6a805 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
> +++ b/drivers/staging/lustre/lustre/ptlrpc/ptlrpc_internal.h
> @@ -226,6 +226,9 @@ struct ptlrpc_nrs_policy *nrs_request_policy(struct ptlrpc_nrs_request *nrq)
> sizeof(NRS_LPROCFS_QUANTUM_NAME_REG __stringify(LPROCFS_NRS_QUANTUM_MAX) " " \
> NRS_LPROCFS_QUANTUM_NAME_HP __stringify(LPROCFS_NRS_QUANTUM_MAX))
>
> +/* ptlrpc/nrs_fifo.c */
> +extern struct ptlrpc_nrs_pol_conf nrs_conf_fifo;
> +
> /* recovd_thread.c */
>
> int ptlrpc_expire_one_request(struct ptlrpc_request *req, int async_unlink);
> --
> 2.7.4
>
>

2016-12-08 00:44:44

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 1/8] staging/lustre/llite: move root_squash from sysfs to debugfs


> root_squash control got accidentally moved to sysfs instead of
> debugfs, and the write side of it was also broken expecting a
> userspace buffer.
> It contains both uid and gid values in a single file, so debugfs
> is a clear place for it.

I see why this was missed. The uid/gid pair I needed for my testing
was missing so the test was skipped. I fixed it up and tested this
patch out. Now it passes.

Reviewed-by: James Simmons <[email protected]>

> Reported-by: Al Viro <[email protected]>
> Fixes: c948390f10ccc "fix inconsistencies of root squash feature"
> Signed-off-by: Oleg Drokin <[email protected]>
> ---
> drivers/staging/lustre/lustre/llite/lproc_llite.c | 27 +++++++++++++----------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/lproc_llite.c b/drivers/staging/lustre/lustre/llite/lproc_llite.c
> index 03682c1..f3ee584 100644
> --- a/drivers/staging/lustre/lustre/llite/lproc_llite.c
> +++ b/drivers/staging/lustre/lustre/llite/lproc_llite.c
> @@ -924,27 +924,29 @@ static ssize_t ll_unstable_stats_seq_write(struct file *file,
> }
> LPROC_SEQ_FOPS(ll_unstable_stats);
>
> -static ssize_t root_squash_show(struct kobject *kobj, struct attribute *attr,
> - char *buf)
> +static int ll_root_squash_seq_show(struct seq_file *m, void *v)
> {
> - struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info,
> - ll_kobj);
> + struct super_block *sb = m->private;
> + struct ll_sb_info *sbi = ll_s2sbi(sb);
> struct root_squash_info *squash = &sbi->ll_squash;
>
> - return sprintf(buf, "%u:%u\n", squash->rsi_uid, squash->rsi_gid);
> + seq_printf(m, "%u:%u\n", squash->rsi_uid, squash->rsi_gid);
> + return 0;
> }
>
> -static ssize_t root_squash_store(struct kobject *kobj, struct attribute *attr,
> - const char *buffer, size_t count)
> +static ssize_t ll_root_squash_seq_write(struct file *file,
> + const char __user *buffer,
> + size_t count, loff_t *off)
> {
> - struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info,
> - ll_kobj);
> + struct seq_file *m = file->private_data;
> + struct super_block *sb = m->private;
> + struct ll_sb_info *sbi = ll_s2sbi(sb);
> struct root_squash_info *squash = &sbi->ll_squash;
>
> return lprocfs_wr_root_squash(buffer, count, squash,
> - ll_get_fsname(sbi->ll_sb, NULL, 0));
> + ll_get_fsname(sb, NULL, 0));
> }
> -LUSTRE_RW_ATTR(root_squash);
> +LPROC_SEQ_FOPS(ll_root_squash);
>
> static int ll_nosquash_nids_seq_show(struct seq_file *m, void *v)
> {
> @@ -997,6 +999,8 @@ static struct lprocfs_vars lprocfs_llite_obd_vars[] = {
> { "statahead_stats", &ll_statahead_stats_fops, NULL, 0 },
> { "unstable_stats", &ll_unstable_stats_fops, NULL },
> { "sbi_flags", &ll_sbi_flags_fops, NULL, 0 },
> + { .name = "root_squash",
> + .fops = &ll_root_squash_fops },
> { .name = "nosquash_nids",
> .fops = &ll_nosquash_nids_fops },
> { NULL }
> @@ -1027,7 +1031,6 @@ static struct attribute *llite_attrs[] = {
> &lustre_attr_max_easize.attr,
> &lustre_attr_default_easize.attr,
> &lustre_attr_xattr_cache.attr,
> - &lustre_attr_root_squash.attr,
> NULL,
> };
>
> --
> 2.7.4
>
>

2016-12-13 04:08:38

by Oleg Drokin

[permalink] [raw]
Subject: Re: [PATCH 0/8] Sparse warning fixes in Lustre.


On Dec 7, 2016, at 6:46 PM, Al Viro wrote:

> On Wed, Dec 07, 2016 at 05:41:26PM -0500, Oleg Drokin wrote:
>> This set of fixes aims at sparse warnings.
>
> Speaking of the stuff sparse catches there: class_process_proc_param().
> I've tried to describe what I think of that Fine Piece Of Software
> several times, but I had to give up - my command of obscenity is not
> up to the task, neither in English nor in Russian. Please, take it
> out. Preferably - along with the ->ldo_process_config()/->process_config()
> thing.

Well, I can guess what you don't like in the remnants of the
"well, we have uniform procfs, so let's use that to our advantage and simplify
or config parsing".

But what's your beef with ldo_process_config()/->process_config(), I wonder?
Just a way to propagate config info across the layers.