Re: [PATCH 0/3] rbd: header read/refresh improvements

[Alex Elder <elder@ieee.org>]

On 04/24/2015 09:40 AM, Douglas Fuller wrote:
>
>> On Apr 24, 2015, at 10:17 AM, Ilya Dryomov <idryomov@gmail.com>
>> wrote:
>>
>> On Fri, Apr 24, 2015 at 4:11 PM, Alex Elder <elder@ieee.org>
>> wrote:
>>> On 04/23/2015 02:06 PM, Douglas Fuller wrote:
>>>>
>>>> Support multiple class op calls in one ceph_msg and consolidate
>>>> rbd header read and refresh processes to use this feature to
>>>> reduce the number of ceph_msgs sent for that process. Refresh
>>>> features on header refresh and begin returning EIO if features
>>>> have changed since mapping.
>>>>
>>>> Douglas Fuller (3): ceph: support multiple class method calls
>>>> in one ceph_msg rbd: combine object method calls in header
>>>> refresh using fewer ceph_msgs rbd: re-read features during
>>>> header refresh and detect changes.
>>>>
>>>> drivers/block/rbd.c             | 518
>>>> +++++++++++++++++++++++++++++-----------
>>>> include/linux/ceph/osd_client.h |   3 +- net/ceph/messenger.c
>>>> |   4 + net/ceph/osd_client.c           |  92 ++++++- 4 files
>>>> changed, 470 insertions(+), 147 deletions(-)
>>>>
>>>
>>> In case Ilya or others don't get to it soon, I plan to review
>>> this series tomorrow.
>>
>> I was planning take a look while I'm the road during the weekend.
>>
>> Doug, from a quick look this revision still has a bunch of style
>> issues, most notably the alignment of function parameters and
>> braces around if / else.  See Documentation/CodingStyle in the
>> kernel tree for examples.
>
> I needed to put out v2 in part because I squashed a couple fixup
> commits in the wrong place, leaving some things behind in #2 that
> were corrected in #3.
>
> I changed the braces in that version, but the function parameter
> indents are inconsistent throughout the code. I’ll try to come up
> with a compromise.

When in doubt, lean toward the style used in the rest of
the kernel.  I used a few conventions that are not consistent
with that in a lot of places, and those can be gradually
phased toward what's recommended for the kernel.  Some
examples are:
     sizeof x or sizeof (x)  -->  sizeof(x)
     (cast) foo  --> (cast)foo
     White space under comment blocks
     static int\nfunction(...)   -> static int function(...)

					-Alex

>>
>> You might also want to run your patches through
>> scripts/checkpatch.pl, but take it with a grain of salt - it can be
>> a bit too extreme at times.  No need to post v3 with just style
>> fixes, wait for more feedback.
>
> Thanks again for all feedback.
>
>>
>> Thanks,
>>
>> Ilya
>

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html