On Wed, Sep 14, 2016 at 4:33 AM, Jamal Hadi Salim <j...@mojatatu.com> wrote: > On 16-09-13 03:47 PM, Jamal Hadi Salim wrote: >> >> On 16-09-13 12:20 PM, Cong Wang wrote: >>> >>> On Mon, Sep 12, 2016 at 4:07 PM, Jamal Hadi Salim <j...@mojatatu.com> >>> wrote: > > > [..] >>> >>> I am still trying to understand this piece, so here you hold the refcnt >>> for the same action used by the later iteration? Otherwise there is >>> almost none user inbetween hold and release... >>> >>> The comment you add is not clear to me, we use RTNL/RCU to >>> sync destroy and replace, so how could that happen? >>> >> >> I was worried about the destroy() hitting an error in that function. >> If an action already existed and all we asked for was to >> replace some attribute it would be deleted. It was the way the code was >> before your changes so i just restored it to its original form. >> > > And I have verified this is needed. I went and made gact return > a failure if you replace something. I added a gact action; then > when i replaced it failed. And when it failed it replace the existing > action.
Oh, I see, the comments actually mislead me. ;) You are trying to fix the rollback for failure path here... Makes sense to me now. Thanks for verifying it.