| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BANLkTinGRy5KRJLYmEzzeFhOm0WpngzhhA@mail.gmail.com>
Date: Wed, 22 Jun 2011 11:08:06 +0200
From: Per Forlin <per.forlin@...aro.org>
To: "S, Venkatraman" <svenkatr@...com>
Cc: linaro-dev <linaro-dev@...ts.linaro.org>,
Nicolas Pitre <nicolas.pitre@...aro.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-mmc <linux-mmc@...r.kernel.org>,
Chris Ball <cjb@...top.org>
Subject: Re: [PATCH v7 01/11] mmc: add non-blocking mmc request function
On 22 June 2011 10:53, S, Venkatraman <svenkatr@...com> wrote:
> On Wed, Jun 22, 2011 at 2:15 PM, Per Forlin <per.forlin@...aro.org> wrote:
>> On 22 June 2011 09:42, Venkatraman S <svenkatr@...com> wrote:
>>> On Wed, Jun 22, 2011 at 5:08 AM, Per Forlin <per.forlin@...aro.org> wrote:
>>>> Previously there has only been one function mmc_wait_for_req()
>>>> to start and wait for a request. This patch adds
>>>> * mmc_start_req() - starts a request wihtout waiting
>>>> If there is on ongoing request wait for completion
>>>> of that request and start the new one and return.
>>>> Does not wait for the new command to complete.
>>>>
>>>> This patch also adds new function members in struct mmc_host_ops
>>>> only called from core.c
>>>> * pre_req - asks the host driver to prepare for the next job
>>>> * post_req - asks the host driver to clean up after a completed job
>>>>
>>>> The intention is to use pre_req() and post_req() to do cache maintenance
>>>> while a request is active. pre_req() can be called while a request is active
>>>> to minimize latency to start next job. post_req() can be used after the next
>>>> job is started to clean up the request. This will minimize the host driver
>>>> request end latency. post_req() is typically used before ending the block
>>>> request and handing over the buffer to the block layer.
>>>>
>>>> Add a host-private member in mmc_data to be used by
>>>> pre_req to mark the data. The host driver will then
>>>> check this mark to see if the data is prepared or not.
>>>>
>>>> Signed-off-by: Per Forlin <per.forlin@...aro.org>
>>>> ---
>>>> drivers/mmc/core/core.c | 110 +++++++++++++++++++++++++++++++++++++++++----
>>>> include/linux/mmc/core.h | 6 ++-
>>>> include/linux/mmc/host.h | 21 +++++++++
>>>> 3 files changed, 126 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>> index 68091dd..c82fa3b 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -198,9 +198,106 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
>>>>
>>>> static void mmc_wait_done(struct mmc_request *mrq)
>>>> {
>>>> - complete(mrq->done_data);
>>>> + complete(&mrq->completion);
>>>> }
>>>>
>>>> +static void __mmc_start_req(struct mmc_host *host, struct mmc_request *mrq)
>>>> +{
>>>> + init_completion(&mrq->completion);
>>>> + mrq->done = mmc_wait_done;
>>>> + mmc_start_request(host, mrq);
>>>> +}
>>>> +
>>>> +static void mmc_wait_for_req_done(struct mmc_host *host,
>>>> + struct mmc_request *mrq)
>>>> +{
>>>> + wait_for_completion(&mrq->completion);
>>>> +}
>>>> +
>>>> +/**
>>>> + * mmc_pre_req - Prepare for a new request
>>>> + * @host: MMC host to prepare command
>>>> + * @mrq: MMC request to prepare for
>>>> + * @is_first_req: true if there is no previous started request
>>>> + * that may run in parellel to this call, otherwise false
>>>> + *
>>>> + * mmc_pre_req() is called in prior to mmc_start_req() to let
>>>> + * host prepare for the new request. Preparation of a request may be
>>>> + * performed while another request is running on the host.
>>>> + */
>>>> +static void mmc_pre_req(struct mmc_host *host, struct mmc_request *mrq,
>>>> + bool is_first_req)
>>>> +{
>>>> + if (host->ops->pre_req)
>>>> + host->ops->pre_req(host, mrq, is_first_req);
>>>> +}
>>>> +
>>>> +/**
>>>> + * mmc_post_req - Post process a completed request
>>>> + * @host: MMC host to post process command
>>>> + * @mrq: MMC request to post process for
>>>> + * @err: Error, if non zero, clean up any resources made in pre_req
>>>> + *
>>>> + * Let the host post process a completed request. Post processing of
>>>> + * a request may be performed while another reuqest is running.
>>>> + */
>>>> +static void mmc_post_req(struct mmc_host *host, struct mmc_request *mrq,
>>>> + int err)
>>>> +{
>>>> + if (host->ops->post_req)
>>>> + host->ops->post_req(host, mrq, err);
>>>> +}
>>>> +
>>>> +/**
>>>> + * mmc_start_req - start a non-blocking request
>>>> + * @host: MMC host to start command
>>>> + * @areq: async request to start
>>>> + * @error: out parameter returns 0 for success, otherwise non zero
>>>> + *
>>>> + * Start a new MMC custom command request for a host.
>>>> + * If there is on ongoing async request wait for completion
>>>> + * of that request and start the new one and return.
>>>> + * Does not wait for the new request to complete.
>>>> + *
>>>> + * Returns the completed async request, NULL in case of none completed.
>>>> + */
>>>> +struct mmc_async_req *mmc_start_req(struct mmc_host *host,
>>>> + struct mmc_async_req *areq, int *error)
>>>> +{
>>>> + int err = 0;
>>>> + struct mmc_async_req *data = host->areq;
>>>> +
>>>> + /* Prepare a new request */
>>>> + if (areq)
>>>> + mmc_pre_req(host, areq->mrq, !host->areq);
>>>> +
>>>> + if (host->areq) {
>>>> + mmc_wait_for_req_done(host, host->areq->mrq);
>>>> + err = host->areq->err_check(host->card, host->areq);
>>>> + if (err) {
>>>> + mmc_post_req(host, host->areq->mrq, 0);
>>>> + if (areq)
>>>> + mmc_post_req(host, areq->mrq, -EINVAL);
>>>> +
>>>> + host->areq = NULL;
>>>> + goto out;
>>> In this sequence, would the return value (data) have the previous areq ?
>>> Is that intentional - doesn't seem to fit with the description.
>> It will return the data that belongs to the completed request. The
>> completed request will be the same as the previous request. The
>> mmc_start_req will start a new request and return data for the
>> completed request, if any.
>>
>
> I meant that in case of an error (err !=0), data is already assigned
> to host->areq
> and goto out returns 'data'. So my question was 'Does returning a non-null
> pointer for a unsuccessful request doesn't fit with the description, does it ?'
>
Now I get it. Thanks for your patience :)
>>>> + struct mmc_async_req *data = host->areq;
...
>>>> + mmc_wait_for_req_done(host, host->areq->mrq);
>>>> + err = host->areq->err_check(host->card, host->areq);
in case of error host->areq is returned. Basically the failing request
is returned.
block.c needs the failing request in order to do error handling.
>>>> + * @error: out parameter returns 0 for success, otherwise non zero
This parameter indicates error.
Returning NULL is not error. NULL is returned when starting a new
request and there is no ongoing (previous) request.
Regards,
Per
--
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