From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nikolay Aleksandrov Subject: Re: [patch net-next v2 4/7] switchdev: introduce possibility to defer obj_add/del Date: Mon, 12 Oct 2015 16:34:25 +0200 Message-ID: <561BC4F1.5080005@cumulusnetworks.com> References: <1444655710-8279-1-git-send-email-jiri@resnulli.us> <1444655710-8279-5-git-send-email-jiri@resnulli.us> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, idosch@mellanox.com, eladr@mellanox.com, sfeldma@gmail.com, f.fainelli@gmail.com, linux@roeck-us.net, vivien.didelot@savoirfairelinux.com, andrew@lunn.ch, john.fastabend@gmail.com, David.Laight@ACULAB.COM, stephen@networkplumber.org To: Jiri Pirko , netdev@vger.kernel.org Return-path: Received: from mail-wi0-f181.google.com ([209.85.212.181]:38553 "EHLO mail-wi0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751546AbbJLOe2 (ORCPT ); Mon, 12 Oct 2015 10:34:28 -0400 Received: by wieq12 with SMTP id q12so23384555wie.1 for ; Mon, 12 Oct 2015 07:34:27 -0700 (PDT) In-Reply-To: <1444655710-8279-5-git-send-email-jiri@resnulli.us> Sender: netdev-owner@vger.kernel.org List-ID: On 10/12/2015 03:15 PM, Jiri Pirko wrote: > From: Jiri Pirko > > Similar to the attr usecase, the caller knows if he is holding RTNL and is > in atomic section. So let the called to decide the correct call variant. > > This allows drivers to sleep inside their ops and wait for hw to get the > operation status. Then the status is propagated into switchdev core. > This avoids silent errors in drivers. > > Signed-off-by: Jiri Pirko > --- > include/net/switchdev.h | 1 + > net/switchdev/switchdev.c | 137 +++++++++++++++++++++++++++++++++++++--------- > 2 files changed, 112 insertions(+), 26 deletions(-) > [snip] > + > +struct switchdev_obj_work { > + struct work_struct work; > + struct net_device *dev; > + struct switchdev_obj obj; > + bool add; /* add of del */ s/of/or/ ? :-) > +}; > + > +static void switchdev_port_obj_work(struct work_struct *work) > +{ > + struct switchdev_obj_work *ow = > + container_of(work, struct switchdev_obj_work, work); > + bool rtnl_locked = rtnl_is_locked(); > + int err; > + > + if (!rtnl_locked) > + rtnl_lock(); > + if (ow->add) > + err = switchdev_port_obj_add_now(ow->dev, &ow->obj); > + else > + err = switchdev_port_obj_del_now(ow->dev, &ow->obj); > + if (err && err != -EOPNOTSUPP) > + netdev_err(ow->dev, "failed (err=%d) to %s object (id=%d)\n", > + err, ow->add ? "add" : "del", ow->obj.id); > + if (!rtnl_locked) > + rtnl_unlock(); > + > + dev_put(ow->dev); > + kfree(ow); > +} > + > +static int switchdev_port_obj_work_schedule(struct net_device *dev, > + const struct switchdev_obj *obj, > + bool add) > +{ > + struct switchdev_obj_work *ow; > + > + ow = kmalloc(sizeof(*ow), GFP_ATOMIC); > + if (!ow) > + return -ENOMEM; > + > + INIT_WORK(&ow->work, switchdev_port_obj_work); > + This can be called without rtnl, what stops the device from disappearing between the above and the hold below ? > + dev_hold(dev); > + ow->dev = dev; > + memcpy(&ow->obj, obj, sizeof(ow->obj)); > + ow->add = add; > + > + queue_work(switchdev_wq, &ow->work); > + return 0; > +} > + [snip]