2015-05-16 07:39:06

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 0/4] Few lustre fixes and cleanups.

From: Oleg Drokin <[email protected]>

This set fixes a couple of style issues, and a couple of bugs.

Please consider

Oleg Drokin (4):
staging/lustre: Only set INTERRUPTIBLE state before calling schedule
staging/lustre/ptlrpc: Fix wrong indenting in plain_authorize()
staging/lustre/ptlrpc: Fix potential NULL pointer dereference
staging/lustre/llite: Fix wrong identing in ll_setxattr_common

drivers/staging/lustre/lustre/include/lustre_lib.h | 15 +++++----------
drivers/staging/lustre/lustre/llite/xattr.c | 10 +++++-----
drivers/staging/lustre/lustre/lov/lov_pack.c | 3 ++-
drivers/staging/lustre/lustre/ptlrpc/sec_plain.c | 2 +-
4 files changed, 13 insertions(+), 17 deletions(-)

--
2.1.0


2015-05-16 07:39:12

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 1/4] staging/lustre: Only set INTERRUPTIBLE state before calling schedule

From: Oleg Drokin <[email protected]>

In __l_wait_event the condition could be a complicated function that does
allocations and other potentialy blocking activities, so it sohuld
not be called in a task state other than RUNNABLE

Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/lustre/include/lustre_lib.h | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre_lib.h b/drivers/staging/lustre/lustre/include/lustre_lib.h
index bf13563..43ee9f0 100644
--- a/drivers/staging/lustre/lustre/include/lustre_lib.h
+++ b/drivers/staging/lustre/lustre/include/lustre_lib.h
@@ -549,19 +549,13 @@ do { \
__blocked = cfs_block_sigsinv(0); \
\
for (;;) { \
- unsigned __wstate; \
- \
- __wstate = info->lwi_on_signal != NULL && \
- (__timeout == 0 || __allow_intr) ? \
- TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE; \
- \
- set_current_state(TASK_INTERRUPTIBLE); \
- \
if (condition) \
break; \
\
+ set_current_state(TASK_INTERRUPTIBLE); \
+ \
if (__timeout == 0) { \
- schedule(); \
+ schedule(); \
} else { \
long interval = info->lwi_interval? \
min_t(long, \
@@ -582,6 +576,8 @@ do { \
} \
} \
\
+ set_current_state(TASK_RUNNING); \
+ \
if (condition) \
break; \
if (cfs_signal_pending()) { \
@@ -605,7 +601,6 @@ do { \
\
cfs_restore_sigs(__blocked); \
\
- set_current_state(TASK_RUNNING); \
remove_wait_queue(&wq, &__wait); \
} while (0)

--
2.1.0

2015-05-16 07:39:18

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 2/4] staging/lustre/ptlrpc: Fix wrong indenting in plain_authorize()

From: Oleg Drokin <[email protected]>

smatch highlighted a wrongly indented bit of code that almost
hides the extra assignment.

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

diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c b/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
index 989cdcd..65f3ab4 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
@@ -870,7 +870,7 @@ int plain_authorize(struct ptlrpc_request *req)
lustre_msg_buf(msg, PLAIN_PACK_MSG_OFF, 0),
lustre_msg_buflen(msg, PLAIN_PACK_MSG_OFF),
NULL, 0, (unsigned char *)&msg->lm_cksum, &hsize);
- req->rq_reply_off = 0;
+ req->rq_reply_off = 0;
}

return 0;
--
2.1.0

2015-05-16 07:40:15

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 3/4] staging/lustre/ptlrpc: Fix potential NULL pointer dereference

From: Oleg Drokin <[email protected]>

In lov_unpackmd() there's this strange bit of code where we first try
to look inside of lmm striping pattern for it's type, and then
we check if the pattern is NULL which cannot be right.
Move the check under if (lmm) branch so that it's safe.

Found by Coverity version 6.6.1

Signed-off-by: Sebastien Buisson <[email protected]>
Reviewed-on: http://review.whamcloud.com/7827
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-4049
Reviewed-by: John L. Hammond <[email protected]>
Reviewed-by: jacques-Charles Lafoucriere <[email protected]>
Signed-off: 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 5356d53..92b9ffe 100644
--- a/drivers/staging/lustre/lustre/lov/lov_pack.c
+++ b/drivers/staging/lustre/lustre/lov/lov_pack.c
@@ -367,9 +367,11 @@ int lov_unpackmd(struct obd_export *exp, struct lov_stripe_md **lsmp,
if (rc)
return rc;
magic = le32_to_cpu(lmm->lmm_magic);
+ pattern = le32_to_cpu(lmm->lmm_pattern);
} else {
magic = LOV_MAGIC;
stripe_count = lov_get_stripecnt(lov, magic, 0);
+ pattern = LOV_PATTERN_RAID0;
}

/* If we aren't passed an lsmp struct, we just want the size */
@@ -384,7 +386,6 @@ int lov_unpackmd(struct obd_export *exp, struct lov_stripe_md **lsmp,
return 0;
}

- pattern = le32_to_cpu(lmm->lmm_pattern);
lsm_size = lov_alloc_memmd(lsmp, stripe_count, pattern, magic);
if (lsm_size < 0)
return lsm_size;
--
2.1.0

2015-05-16 07:40:37

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 4/4] staging/lustre/llite: Fix wrong identing in ll_setxattr_common

From: Oleg Drokin <[email protected]>

smatch has highlighted wrong indenting that results from
commit 7fc1f831d83f ("staging/lustre/llite: extended attribute cache")

Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/lustre/llite/xattr.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/xattr.c b/drivers/staging/lustre/lustre/llite/xattr.c
index e0fcbe1..362a87d 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lustre/lustre/llite/xattr.c
@@ -188,11 +188,11 @@ int ll_setxattr_common(struct inode *inode, const char *name,
valid |= rce_ops2valid(rce->rce_ops);
}
#endif
- oc = ll_mdscapa_get(inode);
- rc = md_setxattr(sbi->ll_md_exp, ll_inode2fid(inode), oc,
- valid, name, pv, size, 0, flags,
- ll_i2suppgid(inode), &req);
- capa_put(oc);
+ oc = ll_mdscapa_get(inode);
+ rc = md_setxattr(sbi->ll_md_exp, ll_inode2fid(inode), oc,
+ valid, name, pv, size, 0, flags,
+ ll_i2suppgid(inode), &req);
+ capa_put(oc);
#ifdef CONFIG_FS_POSIX_ACL
if (new_value != NULL)
lustre_posix_acl_xattr_free(new_value, size);
--
2.1.0