[PATCH 0/3] orangefs fixes for stable

[PATCH 0/3] orangefs fixes for stable

[Martin Brandenburg]

Here are some patches for 4.9 and 4.10 which we missed before.

I've just looked through the changes to OrangeFS since 4.9 was released.
These are the only fixes which seem to qualify.

Colin's patch is already in 4.10 and only needs to be applied to 4.9.
Mike's patches should be applied to 4.9 and 4.10.

(Is there a better way to submit these?  Should I just submit separately
for the two stable trees?)

Colin Ian King (1):
  orangefs: fix memory leak of string 'new' on exit path

Mike Marshall (2):
  orangefs: Dan Carpenter influenced cleanups...
  orangefs: fix buffer size mis-match between kernel space and user
    space.

 fs/orangefs/devorangefs-req.c    | 5 +++--
 fs/orangefs/orangefs-debugfs.c   | 4 +++-
 fs/orangefs/orangefs-dev-proto.h | 3 +--
 3 files changed, 7 insertions(+), 5 deletions(-)

-- 
2.11.0

[PATCH 1/3] orangefs: fix memory leak of string 'new' on exit path

[Martin Brandenburg]

From: Colin Ian King <colin.king@canonical.com>

commit 4defb5f912a0ba60e07e91a4b62634814cd99b7f upstream.

allocates string 'new' is not free'd on the exit path when
cdm_element_count <= 0. Fix this by kfree'ing it.

Fixes CoverityScan CID#1375923 "Resource Leak"

Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Mike Marshall <hubcap@omnibond.com>
Signed-off-by: Martin Brandenburg <martin@omnibond.com>
---
 fs/orangefs/orangefs-debugfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/orangefs/orangefs-debugfs.c b/fs/orangefs/orangefs-debugfs.c
index 38887cc5577f..b5dbc9c6530c 100644
--- a/fs/orangefs/orangefs-debugfs.c
+++ b/fs/orangefs/orangefs-debugfs.c
@@ -671,8 +671,10 @@ int orangefs_prepare_debugfs_help_string(int at_boot)
 		 */
 		cdm_element_count =
 			orangefs_prepare_cdm_array(client_debug_array_string);
-		if (cdm_element_count <= 0)
+		if (cdm_element_count <= 0) {
+			kfree(new);
 			goto out;
+		}
 
 		for (i = 0; i < cdm_element_count; i++) {
 			strlcat(new, "\t", string_size);
-- 
2.11.0

[PATCH 2/3] orangefs: Dan Carpenter influenced cleanups...

[Martin Brandenburg]

From: Mike Marshall <hubcap@omnibond.com>

commit 05973c2efb40122f2a9ecde2d065f7ea5068d024 upstream.

This patch is simlar to one Dan Carpenter sent me, cleans
up some return codes and whitespace errors. There was one
place where he thought inserting an error message into
the ring buffer might be too chatty, I hope I convinced him
othewise. As a consolation <g> I changed a truly chatty
error message in another location into a debug message,
system-admins had already yelled at me about that one...

Signed-off-by: Mike Marshall <hubcap@omnibond.com>

I have removed the return code and whitespace fixes as they do not cause
a real bug.  I keep the removal of an error message.  This error shows
up when a process dies before the OrangeFS userspace client responds.
This happens all the time on production systems and fills up the ring
buffer.

Signed-off-by: Martin Brandenburg <martin@omnibond.com>
---
 fs/orangefs/devorangefs-req.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/orangefs/devorangefs-req.c b/fs/orangefs/devorangefs-req.c
index 516ffb4dc9a0..f419dd999581 100644
--- a/fs/orangefs/devorangefs-req.c
+++ b/fs/orangefs/devorangefs-req.c
@@ -402,8 +402,9 @@ static ssize_t orangefs_devreq_write_iter(struct kiocb *iocb,
 	/* remove the op from the in progress hash table */
 	op = orangefs_devreq_remove_op(head.tag);
 	if (!op) {
-		gossip_err("WARNING: No one's waiting for tag %llu\n",
-			   llu(head.tag));
+		gossip_debug(GOSSIP_DEV_DEBUG,
+			     "%s: No one's waiting for tag %llu\n",
+			     __func__, llu(head.tag));
 		return ret;
 	}
 
-- 
2.11.0

[PATCH 3/3] orangefs: fix buffer size mis-match between kernel space and user space.

[Martin Brandenburg]

From: Mike Marshall <hubcap@omnibond.com>

commit eb68d0324dc4d88ab0d6159bdcd98c247a3a8954 upstream.

The deamon through which the kernel module communicates with the userspace
part of Orangefs, the "client-core", sends initialization data to the
kernel module with ioctl. The initialization data was built by the
client-core in a 2k buffer and copy_from_user'd into a 1k buffer
in the kernel module. When more than 1k of initialization data needed
to be sent, some was lost, reducing the usability of the control by which
debug levels are set. This patch sets the kernel side buffer to 2K to
match the userspace side...

Signed-off-by: Mike Marshall <hubcap@omnibond.com>
Signed-off-by: Martin Brandenburg <martin@omnibond.com>
---
 fs/orangefs/orangefs-dev-proto.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/orangefs/orangefs-dev-proto.h b/fs/orangefs/orangefs-dev-proto.h
index a3d84ffee905..f380f9ed1b28 100644
--- a/fs/orangefs/orangefs-dev-proto.h
+++ b/fs/orangefs/orangefs-dev-proto.h
@@ -50,8 +50,7 @@
  * Misc constants. Please retain them as multiples of 8!
  * Otherwise 32-64 bit interactions will be messed up :)
  */
-#define ORANGEFS_MAX_DEBUG_STRING_LEN	0x00000400
-#define ORANGEFS_MAX_DEBUG_ARRAY_LEN	0x00000800
+#define ORANGEFS_MAX_DEBUG_STRING_LEN	0x00000800
 
 /*
  * The maximum number of directory entries in a single request is 96.
-- 
2.11.0

Re: [PATCH 2/3] orangefs: Dan Carpenter influenced cleanups...

[Greg KH]

On Tue, Apr 11, 2017 at 12:41:27PM -0400, Martin Brandenburg wrote:
> From: Mike Marshall <hubcap@omnibond.com>
> 
> commit 05973c2efb40122f2a9ecde2d065f7ea5068d024 upstream.
> 
> This patch is simlar to one Dan Carpenter sent me, cleans
> up some return codes and whitespace errors. There was one
> place where he thought inserting an error message into
> the ring buffer might be too chatty, I hope I convinced him
> othewise. As a consolation <g> I changed a truly chatty
> error message in another location into a debug message,
> system-admins had already yelled at me about that one...
> 
> Signed-off-by: Mike Marshall <hubcap@omnibond.com>
> 
> I have removed the return code and whitespace fixes as they do not cause
> a real bug.  I keep the removal of an error message.  This error shows
> up when a process dies before the OrangeFS userspace client responds.
> This happens all the time on production systems and fills up the ring
> buffer.

No, please keep patches identical to what is in Linus's tree.  That way
you don't mess something up, and any future patches over the next 2+
years the kernel tree will be around apply cleanly.

Almost every single time we "modify" a patch from what is in Linus's
tree it is broken.

I've taken the original patch here for 4.9 and 4.10 stable trees.

thanks,

greg k-h

Re: [PATCH 0/3] orangefs fixes for stable

[Greg KH]

On Tue, Apr 11, 2017 at 12:41:25PM -0400, Martin Brandenburg wrote:
> Here are some patches for 4.9 and 4.10 which we missed before.
> 
> I've just looked through the changes to OrangeFS since 4.9 was released.
> These are the only fixes which seem to qualify.
> 
> Colin's patch is already in 4.10 and only needs to be applied to 4.9.
> Mike's patches should be applied to 4.9 and 4.10.
> 
> (Is there a better way to submit these?  Should I just submit separately
> for the two stable trees?)

This worked just fine.  Or you can just give me a list of git commit ids
as none of these needed to be modified from what was in Linus's tree
from what I could tell.

thanks,

greg k-h

Re: [PATCH 2/3] orangefs: Dan Carpenter influenced cleanups...

[Martin Brandenburg]

On Wed, 12 Apr 2017, Greg KH wrote:

> On Tue, Apr 11, 2017 at 12:41:27PM -0400, Martin Brandenburg wrote:
> > From: Mike Marshall <hubcap@omnibond.com>
> > 
> > commit 05973c2efb40122f2a9ecde2d065f7ea5068d024 upstream.
> > 
> > This patch is simlar to one Dan Carpenter sent me, cleans
> > up some return codes and whitespace errors. There was one
> > place where he thought inserting an error message into
> > the ring buffer might be too chatty, I hope I convinced him
> > othewise. As a consolation <g> I changed a truly chatty
> > error message in another location into a debug message,
> > system-admins had already yelled at me about that one...
> > 
> > Signed-off-by: Mike Marshall <hubcap@omnibond.com>
> > 
> > I have removed the return code and whitespace fixes as they do not cause
> > a real bug.  I keep the removal of an error message.  This error shows
> > up when a process dies before the OrangeFS userspace client responds.
> > This happens all the time on production systems and fills up the ring
> > buffer.
> 
> No, please keep patches identical to what is in Linus's tree.  That way
> you don't mess something up, and any future patches over the next 2+
> years the kernel tree will be around apply cleanly.
> 
> Almost every single time we "modify" a patch from what is in Linus's
> tree it is broken.
> 
> I've taken the original patch here for 4.9 and 4.10 stable trees.

I based that on this line in stable-kernel-rules.rst

 - It cannot contain any "trivial" fixes in it (spelling changes,
   whitespace cleanups, etc).

but I do see your point.  Does that just mean pure trivial patches are
inappropriate, but that if one or two trivial changes comes part and
parcel with a more significant patch it should be kept?

(Honestly what I removed are slightly more than trivial whitespace,
though I wouldn't have sent them on their own if they didn't come with
the logging change.)

Martin

> 
> thanks,
> 
> greg k-h
> 

Re: [PATCH 2/3] orangefs: Dan Carpenter influenced cleanups...

[Greg KH]

On Wed, Apr 12, 2017 at 11:27:25AM -0400, Martin Brandenburg wrote:
> On Wed, 12 Apr 2017, Greg KH wrote:
> 
> > On Tue, Apr 11, 2017 at 12:41:27PM -0400, Martin Brandenburg wrote:
> > > From: Mike Marshall <hubcap@omnibond.com>
> > > 
> > > commit 05973c2efb40122f2a9ecde2d065f7ea5068d024 upstream.
> > > 
> > > This patch is simlar to one Dan Carpenter sent me, cleans
> > > up some return codes and whitespace errors. There was one
> > > place where he thought inserting an error message into
> > > the ring buffer might be too chatty, I hope I convinced him
> > > othewise. As a consolation <g> I changed a truly chatty
> > > error message in another location into a debug message,
> > > system-admins had already yelled at me about that one...
> > > 
> > > Signed-off-by: Mike Marshall <hubcap@omnibond.com>
> > > 
> > > I have removed the return code and whitespace fixes as they do not cause
> > > a real bug.  I keep the removal of an error message.  This error shows
> > > up when a process dies before the OrangeFS userspace client responds.
> > > This happens all the time on production systems and fills up the ring
> > > buffer.
> > 
> > No, please keep patches identical to what is in Linus's tree.  That way
> > you don't mess something up, and any future patches over the next 2+
> > years the kernel tree will be around apply cleanly.
> > 
> > Almost every single time we "modify" a patch from what is in Linus's
> > tree it is broken.
> > 
> > I've taken the original patch here for 4.9 and 4.10 stable trees.
> 
> I based that on this line in stable-kernel-rules.rst
> 
>  - It cannot contain any "trivial" fixes in it (spelling changes,
>    whitespace cleanups, etc).
> 
> but I do see your point.  Does that just mean pure trivial patches are
> inappropriate, but that if one or two trivial changes comes part and
> parcel with a more significant patch it should be kept?

Yes.  You shouldn't be taking patches that do more than one thing in a
single patch anyway :)

thanks,

greg k-h