lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 16 Jun 2014 10:42:13 +0200
From:	Ulf Hansson <ulf.hansson@...aro.org>
To:	micky <micky_ching@...lsil.com.cn>
Cc:	Samuel Ortiz <sameo@...ux.intel.com>,
	Lee Jones <lee.jones@...aro.org>,
	Chris Ball <chris@...ntf.net>, devel@...uxdriverproject.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-mmc <linux-mmc@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	Roger <rogerable@...ltek.com>, Wei WANG <wei_wang@...lsil.com.cn>
Subject: Re: [PATCH 2/2] mmc: rtsx: add support for async request

On 6 June 2014 09:05,  <micky_ching@...lsil.com.cn> wrote:
> From: Micky Ching <micky_ching@...lsil.com.cn>
>
> Add support for non-blocking request, pre_req() runs dma_map_sg() and
> post_req() runs dma_unmap_sg(). This patch can increase card read/write
> speed, especially for high speed card and slow speed CPU.
>
> Test on intel i3(800MHz - 2.3GHz) performance mode(2.3GHz), SD card
> clock 208MHz
>
> run dd if=/dev/mmcblk0 of=/dev/null bs=64k count=1024
> before:
> 67108864 bytes (67 MB) copied, 0.85427 s, 78.6 MB/s
> after:
> 67108864 bytes (67 MB) copied, 0.74799 s, 89.7 MB/s
>
> Signed-off-by: Micky Ching <micky_ching@...lsil.com.cn>
> ---
>  drivers/mmc/host/rtsx_pci_sdmmc.c |  133 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 127 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
> index 1c68e0d..a2c0858 100644
> --- a/drivers/mmc/host/rtsx_pci_sdmmc.c
> +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
> @@ -24,6 +24,7 @@
>  #include <linux/highmem.h>
>  #include <linux/delay.h>
>  #include <linux/platform_device.h>
> +#include <linux/workqueue.h>
>  #include <linux/mmc/host.h>
>  #include <linux/mmc/mmc.h>
>  #include <linux/mmc/sd.h>
> @@ -36,7 +37,10 @@ struct realtek_pci_sdmmc {
>         struct rtsx_pcr         *pcr;
>         struct mmc_host         *mmc;
>         struct mmc_request      *mrq;
> +       struct workqueue_struct *workq;
> +#define SDMMC_WORKQ_NAME       "rtsx_pci_sdmmc_workq"
>
> +       struct work_struct      work;

I am trying to understand why you need a work/workqueue to implement
this feature. Is that really the case?

Could you elaborate on the reasons?

Kind regards
Uffe

>         struct mutex            host_mutex;
>
>         u8                      ssc_depth;
> @@ -48,6 +52,11 @@ struct realtek_pci_sdmmc {
>         int                     power_state;
>  #define SDMMC_POWER_ON         1
>  #define SDMMC_POWER_OFF                0
> +
> +       unsigned int            sg_count;
> +       s32                     cookie;
> +       unsigned int            cookie_sg_count;
> +       bool                    using_cookie;
>  };
>
>  static inline struct device *sdmmc_dev(struct realtek_pci_sdmmc *host)
> @@ -86,6 +95,77 @@ static void sd_print_debug_regs(struct realtek_pci_sdmmc *host)
>  #define sd_print_debug_regs(host)
>  #endif /* DEBUG */
>
> +/*
> + * sd_pre_dma_transfer - do dma_map_sg() or using cookie
> + *
> + * @pre: if called in pre_req()
> + * return:
> + *     0 - do dma_map_sg()
> + *     1 - using cookie
> + */
> +static int sd_pre_dma_transfer(struct realtek_pci_sdmmc *host,
> +               struct mmc_data *data, bool pre)
> +{
> +       struct rtsx_pcr *pcr = host->pcr;
> +       int read = data->flags & MMC_DATA_READ;
> +       int count = 0;
> +       int using_cookie = 0;
> +
> +       if (!pre && data->host_cookie && data->host_cookie != host->cookie) {
> +               dev_err(sdmmc_dev(host),
> +                       "error: data->host_cookie = %d, host->cookie = %d\n",
> +                       data->host_cookie, host->cookie);
> +               data->host_cookie = 0;
> +       }
> +
> +       if (pre || data->host_cookie != host->cookie) {
> +               count = rtsx_pci_dma_map_sg(pcr, data->sg, data->sg_len, read);
> +       } else {
> +               count = host->cookie_sg_count;
> +               using_cookie = 1;
> +       }
> +
> +       if (pre) {
> +               host->cookie_sg_count = count;
> +               if (++host->cookie < 0)
> +                       host->cookie = 1;
> +               data->host_cookie = host->cookie;
> +       } else {
> +               host->sg_count = count;
> +       }
> +
> +       return using_cookie;
> +}
> +
> +static void sdmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
> +               bool is_first_req)
> +{
> +       struct realtek_pci_sdmmc *host = mmc_priv(mmc);
> +       struct mmc_data *data = mrq->data;
> +
> +       if (data->host_cookie) {
> +               dev_err(sdmmc_dev(host),
> +                       "error: reset data->host_cookie = %d\n",
> +                       data->host_cookie);
> +               data->host_cookie = 0;
> +       }
> +
> +       sd_pre_dma_transfer(host, data, true);
> +       dev_dbg(sdmmc_dev(host), "pre dma sg: %d\n", host->cookie_sg_count);
> +}
> +
> +static void sdmmc_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
> +               int err)
> +{
> +       struct realtek_pci_sdmmc *host = mmc_priv(mmc);
> +       struct rtsx_pcr *pcr = host->pcr;
> +       struct mmc_data *data = mrq->data;
> +       int read = data->flags & MMC_DATA_READ;
> +
> +       rtsx_pci_dma_unmap_sg(pcr, data->sg, data->sg_len, read);
> +       data->host_cookie = 0;
> +}
> +
>  static int sd_read_data(struct realtek_pci_sdmmc *host, u8 *cmd, u16 byte_cnt,
>                 u8 *buf, int buf_len, int timeout)
>  {
> @@ -415,7 +495,7 @@ static int sd_rw_multi(struct realtek_pci_sdmmc *host, struct mmc_request *mrq)
>
>         rtsx_pci_send_cmd_no_wait(pcr);
>
> -       err = rtsx_pci_transfer_data(pcr, data->sg, data->sg_len, read, 10000);
> +       err = rtsx_pci_dma_transfer(pcr, data->sg, host->sg_count, read, 10000);
>         if (err < 0) {
>                 sd_clear_error(host);
>                 return err;
> @@ -640,12 +720,24 @@ static int sd_tuning_rx(struct realtek_pci_sdmmc *host, u8 opcode)
>         return 0;
>  }
>
> -static void sdmmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
> +static inline int sd_rw_cmd(struct mmc_command *cmd)
>  {
> -       struct realtek_pci_sdmmc *host = mmc_priv(mmc);
> +       return mmc_op_multi(cmd->opcode) ||
> +               (cmd->opcode == MMC_READ_SINGLE_BLOCK) ||
> +               (cmd->opcode == MMC_WRITE_BLOCK);
> +}
> +
> +static void sd_request(struct work_struct *work)
> +{
> +       struct realtek_pci_sdmmc *host = container_of(work,
> +                       struct realtek_pci_sdmmc, work);
>         struct rtsx_pcr *pcr = host->pcr;
> +
> +       struct mmc_host *mmc = host->mmc;
> +       struct mmc_request *mrq = host->mrq;
>         struct mmc_command *cmd = mrq->cmd;
>         struct mmc_data *data = mrq->data;
> +
>         unsigned int data_size = 0;
>         int err;
>
> @@ -677,13 +769,13 @@ static void sdmmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>         if (mrq->data)
>                 data_size = data->blocks * data->blksz;
>
> -       if (!data_size || mmc_op_multi(cmd->opcode) ||
> -                       (cmd->opcode == MMC_READ_SINGLE_BLOCK) ||
> -                       (cmd->opcode == MMC_WRITE_BLOCK)) {
> +       if (!data_size || sd_rw_cmd(cmd)) {
>                 sd_send_cmd_get_rsp(host, cmd);
>
>                 if (!cmd->error && data_size) {
>                         sd_rw_multi(host, mrq);
> +                       if (!host->using_cookie)
> +                               sdmmc_post_req(host->mmc, host->mrq, 0);
>
>                         if (mmc_op_multi(cmd->opcode) && mrq->stop)
>                                 sd_send_cmd_get_rsp(host, mrq->stop);
> @@ -712,6 +804,21 @@ finish:
>         mmc_request_done(mmc, mrq);
>  }
>
> +static void sdmmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
> +{
> +       struct realtek_pci_sdmmc *host = mmc_priv(mmc);
> +       struct mmc_data *data = mrq->data;
> +
> +       mutex_lock(&host->host_mutex);
> +       host->mrq = mrq;
> +       mutex_unlock(&host->host_mutex);
> +
> +       if (sd_rw_cmd(mrq->cmd))
> +               host->using_cookie = sd_pre_dma_transfer(host, data, false);
> +
> +       queue_work(host->workq, &host->work);
> +}
> +
>  static int sd_set_bus_width(struct realtek_pci_sdmmc *host,
>                 unsigned char bus_width)
>  {
> @@ -1144,6 +1251,8 @@ out:
>  }
>
>  static const struct mmc_host_ops realtek_pci_sdmmc_ops = {
> +       .pre_req = sdmmc_pre_req,
> +       .post_req = sdmmc_post_req,
>         .request = sdmmc_request,
>         .set_ios = sdmmc_set_ios,
>         .get_ro = sdmmc_get_ro,
> @@ -1222,10 +1331,16 @@ static int rtsx_pci_sdmmc_drv_probe(struct platform_device *pdev)
>                 return -ENOMEM;
>
>         host = mmc_priv(mmc);
> +       host->workq = create_singlethread_workqueue(SDMMC_WORKQ_NAME);
> +       if (!host->workq) {
> +               mmc_free_host(mmc);
> +               return -ENOMEM;
> +       }
>         host->pcr = pcr;
>         host->mmc = mmc;
>         host->pdev = pdev;
>         host->power_state = SDMMC_POWER_OFF;
> +       INIT_WORK(&host->work, sd_request);
>         platform_set_drvdata(pdev, host);
>         pcr->slots[RTSX_SD_CARD].p_dev = pdev;
>         pcr->slots[RTSX_SD_CARD].card_event = rtsx_pci_sdmmc_card_event;
> @@ -1253,6 +1368,8 @@ static int rtsx_pci_sdmmc_drv_remove(struct platform_device *pdev)
>         pcr->slots[RTSX_SD_CARD].card_event = NULL;
>         mmc = host->mmc;
>
> +       cancel_work_sync(&host->work);
> +
>         mutex_lock(&host->host_mutex);
>         if (host->mrq) {
>                 dev_dbg(&(pdev->dev),
> @@ -1271,6 +1388,10 @@ static int rtsx_pci_sdmmc_drv_remove(struct platform_device *pdev)
>         mmc_remove_host(mmc);
>         host->eject = true;
>
> +       flush_workqueue(host->workq);
> +       destroy_workqueue(host->workq);
> +       host->workq = NULL;
> +
>         mmc_free_host(mmc);
>
>         dev_dbg(&(pdev->dev),
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists