From patchwork Wed Sep 9 17:21:28 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emil Velikov X-Patchwork-Id: 7147661 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 2130B9F326 for ; Wed, 9 Sep 2015 17:17:36 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B881620A13 for ; Wed, 9 Sep 2015 17:17:34 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 71E0120A09 for ; Wed, 9 Sep 2015 17:17:33 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BE42E6EC88; Wed, 9 Sep 2015 10:17:32 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wi0-f173.google.com (mail-wi0-f173.google.com [209.85.212.173]) by gabe.freedesktop.org (Postfix) with ESMTPS id 782426EC81 for ; Wed, 9 Sep 2015 10:17:30 -0700 (PDT) Received: by wiclk2 with SMTP id lk2so165403696wic.0 for ; Wed, 09 Sep 2015 10:17:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=rHCeM6CqdW6yGXOD4lE1lX60yXPwQ27Q5cym1Y3CNSE=; b=vpIwycsxoFE/OTYhM8LzXgrUBEZaULkDTKBMOh4GadR4rqBYyhygwYjiSTVWxlWa9J K+kekNwVYCkD2t0CXNc60OX1bkNd166hqwHCRRrJ1Snc8ZcJArVN0Mfu1HCTqz+jCQu5 KdWc2GLye7JNzaizWd+JZhxqzYBe0iliYatQAadAHuXwIFPUCARSMYy78L0QzpgfHOWX hUfIK5Hul2w63caiQZb1jIhnMCktDU+bBQNZoIP9mi7/wcHyyMAMNEctzdVVAevVkLKK 3N9chioJK/t0Gvj3GWDM20NpSUqsC6tfxzLlRh+gq96vbJbVfPTywgMuxMtzAQAEw8Vn mYFQ== X-Received: by 10.180.83.7 with SMTP id m7mr58737842wiy.60.1441819049251; Wed, 09 Sep 2015 10:17:29 -0700 (PDT) Received: from arch-x220.localdomain (cpc12-croy20-2-0-cust52.croy.cable.virginm.net. [82.44.54.53]) by smtp.gmail.com with ESMTPSA id j7sm11257042wjz.11.2015.09.09.10.17.28 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 09 Sep 2015 10:17:28 -0700 (PDT) From: Emil Velikov To: dri-devel@lists.freedesktop.org Subject: [PATCH libdrm 07/12] xf86drm: rework drmGetDevices() Date: Wed, 9 Sep 2015 18:21:28 +0100 Message-Id: <1441819293-25567-8-git-send-email-emil.l.velikov@gmail.com> X-Mailer: git-send-email 2.5.0 In-Reply-To: <1441819293-25567-1-git-send-email-emil.l.velikov@gmail.com> References: <1441819293-25567-1-git-send-email-emil.l.velikov@gmail.com> Cc: emil.l.velikov@gmail.com X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Do a once off memory allocation for each drmDevice. This allows us to ease the error handling and simplify the de-duplication loop. As part of this we need to rework drmFreeDevice() such so that it frees the relevant hunks, rather than leaving that to the caller. Some memory stats from the drmdevice test before: 22 allocs, 22 frees, 66,922 bytes allocated after: 9 allocs, 9 frees, 66,436 bytes allocated Signed-off-by: Emil Velikov --- xf86drm.c | 204 +++++++++++++++++++++++++++++--------------------------------- 1 file changed, 94 insertions(+), 110 deletions(-) diff --git a/xf86drm.c b/xf86drm.c index 310d1e8..8105b42 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -65,6 +65,8 @@ #include "xf86drm.h" #include "libdrm_macros.h" +#include "util_math.h" + #ifdef __OpenBSD__ #define DRM_PRIMARY_MINOR_NAME "drm" #define DRM_CONTROL_MINOR_NAME "drmC" @@ -2928,6 +2930,15 @@ static int drmGetNodeType(const char *name) return -EINVAL; } +static int drmGetMaxNodeName(void) +{ + return sizeof(DRM_DIR_NAME) + + MAX3(sizeof(DRM_PRIMARY_MINOR_NAME), + sizeof(DRM_CONTROL_MINOR_NAME), + sizeof(DRM_RENDER_MINOR_NAME)) + + 3 /* lenght of the node number */; +} + static int drmParsePciDeviceInfo(const char *d_name, drmPciDeviceInfoPtr device) { @@ -2954,20 +2965,13 @@ static int drmParsePciDeviceInfo(const char *d_name, return 0; } -static void drmFreeDevice(drmDevicePtr device) +static void drmFreeDevice(drmDevicePtr *device) { - int i; - if (device == NULL) return; - if (device->nodes != NULL) - for (i = 0; i < DRM_NODE_MAX; i++) - free(device->nodes[i]); - - free(device->nodes); - free(device->businfo.pci); - free(device->deviceinfo.pci); + free(*device); + *device = NULL; } void drmFreeDevices(drmDevicePtr devices[], int count) @@ -2977,11 +2981,8 @@ void drmFreeDevices(drmDevicePtr devices[], int count) if (devices == NULL) return; - for (i = 0; i < count; i++) { - drmFreeDevice(devices[i]); - free(devices[i]); - devices[i] = NULL; - } + for (i = 0; i < count && devices[i] != NULL; i++) + drmFreeDevice(&devices[i]); } /** @@ -2998,29 +2999,30 @@ void drmFreeDevices(drmDevicePtr devices[], int count) */ int drmGetDevices(drmDevicePtr devices[], int max_devices) { - drmDevicePtr devs = NULL; - drmPciBusInfoPtr pcibus = NULL; - drmPciDeviceInfoPtr pcidevice = NULL; - DIR *sysdir = NULL; - struct dirent *dent = NULL; - struct stat sbuf = {0}; - char node[PATH_MAX + 1] = ""; + drmDevicePtr *local_devices; + drmDevicePtr device; + DIR *sysdir; + struct dirent *dent; + struct stat sbuf; + char node[PATH_MAX + 1]; + const int max_node_str = drmGetMaxNodeName(); int node_type, subsystem_type; int maj, min; - int ret, i = 0, j, node_count, device_count = 0; + int ret, i, j, node_count, device_count; int max_count = 16; - int *duplicated = NULL; + void *addr; - devs = calloc(max_count, sizeof(*devs)); - if (devs == NULL) + local_devices = calloc(max_count, sizeof(drmDevicePtr)); + if (local_devices == NULL) return -ENOMEM; sysdir = opendir(DRM_DIR_NAME); if (!sysdir) { ret = -errno; - goto free_locals; + goto close_sysdir; } + i = 0; while ((dent = readdir(sysdir))) { node_type = drmGetNodeType(dent->d_name); if (node_type < 0) @@ -3043,115 +3045,97 @@ int drmGetDevices(drmDevicePtr devices[], int max_devices) switch (subsystem_type) { case DRM_BUS_PCI: - pcibus = calloc(1, sizeof(*pcibus)); - if (pcibus == NULL) { - ret = -ENOMEM; - goto free_locals; + addr = device = calloc(1, sizeof(drmDevice) + + (DRM_NODE_MAX * + (sizeof(void *) + max_node_str)) + + sizeof(drmPciBusInfo) + + sizeof(drmPciDeviceInfo)); + if (!device) + goto free_devices; + + device->bustype = subsystem_type; + device->available_nodes = 1 << node_type; + + addr += sizeof(drmDevice); + device->nodes = addr; + + addr += DRM_NODE_MAX * sizeof(void *); + for (j = 0; j < DRM_NODE_MAX; j++) { + device->nodes[j] = addr; + addr += max_node_str; } + memcpy(device->nodes[node_type], node, max_node_str); - ret = drmParsePciBusInfo(maj, min, pcibus); - if (ret) - goto free_locals; - - if (i >= max_count) { - max_count += 16; - devs = realloc(devs, max_count * sizeof(*devs)); - } + device->businfo.pci = addr; - devs[i].businfo.pci = pcibus; - devs[i].bustype = subsystem_type; - devs[i].nodes = calloc(DRM_NODE_MAX, sizeof(char *)); - if (devs[i].nodes == NULL) { - ret = -ENOMEM; - goto free_locals; - } - devs[i].nodes[node_type] = strdup(node); - if (devs[i].nodes[node_type] == NULL) { - ret = -ENOMEM; - goto free_locals; - } - devs[i].available_nodes = 1 << node_type; + ret = drmParsePciBusInfo(maj, min, device->businfo.pci); + if (ret) + goto free_devices; + // Fetch the device info if the user has requested it if (devices != NULL) { - pcidevice = calloc(1, sizeof(*pcidevice)); - if (pcidevice == NULL) { - ret = -ENOMEM; - goto free_locals; - } + addr += sizeof(drmPciBusInfo); + device->deviceinfo.pci = addr; - ret = drmParsePciDeviceInfo(dent->d_name, pcidevice); + ret = drmParsePciDeviceInfo(dent->d_name, + device->deviceinfo.pci); if (ret) - goto free_locals; - - devs[i].deviceinfo.pci = pcidevice; + goto free_devices; } + break; default: fprintf(stderr, "The subsystem type is not supported yet\n"); break; } - i++; - } - node_count = i; + if (i >= max_count) { + drmDevicePtr *temp; - /* merge duplicated devices with same domain/bus/device/func IDs */ - duplicated = calloc(node_count, sizeof(*duplicated)); - if (duplicated == NULL) { - ret = -ENOMEM; - goto free_locals; + max_count += 16; + temp = realloc(local_devices, max_count * sizeof(drmDevicePtr)); + if (!temp) + goto free_devices; + local_devices = temp; + } + + local_devices[i] = device; + i++; } + node_count = i; + /* Fold nodes into a single device if they share the same bus info */ for (i = 0; i < node_count; i++) { - for (j = i+1; j < node_count; j++) { - if (duplicated[i] || duplicated[j]) - continue; - if (drmCompareBusInfo(&devs[i], &devs[j]) == 0) { - duplicated[j] = 1; - devs[i].available_nodes |= devs[j].available_nodes; - node_type = log2(devs[j].available_nodes); - devs[i].nodes[node_type] = devs[j].nodes[node_type]; - free(devs[j].nodes); - free(devs[j].businfo.pci); - free(devs[j].deviceinfo.pci); + for (j = i + 1; j < node_count; j++) { + if (drmCompareBusInfo(local_devices[i], local_devices[j]) == 0) { + local_devices[i]->available_nodes |= local_devices[j]->available_nodes; + node_type = log2(local_devices[j]->available_nodes); + memcpy(local_devices[i]->nodes[node_type], + local_devices[j]->nodes[node_type], max_node_str); + drmFreeDevice(&local_devices[j]); } } } - for (i = 0; i < node_count; i++) { - if(duplicated[i] == 0) { - if ((devices != NULL) && (device_count < max_devices)) { - devices[device_count] = calloc(1, sizeof(drmDevice)); - if (devices[device_count] == NULL) { - ret = -ENOMEM; - break; - } - memcpy(devices[device_count], &devs[i], sizeof(drmDevice)); - } else - drmFreeDevice(&devs[i]); - device_count++; - } - } + device_count = 0; + for (i = 0; i < node_count && local_devices[i]; i++) { + if ((devices != NULL) && (device_count < max_devices)) + devices[device_count] = local_devices[i]; + else + drmFreeDevice(&local_devices[i]); - if (i < node_count) { - drmFreeDevices(devices, device_count); - for ( ; i < node_count; i++) - if(duplicated[i] == 0) - drmFreeDevice(&devs[i]); - } else - ret = device_count; + device_count++; + } - free(duplicated); - free(devs); + free(local_devices); closedir(sysdir); - return ret; + return device_count; + +free_devices: + drmFreeDevices(local_devices, i); + free(local_devices); -free_locals: - for (j = 0; j < i; j++) - drmFreeDevice(&devs[j]); - free(pcidevice); - free(pcibus); - free(devs); +close_sysdir: closedir(sysdir); return ret; }