From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D5F0DC433EF for ; Fri, 24 Sep 2021 06:25:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B6FFF61250 for ; Fri, 24 Sep 2021 06:25:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244203AbhIXG1P (ORCPT ); Fri, 24 Sep 2021 02:27:15 -0400 Received: from so254-9.mailgun.net ([198.61.254.9]:58839 "EHLO so254-9.mailgun.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244191AbhIXG1M (ORCPT ); Fri, 24 Sep 2021 02:27:12 -0400 DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=mg.codeaurora.org; q=dns/txt; s=smtp; t=1632464739; h=Message-ID: References: In-Reply-To: Subject: Cc: To: From: Date: Content-Transfer-Encoding: Content-Type: MIME-Version: Sender; bh=QlR56ZIA9jMdRCNZViamyRPbdHO9b5RbsDdO6yIrMAE=; b=vmRfk1S3sFqR9jPpPAZNrIYHN3TGgTRLZfHLzu9gUQK9FAQPihZba09poCY3QSdl3ZFhlVlE bi8ow1u5TQz+9ywv6IDxHyfSjIxSOd37LRGaNpGuuT46nJkh33Olx+6K6iMnoAGazT+6N59f +GMkLSboSbewoqIonJ7yQ672W1Y= X-Mailgun-Sending-Ip: 198.61.254.9 X-Mailgun-Sid: WyI0MWYwYSIsICJsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnIiwgImJlOWU0YSJd Received: from smtp.codeaurora.org (ec2-35-166-182-171.us-west-2.compute.amazonaws.com [35.166.182.171]) by smtp-out-n03.prod.us-east-1.postgun.com with SMTP id 614d6f5ee0480a7d6f53555b (version=TLS1.2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256); Fri, 24 Sep 2021 06:25:34 GMT Sender: jeyr=codeaurora.org@mg.codeaurora.org Received: by smtp.codeaurora.org (Postfix, from userid 1001) id 80BFCC4360D; Fri, 24 Sep 2021 06:25:33 +0000 (UTC) Received: from mail.codeaurora.org (localhost.localdomain [127.0.0.1]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: jeyr) by smtp.codeaurora.org (Postfix) with ESMTPSA id BDCD1C4338F; Fri, 24 Sep 2021 06:25:32 +0000 (UTC) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Fri, 24 Sep 2021 11:55:32 +0530 From: jeyr@codeaurora.org To: Greg KH Cc: linux-arm-msm@vger.kernel.org, srinivas.kandagatla@linaro.org, linux-kernel@vger.kernel.org, fastrpc.upstream@qti.qualcomm.com Subject: Re: [PATCH] misc: fastrpc: copy to user only for non-DMA-BUF heap buffers In-Reply-To: References: <1632386272-18139-1-git-send-email-jeyr@codeaurora.org> Message-ID: X-Sender: jeyr@codeaurora.org User-Agent: Roundcube Webmail/1.3.9 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2021-09-23 23:13, Greg KH wrote: > On Thu, Sep 23, 2021 at 03:54:15PM +0530, jeyr@codeaurora.org wrote: >> On 2021-09-23 14:19, Greg KH wrote: >> > On Thu, Sep 23, 2021 at 02:07:52PM +0530, Jeya R wrote: >> > > fastrpc_put_args is copying all the output buffers to user. For large >> > > number of output context buffers, this might cause performance >> > > degradation. Copying is not needed for DMA-BUF heap buffers. >> > >> > What does "performance degradation" really mean? >> >> Unnecessary copying for large number of buffers would cause some >> additional time which would get added to overall fastrpc call cost. >> >> > >> > > >> > > Signed-off-by: Jeya R >> > > --- >> > > drivers/misc/fastrpc.c | 18 ++++++++++-------- >> > > 1 file changed, 10 insertions(+), 8 deletions(-) >> > > >> > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c >> > > index beda610..536eabf 100644 >> > > --- a/drivers/misc/fastrpc.c >> > > +++ b/drivers/misc/fastrpc.c >> > > @@ -890,15 +890,17 @@ static int fastrpc_put_args(struct >> > > fastrpc_invoke_ctx *ctx, >> > > inbufs = REMOTE_SCALARS_INBUFS(ctx->sc); >> > > >> > > for (i = inbufs; i < ctx->nbufs; ++i) { >> > > - void *src = (void *)(uintptr_t)rpra[i].pv; >> > > - void *dst = (void *)(uintptr_t)ctx->args[i].ptr; >> > > - u64 len = rpra[i].len; >> > > + if (!ctx->maps[i]) { >> > > + void *src = (void *)(uintptr_t)rpra[i].pv; >> > > + void *dst = (void *)(uintptr_t)ctx->args[i].ptr; >> > >> > uintptr_t is not a kernel variable type. Please use the real kernel >> > type for this as you are touching these lines. >> > >> >> Sure, thanks for pointing this. Will update this in the next patch. >> >> > > + u64 len = rpra[i].len; >> > > >> > > - if (!kernel) { >> > > - if (copy_to_user((void __user *)dst, src, len)) >> > > - return -EFAULT; >> > > - } else { >> > > - memcpy(dst, src, len); >> > > + if (!kernel) { >> > > + if (copy_to_user((void __user *)dst, src, len)) >> > > + return -EFAULT; >> > > + } else { >> > > + memcpy(dst, src, len); >> > > + } >> > >> > So you were copying buffers that didn't need to be copied? So you are >> > now doing less work? Or is this fixing a bug where you were copying >> > things that you should not have been copying? >> > >> > What commit does this fix? Does this need to go to the stable kernel >> > trees? >> > >> >> Yes, not all buffer needs to be copied. This change would avoid >> unnecessary >> copying of buffers. Not adding fix tag as it's not exactly fixing any >> bug. >> This should go to stable kernel trees. > > If it's not fixing a bug, why should it go into the stable trees? This is not a bug fix, it can be considered as an enhancement and it can go in to new release.