On Sun, Dec 13, 2020 at 01:08:55AM +0100, Andrew Lunn wrote:
> > > And you need some way to cleanup the allocated memory when the commit
> > > never happens because some other layer has said No!
> >
> > So this would be a fatal problem with the switchdev transactional model
> > if I am not misunderstanding it. On one hand there's this nice, bubbly
> > idea that you should preallocate memory in the prepare phase, so that
> > there's one reason less to fail at commit time. But on the other hand,
> > if "the commit phase might never happen" is even a remove possibility,
> > all of that goes to trash - how are you even supposed to free the
> > preallocated memory.
>
> It can definitely happen, that commit is never called:
>
> static int switchdev_port_obj_add_now(struct net_device *dev,
> const struct switchdev_obj *obj,
> struct netlink_ext_ack *extack)
> {
>
> /* 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
> * should reserve resources needed for the commit phase here,
> * but should not commit the obj.
> */
>
> trans.ph_prepare = true;
> err = switchdev_port_obj_notify(SWITCHDEV_PORT_OBJ_ADD,
> dev, obj, &trans, extack);
> if (err)
> return err;
>
> /* Phase II: commit obj add. This cannot fail as a fault
> * of driver/device. If it does, it's a bug in the driver/device
> * because the driver said everythings was OK in phase I.
> */
>
> trans.ph_prepare = false;
> err = switchdev_port_obj_notify(SWITCHDEV_PORT_OBJ_ADD,
> dev, obj, &trans, extack);
> WARN(err, "%s: Commit of object (id=%d) failed.\n", dev->name,
> obj->id);
>
> return err;
>
> So if any notifier returns an error during prepare, the commit is
> never called.
>
> So the memory you allocated and added to the list may never get
> used. Its refcount stays zero. Which is why i suggested making the
> MDB remove call do a general garbage collect. It is not perfect, the
> cleanup could be deferred a long time, but is should get removed
> eventually.
What would the garbage collection look like?
