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=ham 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 957D1C433E0 for ; Tue, 22 Dec 2020 05:07:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5983122518 for ; Tue, 22 Dec 2020 05:07:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725850AbgLVFHF (ORCPT ); Tue, 22 Dec 2020 00:07:05 -0500 Received: from m43-15.mailgun.net ([69.72.43.15]:22292 "EHLO m43-15.mailgun.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725785AbgLVFHF (ORCPT ); Tue, 22 Dec 2020 00:07:05 -0500 DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=mg.codeaurora.org; q=dns/txt; s=smtp; t=1608613603; h=Message-ID: References: In-Reply-To: Subject: Cc: To: From: Date: Content-Transfer-Encoding: Content-Type: MIME-Version: Sender; bh=soddIF0hDqxFAa6BrswWI5PMeuSbczjjbn486k4/OZM=; b=CiZDiQOJZhZdDKT1m9Wuqbz+I5LXKqfPt/e5Re8tnRpCFjp0NY2A3YMNiCNxbpbT4IqVgVDU zazR3TIHzYdW9mBkLiQXFycP6silBpXH9ryHQFg5RHfxq06SlWhRb+G+ZU991IpAooALT+P+ RCLd1Y0TvH3Y6/snrZvMVU0ss4w= X-Mailgun-Sending-Ip: 69.72.43.15 X-Mailgun-Sid: WyJlNmU5NiIsICJsaW51eC1zY3NpQHZnZXIua2VybmVsLm9yZyIsICJiZTllNGEiXQ== Received: from smtp.codeaurora.org (ec2-35-166-182-171.us-west-2.compute.amazonaws.com [35.166.182.171]) by smtp-out-n05.prod.us-east-1.postgun.com with SMTP id 5fe17ec8120d248bb5df0a22 (version=TLS1.2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256); Tue, 22 Dec 2020 05:06:16 GMT Sender: cang=codeaurora.org@mg.codeaurora.org Received: by smtp.codeaurora.org (Postfix, from userid 1001) id 6BAD9C433ED; Tue, 22 Dec 2020 05:06:15 +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: cang) by smtp.codeaurora.org (Postfix) with ESMTPSA id C8DFEC433CA; Tue, 22 Dec 2020 05:06:14 +0000 (UTC) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Tue, 22 Dec 2020 13:06:14 +0800 From: Can Guo To: Kiwoong Kim Cc: linux-scsi@vger.kernel.org Subject: Re: [PATCH v3 2/2] ufs: ufs-exynos: set dma_alignment to 4095 In-Reply-To: <000801d6d81c$7e9bc530$7bd34f90$@samsung.com> References: <0cc3dc22424d2052c0cdde8b80aa237b@codeaurora.org> <000101d6d811$052b0f40$0f812dc0$@samsung.com> <000801d6d81c$7e9bc530$7bd34f90$@samsung.com> Message-ID: <614dd657ce4f235f556e6cf49e25bc04@codeaurora.org> X-Sender: cang@codeaurora.org User-Agent: Roundcube Webmail/1.3.9 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org On 2020-12-22 12:39, Kiwoong Kim wrote: >> On 2020-12-22 11:34, Can Guo wrote: >> > On 2020-12-22 11:17, Kiwoong Kim wrote: >> >>> On 2020-12-22 10:21, Kiwoong Kim wrote: >> >>> > Exynos requires one scatterlist entry for smaller than page size, >> i.e. >> >>> > 4KB. For the cases of dispatching commands with more than one >> >>> > scatterlist entry and under 4KB size, Exynos behaves as follows: >> >>> > >> >>> > Given that a command to read something from device is dispatched >> >>> > with two scatterlist entries that are named AAA and BBB. After >> >>> > dispatching, host builds two PRDT entries and during transmission, >> >>> > device sends just one DATA IN because device doesn't care on host >> dma. >> >>> >> >>> If my understanding is correct, above is same to all hosts, only >> >>> below part is Exynos's behavior. Please correct me if I am wrong. >> >> You're correct. >> >> >> >>> >> >>> > The host then tranfers >> >>> > the whole data from start address of the area named AAA. >> >>> > In consequebnce, the area that follows AAA would be corrupted. >> >>> >> >>> In consequence >> >>> >> >>> > >> >>> > |<------------->| >> >>> > +-------+------------ +-------+ >> >>> > + AAA + (corrupted) ... + BBB + >> >>> > +-------+------------ +-------+ >> >>> > >> >>> >> >>> AFAIK, queue->dma_alignment is only used in the case of direct-io, >> >>> i.e. in >> >>> blk_rq_map_user/kern(), which are mainly used in IOCTL. >> >>> If a request's buffer len and/or buffer start addr is not aligned >> >>> with >> >>> queue->dma_alignment, bio.c will make a bounce bio such that the >> >>> request >> >>> get a new buffer which starts on a new page. After the bounce bio is >> >> ended, >> >>> the data in the bound bio will be copied to the initial buffer. >> >>> >> >>> So in this fix, you are making sure the AAA and BBB are all mapped to >> >>> one >> >>> bounce bio and stay in one bi_vec, so when we do map_sg they come in >> >>> one >> >>> sglist, please correct me if I am wrong. >> >>> >> >>> If my understanding is correct, what is the real use case here - >> >>> why/how >> >>> user starts a request which can generate more than one sglists whose >> >>> sizes >> >>> are all under 4KB? I am just curious. >> >>> >> >>> Thanks, >> >>> >> >>> Can Guo. >> >> >> >> You nearly exactly got what I’m thinking. >> >> And I think there could be various cases making those situations, >> >> which are definitely up to user programs. That is the case using >> >> different memory areas to contain something. >> >> >> > >> >> Hi Kiwoong, >> >> Sorry if I made you confused. I think I know your intention here now. >> When bio.c makes the bounce bio, it allocates one new page and >> add this page to the bounce bio. In this way, only one bi_vec is >> needed and only one sglist entry shall be generated. Am I right? >> >> Thanks, >> >> Can Guo. > > Yes, that's it. > > > Thanks. > Kiwoong Kim Other than the typo in commit msg, the change looks good to me. :) Regards, Can Guo. >> > >> >> Thanks. >> >> Kiwoong Kim >> >>> >> >>> > Signed-off-by: Kiwoong Kim >> >>> > --- >> >>> > drivers/scsi/ufs/ufs-exynos.c | 9 +++++++++ >> >>> > 1 file changed, 9 insertions(+) >> >>> > >> >>> > diff --git a/drivers/scsi/ufs/ufs-exynos.c >> >>> > b/drivers/scsi/ufs/ufs-exynos.c index a8770ff..8635d9d 100644 >> >>> > --- a/drivers/scsi/ufs/ufs-exynos.c >> >>> > +++ b/drivers/scsi/ufs/ufs-exynos.c >> >>> > @@ -14,6 +14,7 @@ >> >>> > #include >> >>> > #include >> >>> > #include >> >>> > +#include >> >>> > >> >>> > #include "ufshcd.h" >> >>> > #include "ufshcd-pltfrm.h" >> >>> > @@ -1193,6 +1194,13 @@ static int exynos_ufs_resume(struct ufs_hba >> >>> > *hba, enum ufs_pm_op pm_op) >> >>> > return 0; >> >>> > } >> >>> > >> >>> > +static void exynos_ufs_slave_configure(struct scsi_device *sdev) { >> >>> > + struct request_queue *q = sdev->request_queue; >> >>> > + >> >>> > + blk_queue_update_dma_alignment(q, PAGE_SIZE - 1); } >> >>> > + >> >>> > static struct ufs_hba_variant_ops ufs_hba_exynos_ops = { >> >>> > .name = "exynos_ufs", >> >>> > .init = exynos_ufs_init, >> >>> > @@ -1204,6 +1212,7 @@ static struct ufs_hba_variant_ops >> >>> > ufs_hba_exynos_ops = { >> >>> > .hibern8_notify = exynos_ufs_hibern8_notify, >> >>> > .suspend = exynos_ufs_suspend, >> >>> > .resume = exynos_ufs_resume, >> >>> > + .slave_configure = exynos_ufs_slave_configure, >> >>> > }; >> >>> > >> >>> > static int exynos_ufs_probe(struct platform_device *pdev)