From patchwork Tue May 17 15:48:48 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Ledford X-Patchwork-Id: 9113971 Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id CE8EF9F30C for ; Tue, 17 May 2016 15:48:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id CAEB220268 for ; Tue, 17 May 2016 15:48:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AD58E2025A for ; Tue, 17 May 2016 15:48:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755560AbcEQPsz (ORCPT ); Tue, 17 May 2016 11:48:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33106 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753986AbcEQPsz (ORCPT ); Tue, 17 May 2016 11:48:55 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6D685C05B1E6; Tue, 17 May 2016 15:48:49 +0000 (UTC) Received: from linux-ws.xsintricity.com (ovpn-116-52.rdu2.redhat.com [10.10.116.52]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4HFmmCg024576 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 17 May 2016 11:48:49 -0400 Subject: Re: [PATCH rdma-next V2 5/5] IB/core: Integrate IB address resolution module into core To: Mark Bloch , "leon@kernel.org" References: <1462563928-29164-1-git-send-email-leon@kernel.org> <1462563928-29164-6-git-send-email-leon@kernel.org> <20160516163048.GB4662@leon.nu> <298657b0-6e57-745b-5eb3-001984bffbc3@redhat.com> Cc: "linux-rdma@vger.kernel.org" From: Doug Ledford Openpgp: id=AE6B1BDA122B23B4265B1274B826A3330E572FDD; url=pgp.mit.edu X-Enigmail-Draft-Status: N1110 Organization: Red Hat, Inc. Message-ID: Date: Tue, 17 May 2016 11:48:48 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Tue, 17 May 2016 15:48:49 +0000 (UTC) Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 05/17/2016 04:29 AM, Mark Bloch wrote: > > >> -----Original Message----- >> From: Doug Ledford [mailto:dledford@redhat.com] >> Sent: Monday, May 16, 2016 8:43 PM >> To: leon@kernel.org >> Cc: Mark Bloch ; linux-rdma@vger.kernel.org >> Subject: Re: [PATCH rdma-next V2 5/5] IB/core: Integrate IB address >> resolution module into core >> 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? > Sorry, but I don't understand what do you mean by this. > If you would like to keep the current module structure (ib_core.ko and ib_addr.ko) > The only way to use ibnl from within addr.c is to move the ibnl initialization to addr.c > and build netlink.c as part of ib_addr.ko, but it seems kind of strange if > ib_addr is responsible to initialize ibnl. Not according to what I was looking at. In the original patch, you moved addr.c into ib_core and changed addr_init and addr_cleanup from static to normal. You were then able to control whether the addr code went before or after the netlink code by rearranging the order of init sequences in device.c:ib_core_init(). It is entirely possible that you could leave ib_addr as a module, change addr_init to rdma_addr_init and EXPORT_SYMBOL it, and likewise with the cleanup function, and remove the module_init(addr_init); and module_exit(addr_cleanup); from the addr.c file. This would then allow you to control when ib_addr was inited by adding a call to rdma_addr_init() in device.c:ib_core_init() just like you had in your patch. It would solve the problem and give you complete control while being transparent to user space. This is what I mean: commit ec5394412286e325b83b6c64d026f4d7bab40ccd Author: Doug Ledford Date: Tue May 17 11:43:57 2016 -0400 IB/core: change module init ordering Change ib_addr to be init'ed by ib_core and not by the module load process. This gives us precise controle of when the module is initialized while still preserving the existing module names and load ordering. This is to be backward compatible with existing, older SysV init scripts. When those older systems are dead and gone, we can revisit this and change the module names and load orders however we want. Signed-off-by: Doug Ledford diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c index 337353d86cfa..fda808546e0e 100644 --- a/drivers/infiniband/core/addr.c +++ b/drivers/infiniband/core/addr.c @@ -634,7 +634,17 @@ static struct notifier_block nb = { .notifier_call = netevent_callback }; -static int __init addr_init(void) +/* + * This is a little unorthodox. We export our init function so that the + * ib_core module can call it at the right time after the other modules + * we depend on have registered. We do this solely because there are some + * user space init scripts that expect ib_addr to be loaded prior to ib_core + * and due to recent changes, we need the ib netlink code brought up before + * we bring this code up and that code is now in ib_core. When the older + * SysV init script systems that have these init scripts are dead and gone, + * we can revisit this and possibly just include addr.c in ib_core. + */ +int rdma_addr_init(void) { addr_wq = create_singlethread_workqueue("ib_addr"); if (!addr_wq) @@ -644,13 +654,12 @@ static int __init addr_init(void) rdma_addr_register_client(&self); return 0; } +EXPORT_SYMBOL(rdma_addr_init); -static void __exit addr_cleanup(void) +void rdma_addr_cleanup(void) { rdma_addr_unregister_client(&self); unregister_netevent_notifier(&nb); destroy_workqueue(addr_wq); } - -module_init(addr_init); -module_exit(addr_cleanup); +EXPORT_SYMBOL(rdma_addr_cleanup); diff --git a/drivers/infiniband/core/core_priv.h b/drivers/infiniband/core/core_priv.h index eab32215756b..e0a5187ea98c 100644 --- a/drivers/infiniband/core/core_priv.h +++ b/drivers/infiniband/core/core_priv.h @@ -137,4 +137,7 @@ static inline bool rdma_is_upper_dev_rcu(struct net_device *dev, return _upper == upper; } +int rdma_addr_init(void); +void rdma_addr_cleanup(void); + #endif /* _CORE_PRIV_H */ diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c index 10979844026a..4fdf87a485cc 100644 --- a/drivers/infiniband/core/device.c +++ b/drivers/infiniband/core/device.c @@ -983,10 +983,18 @@ static int __init ib_core_init(void) goto err_sysfs; } + ret = rdma_addr_init(); + if (ret) { + pr_warn("Couldn't init IB address resolution module\n"); + goto err_ibnl; + } + ib_cache_setup(); return 0; +err_ibnl: + ibnl_cleanup(); err_sysfs: class_unregister(&ib_class); err_comp: @@ -999,6 +1007,7 @@ err: static void __exit ib_core_cleanup(void) { ib_cache_cleanup(); + rdma_addr_cleanup(); ibnl_cleanup(); class_unregister(&ib_class); destroy_workqueue(ib_comp_wq);