Re: [PATCH rdma-next V2 5/5] IB/core: Integrate IB address resolution module into core

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/16/2016 02:54 PM, Leon Romanovsky wrote:
> On Mon, May 16, 2016 at 01:42:34PM -0400, Doug Ledford wrote:
>> On 05/16/2016 12:30 PM, Leon Romanovsky wrote:
>>> On Mon, May 16, 2016 at 11:09:44AM -0400, Doug Ledford wrote:
>>>> On 05/15/2016 06:51 AM, Mark Bloch wrote:
>>>>
>>>>>> But, just as importantly, after reading addr.c to see how it uses the ibnl
>>>>>> infrastructure, I don't even see what the original problem can be.
>>>>> As it stands today:
>>>>> - ibnl is part of ib_core.
>>>>> - ib_core needs ib_addr.
>>>>>
>>>>> So if we add ibnl usage to ib_addr it means ib_addr will need ib_core,
>>>>> which causes a dependency cycle.
>>>>
>>>> Right, but in that case, this patch needs to be part of the series that
>>>> adds the ibnl support into the ib_addr functionality.  Because you split
>>>> them into separate series, this was a patch looking for a problem to
>>>> solve and it wasn't clear what it was.  If I had taken the other series
>>>> and not this series, it would have broken things.  So please keep
>>>> patches like this together with the other patches that depend on it.
>>>>
>>>> That said, I also don't want to redo modules if we don't have to.  As my
>>>> previous email points out, changing modules breaks init scripts and
>>>> systemd unit files.  It is to be avoided when possible.
>>>
>>> Sorry,
>>> I was in the mood of fixing things when I wrote and sent this patch.
>>> The question is which version will you more likely to accept: this one
>>> (remove ib_addr module) or previous one (add ib_netlink module)?
>>
>> Can you build netlink in and then init the ib_addr module after the
>> netlink init is complete?  Wouldn't that resolve the dependency ordering
>> issue without changing the module names?
> 
> It seems reasonable and we will test it, before reposting.

Thanks.

> However generally speaking, I agree with Jason and Ira that this module
> (ib_addr.ko) is useless as module and can be part of ib_core.ko.
> 
> It doesn't seem as a big deal to fix all that init scripts (remove two
> lines).

It's not removing two lines.  If someone wants to be able to boot both
kernels prior to this change and kernel post this change, they have to
now write their script to handle both conditions sanely.  And they have
to know to do so, where as very often they simply get caught off guard
and things are broken for some period of time.  If the change to ib_addr
actually solved a problem that couldn't be solved otherwise, that would
be one thing.  But it doesn't.  It's a complete no-op.


-- 
Doug Ledford <dledford@xxxxxxxxxx>
              GPG KeyID: 0E572FDD


Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux