>
> @@ -1262,21 +1608,32 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
> *mq, struct request *rqc)
> int ret = 1, disable_multi = 0, retry = 0, type;
> enum mmc_blk_status status;
> struct mmc_queue_req *mq_rq;
> - struct request *req;
> + struct request *req, *prq;
> struct mmc_async_req *areq;
> + u8 reqs = 0;
>
> if (!rqc && !mq->mqrq_prev->req)
> return 0;
>
> + if (rqc)
> + reqs = mmc_blk_prep_packed_list(mq, rqc);
> +
> do {
> if (rqc) {
> - mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
> + if (reqs >= card->host->packed_min)
In case host->packed_min will be set to a value bigger than 2 you will
loose all the requests that were added to the packed list. If you want to
support dynamic number of min packed requests you need to move the packed
list preparation to queue.c where you can issue the fetched requests one
after another, when (reqs < card->host->packed_min).
> + mmc_blk_packed_hdr_wrq_prep(mq->mqrq_cur, card, mq, reqs);
> + else
> + mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
Thanks,
Maya Erez
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
2012/2/21 <[email protected]>:
>>
>> @@ -1262,21 +1608,32 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>> *mq, struct request *rqc)
>> int ret = 1, disable_multi = 0, retry = 0, type;
>> enum mmc_blk_status status;
>> struct mmc_queue_req *mq_rq;
>> - struct request *req;
>> + struct request *req, *prq;
>> struct mmc_async_req *areq;
>> + u8 reqs = 0;
>>
>> if (!rqc && !mq->mqrq_prev->req)
>> return 0;
>>
>> + if (rqc)
>> + reqs = mmc_blk_prep_packed_list(mq, rqc);
>> +
>> do {
>> if (rqc) {
>> - mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
>> + if (reqs >= card->host->packed_min)
> In case host->packed_min will be set to a value bigger than 2 you will
> loose all the requests that were added to the packed list. If you want to
> support dynamic number of min packed requests you need to move the packed
> list preparation to queue.c where you can issue the fetched requests one
> after another, when (reqs < card->host->packed_min).
I don't understand why packed list preparation should be moved to
queue.c for dynamic number of packed min.
we can change packed min value via sysfs without compiling.
Would you explain more ?
>> + mmc_blk_packed_hdr_wrq_prep(mq->mqrq_cur, card, mq, reqs);
>> + else
>> + mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
>
> Thanks,
> Maya Erez
> Consultant for Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Maya Erez <[email protected]> wrote:
> > @@ -1262,21 +1608,32 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
> > *mq, struct request *rqc)
> > int ret = 1, disable_multi = 0, retry = 0, type;
> > enum mmc_blk_status status;
> > struct mmc_queue_req *mq_rq;
> > - struct request *req;
> > + struct request *req, *prq;
> > struct mmc_async_req *areq;
> > + u8 reqs = 0;
> >
> > if (!rqc && !mq->mqrq_prev->req)
> > return 0;
> >
> > + if (rqc)
> > + reqs = mmc_blk_prep_packed_list(mq, rqc);
> > +
> > do {
> > if (rqc) {
> > - mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
> > + if (reqs >= card->host->packed_min)
> In case host->packed_min will be set to a value bigger than 2 you will
> loose all the requests that were added to the packed list. If you want to
> support dynamic number of min packed requests you need to move the packed
> list preparation to queue.c where you can issue the fetched requests one
> after another, when (reqs < card->host->packed_min).
That's a good point.
The requests added to the packed list will be remained
if current number of packed request is less than packed_min.
In such a case, requests should be put back to request_queue from packed list.
I think it's a additional and unnecessary work due to packed_min.
It's not interesting. So I want to revert this regulation of packed_min.
And we have discussed the location of mmc_blk_prep_packed_list(block.c or queue.c) several times.
Packed command is valid only if request type is read/write,
so it may be a good location to decide in mmc_blk_issue_rw_rq in block.c.
I guess you want this function to be located in mmc_queue_thread in queue.c, right?
Is it advantageous? Could you explain for this?
Thanks,
Seungwon Jeon
> > + mmc_blk_packed_hdr_wrq_prep(mq->mqrq_cur, card, mq, reqs);
> > + else
> > + mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
>
> Thanks,
> Maya Erez
> Consultant for Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Maya Erez <[email protected]> wrote:
>> > @@ -1262,21 +1608,32 @@ static int mmc_blk_issue_rw_rq(struct
>> mmc_queue
>> > *mq, struct request *rqc)
>> > int ret = 1, disable_multi = 0, retry = 0, type;
>> > enum mmc_blk_status status;
>> > struct mmc_queue_req *mq_rq;
>> > - struct request *req;
>> > + struct request *req, *prq;
>> > struct mmc_async_req *areq;
>> > + u8 reqs = 0;
>> >
>> > if (!rqc && !mq->mqrq_prev->req)
>> > return 0;
>> >
>> > + if (rqc)
>> > + reqs = mmc_blk_prep_packed_list(mq, rqc);
>> > +
>> > do {
>> > if (rqc) {
>> > - mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
>> > + if (reqs >= card->host->packed_min)
>> In case host->packed_min will be set to a value bigger than 2 you will
>> loose all the requests that were added to the packed list. If you want
>> to
>> support dynamic number of min packed requests you need to move the
>> packed
>> list preparation to queue.c where you can issue the fetched requests one
>> after another, when (reqs < card->host->packed_min).
>
> That's a good point.
> The requests added to the packed list will be remained
> if current number of packed request is less than packed_min.
> In such a case, requests should be put back to request_queue from packed
> list.
> I think it's a additional and unnecessary work due to packed_min.
> It's not interesting. So I want to revert this regulation of packed_min.
>
> And we have discussed the location of mmc_blk_prep_packed_list(block.c or
> queue.c) several times.
> Packed command is valid only if request type is read/write,
> so it may be a good location to decide in mmc_blk_issue_rw_rq in block.c.
> I guess you want this function to be located in mmc_queue_thread in
> queue.c, right?
> Is it advantageous? Could you explain for this?
>
> Thanks,
> Seungwon Jeon
Regarding moving the code to queue.c, I think that if we decide to keep
packed_min it will be more efficient to send the fetched requests one
after another instead of requeueing them and fetch them again next time
(which can lead to long fetch->requeue sequences).
I agree we can remove the packed_min and use a constant of 2 instead. It
will make the code much more complex and I don't see a real need for it.
>> > + mmc_blk_packed_hdr_wrq_prep(mq->mqrq_cur, card, mq, reqs);
>> > + else
>> > + mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
>>
>> Thanks,
>> Maya Erez
>> Consultant for Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>