Re: RFC : mmc: mmc_blk_issue_secdiscard_rq() and mmc_blk_issue_discard_rq()effectively merged into one.

From: NamJae Jeon
Date: Wed Aug 24 2011 - 11:21:11 EST


Hi. Park.

I understand. Thanks for your reply.

2011/8/24 Kyungmin Park <kmpark@xxxxxxxxxxxxx>:
> Hi,
>
> On Wed, Aug 24, 2011 at 2:09 PM, NamJae Jeon <linkinjeon@xxxxxxxxx> wrote:
>> Hi.
>>
>> I am wordering why mmc_blk_issue_secdiscard_rq() and
>> mmc_blk_issue_discard_rq() is seperated.
>> So I try to make mmc_blk_issue_secdiscard_rq() and
>> mmc_blk_issue_discard_rq() effectively merged into one.
>>
>> I want to know your opinion.
>
> I think not special reason. It can merge one function. but note that
> secdiscard function is rarely called. there's only one path from
> BLKSECDISCARD ioctl.
>
> however discard is different. it's called relatively frequently if you
> turn on discard mount option. or often when batched discard.
> So it's helpful to reduce the code size if one function is used. it's
> more helpful to optimize the code flow when discard is used.
>
> Thank you,
> Kyungmin Park
>>
>> Thanks.
>>
>> ----------------------------------------------------------------------------------------------------------------------------------------------------------------------
>> static int mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
>> {
>> Â Â Â Âstruct mmc_blk_data *md = mq->data;
>> Â Â Â Âstruct mmc_card *card = md->queue.card;
>> Â Â Â Âunsigned int from, nr, arg;
>> Â Â Â Âint err = 0;
>> Â Â Â Âfrom = blk_rq_pos(req);
>> Â Â Â Ânr = blk_rq_sectors(req);
>> Â Â Â Âif (req->cmd_flags & REQ_SECURE) {
>> Â Â Â Â Â Â Â Âif (!mmc_can_secure_erase_trim(card)) {
>> Â Â Â Â Â Â Â Â Â Â Â Âerr = -EOPNOTSUPP;
>> Â Â Â Â Â Â Â Â Â Â Â Âgoto out;
>> Â Â Â Â Â Â Â Â}
>> Â Â Â Â Â Â Â Âif(mmc_can_trim(card) &&
>> !mmc_erase_group_aligned(card, from, nr))
>> Â Â Â Â Â Â Â Â Â Â Â Âarg = MMC_SECURE_TRIM1_ARG;
>> Â Â Â Â Â Â Â Âelse
>> Â Â Â Â Â Â Â Â Â Â Â Âarg = MMC_SECURE_ERASE_ARG;
>> Â Â Â Â}
>> Â Â Â Âelse {
>> Â Â Â Â Â Â Â Âif (!mmc_can_erase(card)) {
>> Â Â Â Â Â Â Â Â Â Â Â Âerr = -EOPNOTSUPP;
>> Â Â Â Â Â Â Â Â Â Â Â Âgoto out;
>> Â Â Â Â Â Â Â Â}
>> Â Â Â Â Â Â Â Âif (mmc_can_trim(card))
>> Â Â Â Â Â Â Â Â Â Â Â Âarg = MMC_TRIM_ARG;
>> Â Â Â Â Â Â Â Âelse
>> Â Â Â Â Â Â Â Â Â Â Â Âarg = MMC_ERASE_ARG;
>> Â Â Â Â}
>>
>> Â Â Â Âif (card->quirks & MMC_QUIRK_INAND_CMD38) {
>> Â Â Â Â Â Â Â Âif(req->cmd_flags & REQ_SECURE)
>> Â Â Â Â Â Â Â Â Â Â Â Âerr = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂNAND_CMD38_ARG_EXT_CSD,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âarg == MMC_SECURE_TRIM1_ARG ?
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂINAND_CMD38_ARG_SECTRIM1 :
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂINAND_CMD38_ARG_SECERASE,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â0);
>> Â Â Â Â Â Â Â Âelse
>> Â Â Â Â Â Â Â Â Â Â Â Âerr = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂINAND_CMD38_ARG_EXT_CSD,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âarg == MMC_TRIM_ARG ?
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂINAND_CMD38_ARG_TRIM :
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂINAND_CMD38_ARG_ERASE,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â0);
>> Â Â Â Â Â Â Â Âif (err)
>> Â Â Â Â Â Â Â Â Â Â Â Âgoto out;
>> Â Â Â Â}
>> Â Â Â Âerr = mmc_erase(card, from, nr, arg);
>> Â Â Â Âif (!err && arg == MMC_SECURE_TRIM1_ARG) {
>> Â Â Â Â Â Â Â Âif (card->quirks & MMC_QUIRK_INAND_CMD38) {
>> Â Â Â Â Â Â Â Â Â Â Â Âerr = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â INAND_CMD38_ARG_EXT_CSD,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â INAND_CMD38_ARG_SECTRIM2,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â 0);
>> Â Â Â Â Â Â Â Â Â Â Â Âif (err)
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âgoto out;
>> Â Â Â Â Â Â Â Â}
>> Â Â Â Â Â Â Â Âerr = mmc_erase(card, from, nr, MMC_SECURE_TRIM2_ARG);
>> Â Â Â Â}
>> out:
>> Â Â Â Âspin_lock_irq(&md->lock);
>> Â Â Â Â__blk_end_request(req, err, blk_rq_bytes(req));
>> Â Â Â Âspin_unlock_irq(&md->lock);
>> Â Â Â Âreturn err ? 0 : 1;
>> }
>>
>>
>> static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
>> {
>> Â Â Â Âint ret;
>> Â Â Â Âstruct mmc_blk_data *md = mq->data;
>> Â Â Â Âstruct mmc_card *card = md->queue.card;
>> Â Â Â Âmmc_claim_host(card->host);
>> Â Â Â Âret = mmc_blk_part_switch(card, md);
>> Â Â Â Âif (ret) {
>> Â Â Â Â Â Â Â Âret = 0;
>> Â Â Â Â Â Â Â Âgoto out;
>> Â Â Â Â}
>> Â Â Â Âif (req->cmd_flags & REQ_DISCARD) {
>> Â Â Â Â Â Â Â Âret = mmc_blk_issue_discard_rq(mq, req);
>> Â Â Â Â} else if (req->cmd_flags & REQ_FLUSH) {
>> Â Â Â Â Â Â Â Âret = mmc_blk_issue_flush(mq, req);
>> Â Â Â Â} else {
>> Â Â Â Â Â Â Â Âret = mmc_blk_issue_rw_rq(mq, req);
>> Â Â Â Â}
>> out:
>> Â Â Â Âmmc_release_host(card->host);
>> Â Â Â Âreturn ret;
>> }
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>>
>
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ 0¶ìh®å’i