Orangefs: de-uglify orangefs_devreq_writev, and devorangefs-req.c in general
authorMike Marshall <hubcap@omnibond.com>
Fri, 11 Dec 2015 21:45:03 +0000 (16:45 -0500)
committerMike Marshall <hubcap@omnibond.com>
Mon, 14 Dec 2015 18:32:05 +0000 (13:32 -0500)
commit97f100277cfdcd268f0cf3d83bb6e4d1a345bc80
tree20b043dcf2ec4432e31b5ed92b68f2c470a80400
parentb4cf67a2ba1a58dbd2a967c3d877b807fef83b25
Orangefs: de-uglify orangefs_devreq_writev, and devorangefs-req.c in general

AV dislikes many parts of orangefs_devreq_writev. Besides making
orangefs_devreq_writev more easily readable and better commented,
this patch makes an effort to address some of the problems:

 > The 5th is quietly ignored unless trailer_size is positive and
 > status is zero. If trailer_size > 0 && status == 0, you verify that
 > the length of the 5th segment is no more than trailer_size and copy
 > it to vmalloc'ed buffer. Without bothering to zero the rest of that
 > buffer out.

It was just wrong to allow a 5th segment that is not exactly equal to
trailer_size. Now that that's fixed, there's nothing to zero out in
the vmalloced buffer - it is exactly the right size to hold the
5th segment.

 > Another API bogosity: when the 5th segment is present, successful writev()
 > returns the sum of sizes of the first 4.

Added size of 5th segment to writev return...

 > if concatenation of the first 4 segments is longer than
 > 16 + sizeof(struct pvfs2_downcall_s) by no more than sizeof(long) => whine
 > and proceed with garbage.

If 4th segment isn't exactly sizeof(struct pvfs2_downcall_s), whine and fail.

 > if the 32bit value 4 bytes into op->downcall is zero and 64bit
 > value following it is non-zero, the latter is interpreted as the size of
 > trailer data.

The latter is what userspace claimed was the length of the trailer data.
The kernel module now compares it to the trailer iovec's iov_len as a
sanity check.

 > if there's no trailer, the 5th segment (if present) is completely ignored.

Whine and fail if there should be no trailer, yet a 5th segment is present.

 > if vmalloc fails, act as if status (32bit at offset 5 into
 > op->downcall) had been -ENOMEM and don't look at the 5th segment at all.

whine and fail with -ENOMEM.

Signed-off-by: Mike Marshall <hubcap@omnibond.com>
fs/orangefs/devorangefs-req.c