2014-01-24 05:32:08

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 0/5] Lustre fixes from my testing

I just deployed a mainline lustre client in my test system.
These patches below are for stuff that causes crashes or other issues
in my (so far) very basic sanity testing (more to come).

The patches mostly originate from our main tree, but I decided to
prioritize these as they disrupt my testing, obviously.

Also adding myself as one of the people who others would hopefully
CC all lustre patches to.

Please consider.

Oleg Drokin (5):
staging/lustre: fix compile warning with is_vmalloc_addr
staging/lustre/lnet: Fix use after free in ksocknal_send
lustre: Account for changelog_ext_rec in CR_MAXSIZE
lustre: Correct KUC code max changelog msg size
lustre: add myself to list of people to CC on lustre patches

drivers/staging/lustre/TODO | 5 +++--
drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h | 2 ++
drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 2 +-
drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 5 +++--
drivers/staging/lustre/lustre/include/lustre/lustre_user.h | 3 ++-
drivers/staging/lustre/lustre/mdc/mdc_request.c | 6 +++---
6 files changed, 14 insertions(+), 9 deletions(-)

--
1.8.5.3


2014-01-24 05:32:16

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 4/5] lustre: Correct KUC code max changelog msg size

The kernel to userspace communication routines (KUC) allocate
and limit the maximum cs_buf size to CR_MAXSIZE. However this
fails to account for the fact that the buffer is assumed to begin
with a struct kuc_hdr. To allocate and account for that space,
we introduce a new define, KUC_CHANGELOG_MSG_MAXSIZE.

Signed-off-by: Christopher J. Morrone <[email protected]>
Reviewed-on: http://review.whamcloud.com/7406
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3587
Reviewed-by: Andreas Dilger <[email protected]>
Reviewed-by: jacques-Charles Lafoucriere <[email protected]>
Reviewed-by: Oleg Drokin <[email protected]>
Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h | 2 ++
drivers/staging/lustre/lustre/mdc/mdc_request.c | 6 +++---
2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h
index 596a15f..037ae8a 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_kernelcomm.h
@@ -61,6 +61,8 @@ struct kuc_hdr {
__u16 kuc_msglen; /* Including header */
} __attribute__((aligned(sizeof(__u64))));

+#define KUC_CHANGELOG_MSG_MAXSIZE (sizeof(struct kuc_hdr)+CR_MAXSIZE)
+
#define KUC_MAGIC 0x191C /*Lustre9etLinC */
#define KUC_FL_BLOCK 0x01 /* Wait for send */

diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
index d1ad91c3..8301392 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -1430,7 +1430,7 @@ static struct kuc_hdr *changelog_kuc_hdr(char *buf, int len, int flags)
{
struct kuc_hdr *lh = (struct kuc_hdr *)buf;

- LASSERT(len <= CR_MAXSIZE);
+ LASSERT(len <= KUC_CHANGELOG_MSG_MAXSIZE);

lh->kuc_magic = KUC_MAGIC;
lh->kuc_transport = KUC_TRANSPORT_CHANGELOG;
@@ -1503,7 +1503,7 @@ static int mdc_changelog_send_thread(void *csdata)
CDEBUG(D_CHANGELOG, "changelog to fp=%p start "LPU64"\n",
cs->cs_fp, cs->cs_startrec);

- OBD_ALLOC(cs->cs_buf, CR_MAXSIZE);
+ OBD_ALLOC(cs->cs_buf, KUC_CHANGELOG_MSG_MAXSIZE);
if (cs->cs_buf == NULL)
GOTO(out, rc = -ENOMEM);

@@ -1540,7 +1540,7 @@ out:
if (ctxt)
llog_ctxt_put(ctxt);
if (cs->cs_buf)
- OBD_FREE(cs->cs_buf, CR_MAXSIZE);
+ OBD_FREE(cs->cs_buf, KUC_CHANGELOG_MSG_MAXSIZE);
OBD_FREE_PTR(cs);
return rc;
}
--
1.8.5.3

2014-01-24 05:32:15

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 3/5] lustre: Account for changelog_ext_rec in CR_MAXSIZE

CR_MAXSIZE needs to account for an llog_changelog_rec that actually
contains a changelog_ext_rec structure rather than a changelog_rec.
With out doing so, a file size approaching the Linux kernel NAME_MAX
length that is renamed to a size also close to, or at, NAME_MAX will
exceed CR_MAXSIZE and trip an assertion.

Signed-off-by: Christopher J. Morrone <[email protected]>
Reviewed-on: http://review.whamcloud.com/6993
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3587
Reviewed-by: Niu Yawei <[email protected]>
Reviewed-by: Lai Siyao <[email protected]>
Reviewed-by: Oleg Drokin <[email protected]>
Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/lustre/include/lustre/lustre_user.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h
index 6b6c0240..7893d83 100644
--- a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h
+++ b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h
@@ -760,7 +760,8 @@ static inline void hsm_set_cl_error(int *flags, int error)
*flags |= (error << CLF_HSM_ERR_L);
}

-#define CR_MAXSIZE cfs_size_round(2*NAME_MAX + 1 + sizeof(struct changelog_rec))
+#define CR_MAXSIZE cfs_size_round(2*NAME_MAX + 1 + \
+ sizeof(struct changelog_ext_rec))

struct changelog_rec {
__u16 cr_namelen;
--
1.8.5.3

2014-01-24 05:32:13

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 2/5] staging/lustre/lnet: Fix use after free in ksocknal_send

Call to ksocknal_launch_packet might schedule a callback that
might free the just sent message, and so subsequent access to it
via lntmsg->msg_vmflush goes to freed memory.

Instead we'll just remember if we are in the vmflush thread and
only restore if we happened to set mempressure flag.

Signed-off-by: Oleg Drokin <[email protected]>
Reviewed-on: http://review.whamcloud.com/8667
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-4360
Reviewed-by: Liang Zhen <[email protected]>
Reviewed-by: Amir Shehata <[email protected]>
---
drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
index 68a4f52..b7b53b5 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
@@ -924,7 +924,7 @@ ksocknal_launch_packet (lnet_ni_t *ni, ksock_tx_t *tx, lnet_process_id_t id)
int
ksocknal_send(lnet_ni_t *ni, void *private, lnet_msg_t *lntmsg)
{
- int mpflag = 0;
+ int mpflag = 1;
int type = lntmsg->msg_type;
lnet_process_id_t target = lntmsg->msg_target;
unsigned int payload_niov = lntmsg->msg_niov;
@@ -993,8 +993,9 @@ ksocknal_send(lnet_ni_t *ni, void *private, lnet_msg_t *lntmsg)

/* The first fragment will be set later in pro_pack */
rc = ksocknal_launch_packet(ni, tx, target);
- if (lntmsg->msg_vmflush)
+ if (!mpflag)
cfs_memory_pressure_restore(mpflag);
+
if (rc == 0)
return (0);

--
1.8.5.3

2014-01-24 05:32:12

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 1/5] staging/lustre: fix compile warning with is_vmalloc_addr

Recent commit 175f5475fb9c5800319da4e3c4204413d7280f5c
introduced this compile warning (because vaddr is unsigned long),
so add a cast:
drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c: In function ‘kiblnd_kvaddr_to_page’:
drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:532:2: warning: passing argument 1 of ‘is_vmalloc_addr’ makes pointer from integer without a cast [enabled by default]
if (is_vmalloc_addr(vaddr)) {
^
In file included from drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h:43:0,
from drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c:41:
include/linux/mm.h:336:59: note: expected ‘const void *’ but argument is of type ‘long unsigned int’
static inline int is_vmalloc_addr(const void *x)

Signed-off-by: Oleg Drokin <[email protected]>
CC: Laura Abbott <[email protected]>
---
drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
index 9364863..6f58ead 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
@@ -529,7 +529,7 @@ kiblnd_kvaddr_to_page (unsigned long vaddr)
{
struct page *page;

- if (is_vmalloc_addr(vaddr)) {
+ if (is_vmalloc_addr((void *)vaddr)) {
page = vmalloc_to_page ((void *)vaddr);
LASSERT (page != NULL);
return page;
--
1.8.5.3

2014-01-24 05:32:10

by Oleg Drokin

[permalink] [raw]
Subject: [PATCH 5/5] lustre: add myself to list of people to CC on lustre patches

Signed-off-by: Oleg Drokin <[email protected]>
---
drivers/staging/lustre/TODO | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/TODO b/drivers/staging/lustre/TODO
index 22742d6..0a2b6cb 100644
--- a/drivers/staging/lustre/TODO
+++ b/drivers/staging/lustre/TODO
@@ -9,5 +9,6 @@
* Other minor misc cleanups...

Please send any patches to Greg Kroah-Hartman <[email protected]>, Andreas Dilger
-<[email protected]> and Peng Tao <[email protected]>. CCing
-hpdd-discuss <[email protected]> would be great too.
+<[email protected]>, Oleg Drokin <[email protected]> and
+Peng Tao <[email protected]>. CCing hpdd-discuss <[email protected]>
+would be great too.
--
1.8.5.3

2014-01-24 05:44:43

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 5/5] lustre: add myself to list of people to CC on lustre patches

Add a MAINTAINERS entry too?
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5c5d554..9ca5de8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8284,6 +8284,14 @@ W: http://www.lirc.org/
S: Odd Fixes
F: drivers/staging/media/lirc/

+STAGING - LUSTRE
+M: Andreas Dilger <[email protected]>
+M: Oleg Drokin <[email protected]>
+M: Peng Tao <[email protected]>.
+L: hpdd-discuss <[email protected]>
+S: Odd Fixes
+F: drivers/staging/lustre/
+
STAGING - NVIDIA COMPLIANT EMBEDDED CONTROLLER INTERFACE (nvec)
M: Julian Andres Klode <[email protected]>
M: Marc Dietrich <[email protected]>

2014-01-24 05:51:39

by Oleg Drokin

[permalink] [raw]
Subject: Re: [PATCH 5/5] lustre: add myself to list of people to CC on lustre patches

Hello!

> +STAGING - LUSTRE
> +M: Andreas Dilger <[email protected]>
> +M: Oleg Drokin <[email protected]>
> +M: Peng Tao <[email protected]>.
> +L: hpdd-discuss <[email protected]>
> +S: Odd Fixes

Actually we are at least Maintained here, if not outright Supported.

Bye,
Oleg

2014-01-24 08:12:58

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 5/5] lustre: add myself to list of people to CC on lustre patches

On Thu, Jan 23, 2014 at 11:45:08PM -0500, Oleg Drokin wrote:
> Signed-off-by: Oleg Drokin <[email protected]>

Add yourself to MAINTAINERS if you really want email. No one reads
the email addresses here.

regards,
dan carpenter

2014-01-24 08:55:39

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 5/5] lustre: add myself to list of people to CC on lustre patches

On Fri, Jan 24, 2014 at 6:51 AM, Oleg Drokin <[email protected]> wrote:
>> +STAGING - LUSTRE
>> +M: Andreas Dilger <[email protected]>
>> +M: Oleg Drokin <[email protected]>
>> +M: Peng Tao <[email protected]>.
>> +L: hpdd-discuss <[email protected]>
>> +S: Odd Fixes
>
> Actually we are at least Maintained here, if not outright Supported.

Good to hear that!

So can we assume that https://lkml.org/lkml/2013/9/9/725 will be fixed
shortly, and http://kisskb.ellerman.id.au/kisskb/buildresult/10508264/
will turn green again?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2014-01-25 03:23:57

by Oleg Drokin

[permalink] [raw]
Subject: Re: [PATCH 5/5] lustre: add myself to list of people to CC on lustre patches

Hello!

On Jan 24, 2014, at 3:55 AM, Geert Uytterhoeven wrote:

> On Fri, Jan 24, 2014 at 6:51 AM, Oleg Drokin <[email protected]> wrote:
>>> +STAGING - LUSTRE
>>> +M: Andreas Dilger <[email protected]>
>>> +M: Oleg Drokin <[email protected]>
>>> +M: Peng Tao <[email protected]>.
>>> +L: hpdd-discuss <[email protected]>
>>> +S: Odd Fixes
>>
>> Actually we are at least Maintained here, if not outright Supported.
>
> Good to hear that!
> So can we assume that https://lkml.org/lkml/2013/9/9/725 will be fixed
> shortly,

We are working on it.

> and http://kisskb.ellerman.id.au/kisskb/buildresult/10508264/
> will turn green again?

I certainly hope so.
BTW, do you also provide interface where I can drop in my patch and your builder verify it for me by any chance,
so that I don't need to replicate build environment for every supported architecture?

Thanks.

Bye,
Oleg-

2014-01-28 19:28:36

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 5/5] lustre: add myself to list of people to CC on lustre patches

On Sat, Jan 25, 2014 at 4:23 AM, Oleg Drokin <[email protected]> wrote:
> On Jan 24, 2014, at 3:55 AM, Geert Uytterhoeven wrote:
>> On Fri, Jan 24, 2014 at 6:51 AM, Oleg Drokin <[email protected]> wrote:
>>>> +STAGING - LUSTRE
>>>> +M: Andreas Dilger <[email protected]>
>>>> +M: Oleg Drokin <[email protected]>
>>>> +M: Peng Tao <[email protected]>.
>>>> +L: hpdd-discuss <[email protected]>
>>>> +S: Odd Fixes
>>>
>>> Actually we are at least Maintained here, if not outright Supported.
>>
>> Good to hear that!
>> So can we assume that https://lkml.org/lkml/2013/9/9/725 will be fixed
>> shortly,
>
> We are working on it.

Thanks!

>> and http://kisskb.ellerman.id.au/kisskb/buildresult/10508264/
>> will turn green again?
>
> I certainly hope so.
> BTW, do you also provide interface where I can drop in my patch and your builder verify it for me by any chance,

It's called linux-next, but Lustre is part of staging, which is already
included. No idea how lustre gets updated in thes staging tree, though.

> so that I don't need to replicate build environment for every supported architecture?

Lots of cross compilers are just a download away:
https://www.kernel.org/pub/tools/crosstool/

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2014-01-29 13:01:33

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 5/5] lustre: add myself to list of people to CC on lustre patches

On Tue, Jan 28, 2014 at 08:28:33PM +0100, Geert Uytterhoeven wrote:
> On Sat, Jan 25, 2014 at 4:23 AM, Oleg Drokin <[email protected]> wrote:
> > On Jan 24, 2014, at 3:55 AM, Geert Uytterhoeven wrote:
> >> On Fri, Jan 24, 2014 at 6:51 AM, Oleg Drokin <[email protected]> wrote:
> >>>> +STAGING - LUSTRE
> >>>> +M: Andreas Dilger <[email protected]>
> >>>> +M: Oleg Drokin <[email protected]>
> >>>> +M: Peng Tao <[email protected]>.
> >>>> +L: hpdd-discuss <[email protected]>
> >>>> +S: Odd Fixes
> >>>
> >>> Actually we are at least Maintained here, if not outright Supported.
> >>
> >> Good to hear that!
> >> So can we assume that https://lkml.org/lkml/2013/9/9/725 will be fixed
> >> shortly,
> >
> > We are working on it.
>
> Thanks!
>
> >> and http://kisskb.ellerman.id.au/kisskb/buildresult/10508264/
> >> will turn green again?
> >
> > I certainly hope so.
> > BTW, do you also provide interface where I can drop in my patch and your builder verify it for me by any chance,
>
> It's called linux-next, but Lustre is part of staging, which is already
> included. No idea how lustre gets updated in thes staging tree, though.

Like any other patch that gets sent to drivers/staging, through my
staging.git tree.