From: Markus Elfring <Markus.Elfring@xxxxxx> Sent: Wednesday, January 10, 2024 9:08 AM > > > It occurred to me overnight that the existing error handling > > in create_gpadl_header() is unnecessarily complicated. Here's > > an approach that I think would fix what you have flagged, and > > would reduce complexity instead of increasing it. Thoughts? > > I find this development view interesting. > > > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c > > index 56f7e06c673e..44b1d5c8dfed 100644 > > --- a/drivers/hv/channel.c > > +++ b/drivers/hv/channel.c > > @@ -336,7 +336,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer, > > sizeof(struct gpa_range) + pfncount * sizeof(u64); > > msgheader = kzalloc(msgsize, GFP_KERNEL); > > if (!msgheader) > > - goto nomem; > > + return -ENOMEM; > > > > INIT_LIST_HEAD(&msgheader->submsglist); > > msgheader->msgsize = msgsize; > > @@ -386,8 +386,8 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer, > > list_del(&pos->msglistentry); > > kfree(pos); > > } > > - > > - goto nomem; > > + kfree(msgheader); > > + return -ENOMEM; > > } > > > > msgbody->msgsize = msgsize; > > @@ -416,8 +416,8 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer, > > sizeof(struct vmbus_channel_gpadl_header) + > > sizeof(struct gpa_range) + pagecount * sizeof(u64); > > msgheader = kzalloc(msgsize, GFP_KERNEL); > > - if (msgheader == NULL) > > - goto nomem; > > + if (!msgheader) > > + return -ENOMEM; > > > > INIT_LIST_HEAD(&msgheader->submsglist); > > msgheader->msgsize = msgsize; > > @@ -437,10 +437,6 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer, > > } > > > > return 0; > > -nomem: > > - kfree(msgheader); > > - kfree(msgbody); > > - return -ENOMEM; > > } > > > > /* > > > > Should up to two memory areas still be released after a data processing > failure? The function create_gpadl_header() is responsible only for allocating the memory and filling in various fields. It doesn't do any processing of the data and doesn't generate any errors other than memory allocation failures. If create_gpadl_header() succeeds, it returns a pointer to the allocated memory via the msginfo parameter, and the caller becomes responsible for free'ing the memory. The only caller is __vmbus_establish_gpadl(), which *does* free the memory after communicating with Hyper-V. Processing errors may occur when communicating with Hyper-V, but in a quick review, __vmbus_establish_gpadl() seems to handle those errors and to correctly free the memory. Michael
