On 6/3/21 3:34 AM, lizhij...@fujitsu.com wrote:
>
>
> On 03/06/2021 01.51, Philippe Mathieu-Daudé wrote:
>> Since 00f2cfbbec6 ("glib: bump min required glib library version to
>> 2.48") we can use g_auto/g_autoptr to have the compiler automatically
>> free an allocated variable when it goes out of scope,
> Glad to know this feature.
>
> However per its code, a 'ack' does much more than just free the memory.
> not sure g_autoptr have the ability to do the same.
See
https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#G-DEFINE-AUTOPTR-CLEANUP-FUNC:CAPS
Defines the appropriate cleanup function for a pointer type.
The function will not be called if the variable to be cleaned
up contains NULL.
This will typically be the _free() or _unref() function for
the given type.
This does not change the code to call free(ptr), but to call the
registered cleanup function, which is rdma_ack_cm_event().
>
> 2212 static void ucma_complete_event(struct cma_id_private *id_priv)
> 2213 {
> 2214 pthread_mutex_lock(&id_priv->mut);
> 2215 id_priv->events_completed++;
> 2216 pthread_cond_signal(&id_priv->cond);
> 2217 pthread_mutex_unlock(&id_priv->mut);
> 2218 }
> 2219
> 2220 static void ucma_complete_mc_event(struct cma_multicast *mc)
> 2221 {
> 2222 pthread_mutex_lock(&mc->id_priv->mut);
> 2223 mc->events_completed++;
> 2224 pthread_cond_signal(&mc->cond);
> 2225 mc->id_priv->events_completed++;
> 2226 pthread_cond_signal(&mc->id_priv->cond);
> 2227 pthread_mutex_unlock(&mc->id_priv->mut);
> 2228 }
> 2229
> 2230 int rdma_ack_cm_event(struct rdma_cm_event *event)
> 2231 {
> 2232 struct cma_event *evt;
> 2233
> 2234 if (!event)
> 2235 return ERR(EINVAL);
> 2236
> 2237 evt = container_of(event, struct cma_event, event);
> 2238
> 2239 if (evt->mc)
> 2240 ucma_complete_mc_event(evt->mc);
> 2241 else
> 2242 ucma_complete_event(evt->id_priv);
> 2243 free(evt);
> 2244 return 0;
> 2245 }
>
> Thanks
> Zhijian
>
>> removing this
>> burden on the developers.
>>
>> Per rdma_cm(7) and rdma_ack_cm_event(3) man pages:
>>
>> "rdma_ack_cm_event() - Free a communication event.
>>
>> All events which are allocated by rdma_get_cm_event() must be
>> released, there should be a one-to-one correspondence between
>> successful gets and acks. This call frees the event structure
>> and any memory that it references."
>>
>> Since the 'ack' description doesn't explicit the event is also
>> released (free'd), it is safer to use the GLib g_autoptr feature.
>> The G_DEFINE_AUTOPTR_CLEANUP_FUNC() macro expects a single word
>> for the type name, so add a type definition to achieve this.
>> Convert to use g_autoptr and remove the rdma_ack_cm_event() calls.
>>
>> Inspired-by: Li Zhijian <lizhij...@cn.fujitsu.com>
>> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>> ---
>> RFC: build-tested only
>> ---
>> migration/rdma.c | 27 ++++++++++-----------------
>> 1 file changed, 10 insertions(+), 17 deletions(-)
>>
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index b50ebb9183a..b703bf1b918 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -38,6 +38,9 @@
>> #include "qom/object.h"
>> #include <poll.h>
>>
>> +typedef struct rdma_cm_event rdma_cm_event;
>> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(rdma_cm_event, rdma_ack_cm_event)
>> +
>> /*
>> * Print and error on both the Monitor and the Log file.
>> */
>> @@ -939,7 +942,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma,
>> Error **errp)
>> int ret;
>> struct rdma_addrinfo *res;
>> char port_str[16];
>> - struct rdma_cm_event *cm_event;
>> + g_autoptr(rdma_cm_event) cm_event = NULL;
>> char ip[40] = "unknown";
>> struct rdma_addrinfo *e;
>>
>> @@ -1007,11 +1010,11 @@ route:
>> ERROR(errp, "result not equal to event_addr_resolved %s",
>> rdma_event_str(cm_event->event));
>> perror("rdma_resolve_addr");
>> - rdma_ack_cm_event(cm_event);
>> ret = -EINVAL;
>> goto err_resolve_get_addr;
>> }
