On Tue, 11 Dec 2018 12:05:49 +1100 Andrew Donnellan <andrew.donnel...@au1.ibm.com> wrote:
> On 11/12/18 2:15 am, Greg Kurz wrote: > > The only users of free_spa() are alloc_link() and free_link(), and > > in both cases: > > > > - link->spa != NULL > > > > - free_spa(link) is immediatly followed by kfree(link) > > > > The check isn't needed, and it doesn't bring much to clear the link->spa > > pointer. Drop both. > > > > Signed-off-by: Greg Kurz <gr...@kaod.org> > > I like defensive programming but for this case I don't really care too > much either way > I now realize that I should have mentioned the real motivation for this change. I'm working on refactoring the code so that we can use ocxl in a KVM guest. The concept of link can be shared by both powernv and pseries variants but the SPA is definitely a powernv only thingy. The benefit of this patch is hence to kick 'struct link' out of free_spa() so that it can be utimately moved to powernv specific code. The initial version of this change was just moving the link->spa check and link->spa = NULL to the callers, where it was quite obvious they're not needed... Should I re-post this as two patches for clarity ? > Acked-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com> > > > --- > > drivers/misc/ocxl/link.c | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c > > index 31695a078485..eed92055184d 100644 > > --- a/drivers/misc/ocxl/link.c > > +++ b/drivers/misc/ocxl/link.c > > @@ -352,11 +352,8 @@ static void free_spa(struct link *link) > > pr_debug("Freeing SPA for %x:%x:%x\n", link->domain, link->bus, > > link->dev); > > > > - if (spa && spa->spa_mem) { > > - free_pages((unsigned long) spa->spa_mem, spa->spa_order); > > - kfree(spa); > > - link->spa = NULL; > > - } > > + free_pages((unsigned long) spa->spa_mem, spa->spa_order); > > + kfree(spa); > > } > > > > static int alloc_link(struct pci_dev *dev, int PE_mask, struct link > > **out_link) > > >