Wed, Sep 23, 2015 at 05:46:44PM CEST, [email protected] wrote: >On Wed, Sep 23, 2015 at 12:57 AM, Jiri Pirko <[email protected]> wrote: >> From: Jiri Pirko <[email protected]> >> >> Now, the memory allocation in prepare/commit state is done separatelly >> in each driver (rocker). Introduce the similar mechanism in generic >> switchdev code, in form of queue. That can be used not only for memory >> allocations, but also for different items. Commit/abort item destruction >> is handled as well. >> >> Signed-off-by: Jiri Pirko <[email protected]> >> --- >> v1-v2: >> - added comment blocks to exported functions as suggested by Scott >> - made switchdev_trans_init and switchdev_trans_items_destroy static >> - added documentation for this to switchdev.txt as suggested by Scott > >[cut] > >> @@ -232,10 +304,13 @@ static int __switchdev_port_obj_add(struct net_device >> *dev, >> */ >> int switchdev_port_obj_add(struct net_device *dev, struct switchdev_obj >> *obj) >> { >> + struct switchdev_trans trans; >> int err; >> >> ASSERT_RTNL(); >> >> + switchdev_trans_init(&trans); >> + >> /* Phase I: prepare for obj add. Driver/device should fail >> * here if there are going to be issues in the commit phase, >> * such as lack of resources or support. The driver/device >> @@ -244,7 +319,7 @@ int switchdev_port_obj_add(struct net_device *dev, >> struct switchdev_obj *obj) >> */ >> >> obj->trans_ph = SWITCHDEV_TRANS_PREPARE; >> - err = __switchdev_port_obj_add(dev, obj); >> + err = __switchdev_port_obj_add(dev, obj, &trans); >> if (err) { >> /* Prepare phase failed: abort the transaction. Any >> * resources reserved in the prepare phase are >> @@ -253,7 +328,8 @@ int switchdev_port_obj_add(struct net_device *dev, >> struct switchdev_obj *obj) >> >> if (err != -EOPNOTSUPP) { >> obj->trans_ph = SWITCHDEV_TRANS_ABORT; >> - __switchdev_port_obj_add(dev, obj); >> + __switchdev_port_obj_add(dev, obj, &trans); >> + switchdev_trans_items_destroy(&trans); >> } >> >> return err; >> @@ -265,8 +341,9 @@ int switchdev_port_obj_add(struct net_device *dev, >> struct switchdev_obj *obj) >> */ >> >> obj->trans_ph = SWITCHDEV_TRANS_COMMIT; >> - err = __switchdev_port_obj_add(dev, obj); >> + err = __switchdev_port_obj_add(dev, obj, &trans); >> WARN(err, "%s: Commit of object (id=%d) failed.\n", dev->name, >> obj->id); >> + switchdev_trans_items_destroy(&trans); > >Before this patchset, rocker would BUG_ON that the list was empty >after commit. This was really helpful in finding bugs in the driver >as it meant the commit and prepare phases didn't follow the same code >path. (Everything the driver queued in prepare was dequeued in >commit, so the list should be empty after commit). > >Should this last switchdev_trans_items_destroy(&trans) be replaced with: > > BUG_ON(!list_empty(&trans.item_list));
Sounds reasonable. Will change this. Thanks! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
