On Tue, 30 Mar 2021 at 17:08, Alexei Starovoitov
[off-list ref] wrote:

On Tue, Mar 30, 2021 at 5:06 AM Ciara Loftus [off-list ref] wrote:

quoted

[...]

quoted

        if (--ctx->refcount == 0) {
-               err = xsk_get_mmap_offsets(umem->fd, &off);
-               if (!err) {
-                       munmap(ctx->fill->ring - off.fr.desc,
-                              off.fr.desc + umem->config.fill_size *
-                              sizeof(__u64));
-                       munmap(ctx->comp->ring - off.cr.desc,
-                              off.cr.desc + umem->config.comp_size *
-                              sizeof(__u64));
+               if (unmap) {
+                       err = xsk_get_mmap_offsets(umem->fd, &off);
+                       if (!err) {
+                               munmap(ctx->fill->ring - off.fr.desc,
+                                      off.fr.desc + umem->config.fill_size *
+                               sizeof(__u64));
+                               munmap(ctx->comp->ring - off.cr.desc,
+                                      off.cr.desc + umem->config.comp_size *
+                               sizeof(__u64));
+                       }

The whole function increases indent, since it changes anyway
could you write it as:
{
if (--ctx->refcount)
  return;
if (!unmap)
  goto out_free;
err = xsk_get
if (err)
 goto out_free;
munmap();
out_free:
list_del
free
}

Yes, please try to reduce the nesting, and while at it try to expand
the as much as possible of the munmap arguments to the full 100 chars.


Björn