2009-11-30 05:44:09

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: manual merge of the sysctl tree with the net tree

Hi Eric,

Today's linux-next merge of the sysctl tree got a conflict in
net/sctp/sysctl.c between commit 90f2f5318b3a5b0898fef0fec9b91376c7de7a2c
("sctp: Update SWS avaoidance receiver side algorithm") from the net tree
and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove
unused binary sysctl code") from the sysctl tree.

I fixed it up (see below) and can carry the fix as necessary. I also
removed the strategy member from the new added ctl_table entry.
--
Cheers,
Stephen Rothwell [email protected]

diff --cc net/sctp/sysctl.c
index ae03ded,d50a042..0000000
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] =
.extra1 = &zero,
.extra2 = &addr_scope_max,
},
+ {
- .ctl_name = CTL_UNNUMBERED,
+ .procname = "rwnd_update_shift",
+ .data = &sctp_rwnd_upd_shift,
+ .maxlen = sizeof(int),
+ .mode = 0644,
- .proc_handler = &proc_dointvec_minmax,
- .strategy = &sysctl_intvec,
++ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &one,
+ .extra2 = &rwnd_scale_max,
+ },
+
- { .ctl_name = 0 }
+ { }
};

static struct ctl_path sctp_path[] = {


2009-11-30 05:50:59

by Cong Wang

[permalink] [raw]
Subject: Re: linux-next: manual merge of the sysctl tree with the net tree

On Mon, Nov 30, 2009 at 1:44 PM, Stephen Rothwell <[email protected]> wrote:
> Hi Eric,
>
> Today's linux-next merge of the sysctl tree got a conflict in
> net/sctp/sysctl.c between commit 90f2f5318b3a5b0898fef0fec9b91376c7de7a2c
> ("sctp: Update SWS avaoidance receiver side algorithm") from the net tree
> and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove
> unused binary sysctl code") from the sysctl tree.
>
> I fixed it up (see below) and can carry the fix as necessary.  I also
> removed the strategy member from the new added ctl_table entry.
> --
> Cheers,
> Stephen Rothwell                    [email protected]
>
> diff --cc net/sctp/sysctl.c
> index ae03ded,d50a042..0000000
> --- a/net/sctp/sysctl.c
> +++ b/net/sctp/sysctl.c
> @@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] =
>                .extra1         = &zero,
>                .extra2         = &addr_scope_max,
>        },
>  +      {
> -               .ctl_name       = CTL_UNNUMBERED,
>  +              .procname       = "rwnd_update_shift",
>  +              .data           = &sctp_rwnd_upd_shift,
>  +              .maxlen         = sizeof(int),
>  +              .mode           = 0644,
> -               .proc_handler   = &proc_dointvec_minmax,
> -               .strategy       = &sysctl_intvec,
> ++              .proc_handler   = proc_dointvec_minmax,

Hey, what's this??

2009-11-30 06:39:14

by Eric W. Biederman

[permalink] [raw]
Subject: Re: linux-next: manual merge of the sysctl tree with the net tree

Américo Wang <[email protected]> writes:

> On Mon, Nov 30, 2009 at 1:44 PM, Stephen Rothwell <[email protected]> wrote:
>> Hi Eric,
>>
>> Today's linux-next merge of the sysctl tree got a conflict in
>> net/sctp/sysctl.c between commit 90f2f5318b3a5b0898fef0fec9b91376c7de7a2c
>> ("sctp: Update SWS avaoidance receiver side algorithm") from the net tree
>> and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove
>> unused binary sysctl code") from the sysctl tree.
>>
>> I fixed it up (see below) and can carry the fix as necessary.  I also
>> removed the strategy member from the new added ctl_table entry.
>> --
>> Cheers,
>> Stephen Rothwell                    [email protected]
>>
>> diff --cc net/sctp/sysctl.c
>> index ae03ded,d50a042..0000000
>> --- a/net/sctp/sysctl.c
>> +++ b/net/sctp/sysctl.c
>> @@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] =
>>                .extra1         = &zero,
>>                .extra2         = &addr_scope_max,
>>        },
>>  +      {
>> -               .ctl_name       = CTL_UNNUMBERED,
>>  +              .procname       = "rwnd_update_shift",
>>  +              .data           = &sctp_rwnd_upd_shift,
>>  +              .maxlen         = sizeof(int),
>>  +              .mode           = 0644,
>> -               .proc_handler   = &proc_dointvec_minmax,
>> -               .strategy       = &sysctl_intvec,
>> ++              .proc_handler   = proc_dointvec_minmax,
>
> Hey, what's this??

The short version is I am running a git tree that holds all of
the necessary cleanups to remove the support for binary sysctl handlers.

The binary sysctl support continues to be provided in kernel/sysctl_binary.c
with a compatibility wrapper. This has been reviewed on linux-kernel
and written up in lwn.

In my tree .ctl_name and .strategy have been removed as they exist
only to support binary sysctls and are not strictly needed today.
.ctl_name = CTL_UNNUMBERED is equivalent to .ctl_name = 0, and setting
.strategy on new sysctl table entries without a ctl_name is a harmless
bug. Since I was in there I also removed all of the unnecessary ampersands
from in front of proc_dointvec_minmax.

Since I have touched practically every sysctl table entry in the kernel
new sysctl additions will almost inevitably cause a small by trivially
to resolve conflict (due to the fact I have almost certainly changed
the proceeding and succeeding sysctl table entries).

Currently this only the second sysctl added this kernel cycle, and it
looks like this work happened in parallel, with my changes, and somehow
David missed this commit in his September pull, so the changes just
showed up in net-next.

It would seem to require talent to mess up the merge conflicts, and
getting it wrong will result in a tree that won't compile so I am not
going to worry about it until Linux pulls one of our trees.

Eric

2009-11-30 06:41:01

by Stephen Rothwell

[permalink] [raw]
Subject: Re: linux-next: manual merge of the sysctl tree with the net tree

Hi Américo,

On Mon, 30 Nov 2009 13:51:03 +0800 Américo Wang <[email protected]> wrote:
>
> > diff --cc net/sctp/sysctl.c
> > index ae03ded,d50a042..0000000
> > --- a/net/sctp/sysctl.c
> > +++ b/net/sctp/sysctl.c
> > @@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] =
> >                .extra1         = &zero,
> >                .extra2         = &addr_scope_max,
> >        },
> >  +      {
> > -               .ctl_name       = CTL_UNNUMBERED,
> >  +              .procname       = "rwnd_update_shift",
> >  +              .data           = &sctp_rwnd_upd_shift,
> >  +              .maxlen         = sizeof(int),
> >  +              .mode           = 0644,
> > -               .proc_handler   = &proc_dointvec_minmax,
> > -               .strategy       = &sysctl_intvec,
> > ++              .proc_handler   = proc_dointvec_minmax,
>
> Hey, what's this??

If you mean "what is this strange looking patch", then it is the output
from "git diff --cc" after a merge conflict has been fixed up, but before
it is committed. The '-' lines were added by one side of the merge or
the other, but removed from the final result. The line with '++' did not
appear in either branch, but is in the final result. The lines with a
single '+' appear in one of the branches or the other and in the final
result as well.

--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/


Attachments:
(No filename) (1.52 kB)
(No filename) (198.00 B)
Download all attachments

2009-11-30 09:21:39

by Cong Wang

[permalink] [raw]
Subject: Re: linux-next: manual merge of the sysctl tree with the net tree

On Mon, Nov 30, 2009 at 2:41 PM, Stephen Rothwell <[email protected]> wrote:
> Hi Américo,
>
> On Mon, 30 Nov 2009 13:51:03 +0800 Américo Wang <[email protected]> wrote:
>>
>> > diff --cc net/sctp/sysctl.c
>> > index ae03ded,d50a042..0000000
>> > --- a/net/sctp/sysctl.c
>> > +++ b/net/sctp/sysctl.c
>> > @@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] =
>> >                .extra1         = &zero,
>> >                .extra2         = &addr_scope_max,
>> >        },
>> >  +      {
>> > -               .ctl_name       = CTL_UNNUMBERED,
>> >  +              .procname       = "rwnd_update_shift",
>> >  +              .data           = &sctp_rwnd_upd_shift,
>> >  +              .maxlen         = sizeof(int),
>> >  +              .mode           = 0644,
>> > -               .proc_handler   = &proc_dointvec_minmax,
>> > -               .strategy       = &sysctl_intvec,
>> > ++              .proc_handler   = proc_dointvec_minmax,
>>
>> Hey, what's this??
>
> If you mean "what is this strange looking patch", then it is the output
> from "git diff --cc" after a merge conflict has been fixed up, but before
> it is committed.  The '-' lines were added by one side of the merge or
> the other, but removed from the final result.  The line with '++' did not
> appear in either branch, but is in the final result.  The lines with a
> single '+' appear in one of the branches or the other and in the final
> result as well.
>

Thanks for teaching this! I thought it was a mistake, it is not. :)
No problem then.

2009-11-30 09:26:10

by Cong Wang

[permalink] [raw]
Subject: Re: linux-next: manual merge of the sysctl tree with the net tree

On Mon, Nov 30, 2009 at 2:39 PM, Eric W. Biederman
<[email protected]> wrote:
> Américo Wang <[email protected]> writes:
>
>> On Mon, Nov 30, 2009 at 1:44 PM, Stephen Rothwell <[email protected]> wrote:
>>> Hi Eric,
>>>
>>> Today's linux-next merge of the sysctl tree got a conflict in
>>> net/sctp/sysctl.c between commit 90f2f5318b3a5b0898fef0fec9b91376c7de7a2c
>>> ("sctp: Update SWS avaoidance receiver side algorithm") from the net tree
>>> and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove
>>> unused binary sysctl code") from the sysctl tree.
>>>
>>> I fixed it up (see below) and can carry the fix as necessary.  I also
>>> removed the strategy member from the new added ctl_table entry.
>>> --
>>> Cheers,
>>> Stephen Rothwell                    [email protected]
>>>
>>> diff --cc net/sctp/sysctl.c
>>> index ae03ded,d50a042..0000000
>>> --- a/net/sctp/sysctl.c
>>> +++ b/net/sctp/sysctl.c
>>> @@@ -285,19 -241,7 +242,17 @@@ static ctl_table sctp_table[] =
>>>                .extra1         = &zero,
>>>                .extra2         = &addr_scope_max,
>>>        },
>>>  +      {
>>> -               .ctl_name       = CTL_UNNUMBERED,
>>>  +              .procname       = "rwnd_update_shift",
>>>  +              .data           = &sctp_rwnd_upd_shift,
>>>  +              .maxlen         = sizeof(int),
>>>  +              .mode           = 0644,
>>> -               .proc_handler   = &proc_dointvec_minmax,
>>> -               .strategy       = &sysctl_intvec,
>>> ++              .proc_handler   = proc_dointvec_minmax,
>>
>> Hey, what's this??
>
> The short version is I am running a git tree that holds all of
> the necessary cleanups to remove the support for binary sysctl handlers.
>
> The binary sysctl support continues to be provided in kernel/sysctl_binary.c
> with a compatibility wrapper.  This has been reviewed on linux-kernel
> and written up in lwn.

Yeah, I saw your patches, but didn't have a chance to look at them closely.

>
> In my tree .ctl_name and .strategy have been removed as they exist
> only to support binary sysctls and are not strictly needed today.
> .ctl_name = CTL_UNNUMBERED is equivalent to .ctl_name = 0, and setting
> .strategy on new sysctl table entries without a ctl_name is a harmless
> bug.  Since I was in there I also removed all of the unnecessary ampersands
> from in front of proc_dointvec_minmax.
>
> Since I have touched practically every sysctl table entry in the kernel
> new sysctl additions will almost inevitably cause a small by trivially
> to resolve conflict (due to the fact I have almost certainly changed
> the proceeding and succeeding sysctl table entries).
>
> Currently this only the second sysctl added this kernel cycle, and it
> looks like this work happened in parallel, with my changes, and somehow
> David missed this commit in his September pull, so the changes just
> showed up in net-next.
>
> It would seem to require talent to mess up the merge conflicts, and
> getting it wrong will result in a tree that won't compile so I am not
> going to worry about it until Linux pulls one of our trees.

Thanks for this explanation, I see...

2009-12-01 14:13:29

by Vlad Yasevich

[permalink] [raw]
Subject: Re: linux-next: manual merge of the sysctl tree with the net tree


Stephen Rothwell wrote:
> Hi Eric,
>
> Today's linux-next merge of the sysctl tree got a conflict in
> net/sctp/sysctl.c between commit 90f2f5318b3a5b0898fef0fec9b91376c7de7a2c
> ("sctp: Update SWS avaoidance receiver side algorithm") from the net tree
> and commit f8572d8f2a2ba75408b97dc24ef47c83671795d7 ("sysctl net: Remove
> unused binary sysctl code") from the sysctl tree.
>
> I fixed it up (see below) and can carry the fix as necessary. I also
> removed the strategy member from the new added ctl_table entry.

Thanks for fixing this up Stephen. I was basing my work on linux-next
which, it appears, didn't have the sysctl changes.

-vlad