From owner-svn-src-all@freebsd.org Fri Jun 21 00:04:35 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 20CA615CB9B1; Fri, 21 Jun 2019 00:04:35 +0000 (UTC) (envelope-from mavbsd@gmail.com) Received: from mail-yw1-xc44.google.com (mail-yw1-xc44.google.com [IPv6:2607:f8b0:4864:20::c44]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 9EEB36DDA8; Fri, 21 Jun 2019 00:04:34 +0000 (UTC) (envelope-from mavbsd@gmail.com) Received: by mail-yw1-xc44.google.com with SMTP id y185so1948307ywy.8; Thu, 20 Jun 2019 17:04:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:to:cc:references:from:openpgp:autocrypt:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=MHcUxns/PN03ch6jdGMk7AD8GoiElP37GLgf2lDNsUk=; b=bC00iCXJohDhndPJfj5J0BLEKQB/xQUpzCB1MW3XhyHUo7sdfbXYMQV5x2raKxMWke sMtq6SmguaHizgGLVTYSJhFqaS+3TSN2ICO9viLygpxh3XPmPBZ+d2njBlxFRjcrdmRz YXDcGRUn5OfN+J/De0lx3Aoo3R7aADa03AmQ/DiKFUXM67O0gYg8bdo1+s+0TzbJ0s05 GUj7v9Me/CbIqdtQDmA6tR8pQMbvpw9aqGGUypWkUh6wGhIUhnKFiybrJP8pQ3kdH4C3 mV66eFibqO0mPYVa8d4tvPPBu7sQSie282Ed1PtwHnVNVuPhTmvUvhYDp8oUUljK07Mr orCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:to:cc:references:from:openpgp :autocrypt:message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=MHcUxns/PN03ch6jdGMk7AD8GoiElP37GLgf2lDNsUk=; b=S/tSNzNIGi3krwSFl6G0oFCdW6EdCZHcieV3dEweYDO08zD/rL70cHOrHReR8te0aM cxaJC8YmLR9mv8qyPba/Ck+coviRhEP/D/FnVJJrou38byxVBJLUwnjdgu5Vob26lOUt 9c15YnOzsKsWXt/bJB/01Lv+r93sCrYiYkqjb1V3FrvD1uLSsv0oQpg5KM+eLdW0ucyW qdmLzoLU6E6P13EpRFxaUW3Y9CmWmHRJHLvTfUGKELcWJNmv0pLemQVnwF/DgyYk9nag tijwlDkiSlaeJdrUf9euASPuwX1yQwOVQRo5/CwPFq2S49zz5aCZDndxU5vN1YR7tbFF SNQg== X-Gm-Message-State: APjAAAUOsjWHN438aXqeIjiDBnj2lcnChqqKeuQA38p6zK77JmhQ24nf /o+F7mBkbRC0Aa8H6SVJOev1rRCuNbE= X-Google-Smtp-Source: APXvYqytPl7XZjpHP5MaCbK5AwTYqh4MTIm5stZkKlwOCsHzbeEGp18M+UEOaYAeFy0INygK2iHeNQ== X-Received: by 2002:a81:1a08:: with SMTP id a8mr69573505ywa.29.1561075473316; Thu, 20 Jun 2019 17:04:33 -0700 (PDT) Received: from spectre.mavhome.dp.ua ([2600:1700:3580:3560:228:f8ff:fe04:d12]) by smtp.gmail.com with ESMTPSA id 199sm280205ywr.101.2019.06.20.17.04.32 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Thu, 20 Jun 2019 17:04:32 -0700 (PDT) Sender: Alexander Motin Subject: Re: svn commit: r349243 - head/sys/cam To: Scott Long Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201906202029.x5KKThjn017867@repo.freebsd.org> <6B266DFC-87FF-4742-ACEE-0D37EABEFD7A@samsco.org> From: Alexander Motin Openpgp: preference=signencrypt Autocrypt: addr=mav@FreeBSD.org; prefer-encrypt=mutual; keydata= mQENBFOzxAwBCADkPrax0pI2W/ig0CK9nRJJwsHitAGEZ2HZiFEuti+6/4UVxj81yr4ak/4g 9bKUyC7rMEAp/ZHNhd+MFCPAAcHPvtovnfykqE/vuosCS3wlSLloix2iKVLks0CwbLHGAyne 46lTQW74Xl/33c3W1Z6d8jD9gVFT/xaVzZ0U9xdzOmsYAZaAj4ki0tuxO9F7L+ct9grRe7iP g8t9hai7BL4ee3VRwk2JXnKb7UvBiVITKYWKz1jRvZIrjPokgEcCLOSlv7x/1kjuFnj3xWZU 7HSFFT8J93epBbrSSCsYsppIk2fZH41kaaFXsMQfTPH8wkeM6qwrvOh4HiQM08R+9tThABEB AAG0IUFsZXhhbmRlciBNb3RpbiA8bWF2QEZyZWVCU0Qub3JnPokBVwQTAQoAQQIbAwULCQgH AwUVCgkICwUWAwIBAAIeAQIXgAIZARYhBOmM88TmnMPNDledVYMYw5VbqyJ/BQJZYMKuBQkN McyiAAoJEIMYw5VbqyJ/tuUIAOG3ONOSNYqjK4eTZ1TVh9jdUBAhWk5nhDFnODN49Wj0AbYm 7aIqy8O1hnCDSZG5LttjSAo3UfXJZDKQM0BLb0gpRMBnAYqO6tdolLNqAbPGJBnGoPjsh24y 6KcbDaNnis+lD4GwPXwQM+92wZGhCUFElPV9NciZGVS65TNIgk7X+yEjjhD1MSWKKijZ1r9Z zIt4OzUTxxNOvzdlABZS88nNRdJkatOQJPmFdd1mpP6UzTNCiLUo1pIqOEtJgvVVDYq5WHY6 tciWWYdmZG/tIBexJmv2mV2OLVjXR6ZeKmntVH14H72/wRHJuYHQC+r5SVRcWWayrThsY6jZ Yr4+raS5AQ0EU7PEDAEIAOZgWf2cJIu+58IzP2dkXE/urj3tr4OqrB/yHGWUf71Lz6D0Fi6Z AXgDtmcFLGPfMyWuLAvSM+xmoguk7zC4hRBYvQycmIhuqBq1jO1Wp/Z+lpoPM/1cDYLn8Flv mI/c40MhUZh345DA4jYWWaZNjQHUWVQ1fPf595vdVVMPT/abE8E5DaF6fSkRmqFTmfYRkfbt 3ytU8NdUapDcJVY7cEP2nJBVNZPnOIObR/ZIgSxjjrG5o34yXoqeup8JvwEv+/NylzzuyXEZ R1EdEIzQ/a1nh/0j4NXtzZEqKW4aTWlmSqb6wN8jh1OSOOqkYsfnE3nfxcZbxi4IRoNQYlm5 9R8AEQEAAYkBPAQYAQoAJgIbDBYhBOmM88TmnMPNDledVYMYw5VbqyJ/BQJZYMLYBQkNMczM AAoJEIMYw5VbqyJ/TqgH/RQHClkvecE0262lwKoP/m0Mh4I5TLRgoJJn8S7G1BnqohYJkiLq A6xe6urGD7OqdNAl12UbrjWbdJV+zvea3vJoM4MZuYiYrGaXWxzFXqWJcPwMU9sAh8MRghHu uC5vgPb45Tnftw9/+n0i8GfVhQhOqepUGdQg4NPcXviSkoAvig6pp9Lcxisn0groUQKt15Gc sS9YcQWg3j9Hnipc6Mu416HX98Fb113NHJqc2geTHLkRyuBFOoyIqB6N9GKjzOAIzxxsVdl9 TevwGsrp4M4/RFzWbSgsbOnbE7454lmuVZGfReEjnUm8RHp9Q2UWKXlp3exlZjvOp/uVEpCg lz65AQ0EU7PEDAEIAOZgWf2cJIu+58IzP2dkXE/urj3tr4OqrB/yHGWUf71Lz6D0Fi6ZAXgD tmcFLGPfMyWuLAvSM+xmoguk7zC4hRBYvQycmIhuqBq1jO1Wp/Z+lpoPM/1cDYLn8FlvmI/c 40MhUZh345DA4jYWWaZNjQHUWVQ1fPf595vdVVMPT/abE8E5DaF6fSkRmqFTmfYRkfbt3ytU 8NdUapDcJVY7cEP2nJBVNZPnOIObR/ZIgSxjjrG5o34yXoqeup8JvwEv+/NylzzuyXEZR1Ed EIzQ/a1nh/0j4NXtzZEqKW4aTWlmSqb6wN8jh1OSOOqkYsfnE3nfxcZbxi4IRoNQYlm59R8A EQEAAYkBPAQYAQoAJgIbDBYhBOmM88TmnMPNDledVYMYw5VbqyJ/BQJZYMLYBQkNMczMAAoJ EIMYw5VbqyJ/TqgH/RQHClkvecE0262lwKoP/m0Mh4I5TLRgoJJn8S7G1BnqohYJkiLqA6xe 6urGD7OqdNAl12UbrjWbdJV+zvea3vJoM4MZuYiYrGaXWxzFXqWJcPwMU9sAh8MRghHuuC5v gPb45Tnftw9/+n0i8GfVhQhOqepUGdQg4NPcXviSkoAvig6pp9Lcxisn0groUQKt15GcsS9Y cQWg3j9Hnipc6Mu416HX98Fb113NHJqc2geTHLkRyuBFOoyIqB6N9GKjzOAIzxxsVdl9Tevw Gsrp4M4/RFzWbSgsbOnbE7454lmuVZGfReEjnUm8RHp9Q2UWKXlp3exlZjvOp/uVEpCglz4= Message-ID: <42dbbc7f-51ea-bc3d-0432-5cc9b872d0e7@FreeBSD.org> Date: Thu, 20 Jun 2019 20:04:32 -0400 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <6B266DFC-87FF-4742-ACEE-0D37EABEFD7A@samsco.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 9EEB36DDA8 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.98 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[]; NEURAL_HAM_SHORT(-0.98)[-0.979,0] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Jun 2019 00:04:35 -0000 Hi Scott, You may see this buffer content is not returned directly to user any more. It is used only as temporary storage to pull CDAI_TYPE_SCSI_DEVID, which is then converted into readable form, returned to user. And the code is explicitly made to not read outside the range that was copied to the buffer with simple and dumb memcpy(). The effect is clearly visible in profiler when GEOM confxml calls it 3 times for each disk, taking about 25% of the sysctl's CPU time. With hundreds of disks it is not so small. On 20.06.2019 18:59, Scott Long wrote: > Hi Alexander, > > I’m not a fan of removing the M_ZERO. I understand your argument that > lengths are provided elsewhere, but having the buffer be zero’d is defensive > against future bugs that might leak buffer contents. GETATTR isn’t typically > in a performance path, is there any measurable benefit to this? > > Thanks, > Scott > > >> On Jun 20, 2019, at 2:29 PM, Alexander Motin wrote: >> >> Author: mav >> Date: Thu Jun 20 20:29:42 2019 >> New Revision: 349243 >> URL: https://svnweb.freebsd.org/changeset/base/349243 >> >> Log: >> Optimize xpt_getattr(). >> >> Do not allocate temporary buffer for attributes we are going to return >> as-is, just make sure to NUL-terminate them. Do not zero temporary 64KB >> buffer for CDAI_TYPE_SCSI_DEVID, XPT tells us how much data it filled >> and there are also length fields inside the returned data also. >> >> MFC after: 2 weeks >> Sponsored by: iXsystems, Inc. >> >> Modified: >> head/sys/cam/cam_xpt.c >> >> Modified: head/sys/cam/cam_xpt.c >> ============================================================================== >> --- head/sys/cam/cam_xpt.c Thu Jun 20 20:06:19 2019 (r349242) >> +++ head/sys/cam/cam_xpt.c Thu Jun 20 20:29:42 2019 (r349243) >> @@ -1253,6 +1253,7 @@ xpt_getattr(char *buf, size_t len, const char *attr, s >> cdai.ccb_h.func_code = XPT_DEV_ADVINFO; >> cdai.flags = CDAI_FLAG_NONE; >> cdai.bufsiz = len; >> + cdai.buf = buf; >> >> if (!strcmp(attr, "GEOM::ident")) >> cdai.buftype = CDAI_TYPE_SERIAL_NUM; >> @@ -1262,14 +1263,14 @@ xpt_getattr(char *buf, size_t len, const char *attr, s >> strcmp(attr, "GEOM::lunname") == 0) { >> cdai.buftype = CDAI_TYPE_SCSI_DEVID; >> cdai.bufsiz = CAM_SCSI_DEVID_MAXLEN; >> + cdai.buf = malloc(cdai.bufsiz, M_CAMXPT, M_NOWAIT); >> + if (cdai.buf == NULL) { >> + ret = ENOMEM; >> + goto out; >> + } >> } else >> goto out; >> >> - cdai.buf = malloc(cdai.bufsiz, M_CAMXPT, M_NOWAIT|M_ZERO); >> - if (cdai.buf == NULL) { >> - ret = ENOMEM; >> - goto out; >> - } >> xpt_action((union ccb *)&cdai); /* can only be synchronous */ >> if ((cdai.ccb_h.status & CAM_DEV_QFRZN) != 0) >> cam_release_devq(cdai.ccb_h.path, 0, 0, 0, FALSE); >> @@ -1334,13 +1335,15 @@ xpt_getattr(char *buf, size_t len, const char *attr, s >> ret = EFAULT; >> } >> } else { >> - ret = 0; >> - if (strlcpy(buf, cdai.buf, len) >= len) >> + if (cdai.provsiz < len) { >> + cdai.buf[cdai.provsiz] = 0; >> + ret = 0; >> + } else >> ret = EFAULT; >> } >> >> out: >> - if (cdai.buf != NULL) >> + if ((char *)cdai.buf != buf) >> free(cdai.buf, M_CAMXPT); >> return ret; >> } >> > -- Alexander Motin