* RFC : mmc: mmc_blk_issue_secdiscard_rq() and mmc_blk_issue_discard_rq() effectively merged into one.
@ 2011-08-24 5:09 NamJae Jeon
2011-08-24 5:52 ` Kyungmin Park
0 siblings, 1 reply; 4+ messages in thread
From: NamJae Jeon @ 2011-08-24 5:09 UTC (permalink / raw)
To: linux-mmc, linux-kernel; +Cc: Chris Ball, linus.walleij
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.
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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFC : mmc: mmc_blk_issue_secdiscard_rq() and mmc_blk_issue_discard_rq() effectively merged into one.
2011-08-24 5:09 RFC : mmc: mmc_blk_issue_secdiscard_rq() and mmc_blk_issue_discard_rq() effectively merged into one NamJae Jeon
@ 2011-08-24 5:52 ` Kyungmin Park
2011-08-24 15:21 ` NamJae Jeon
0 siblings, 1 reply; 4+ messages in thread
From: Kyungmin Park @ 2011-08-24 5:52 UTC (permalink / raw)
To: NamJae Jeon; +Cc: linux-mmc, linux-kernel, Chris Ball, linus.walleij
Hi,
On Wed, Aug 24, 2011 at 2:09 PM, NamJae Jeon <linkinjeon@gmail.com> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFC : mmc: mmc_blk_issue_secdiscard_rq() and mmc_blk_issue_discard_rq() effectively merged into one.
2011-08-24 5:52 ` Kyungmin Park
@ 2011-08-24 15:21 ` NamJae Jeon
0 siblings, 0 replies; 4+ messages in thread
From: NamJae Jeon @ 2011-08-24 15:21 UTC (permalink / raw)
To: Kyungmin Park; +Cc: linux-mmc, linux-kernel, Chris Ball, linus.walleij
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 5761 bytes --]
Hi. Park.
I understand. Thanks for your reply.
2011/8/24 Kyungmin Park <kmpark@infradead.org>:
> Hi,
>
> On Wed, Aug 24, 2011 at 2:09 PM, NamJae Jeon <linkinjeon@gmail.com> 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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFC : mmc: mmc_blk_issue_secdiscard_rq() and mmc_blk_issue_discard_rq() effectively merged into one.
@ 2011-08-24 15:21 ` NamJae Jeon
0 siblings, 0 replies; 4+ messages in thread
From: NamJae Jeon @ 2011-08-24 15:21 UTC (permalink / raw)
To: Kyungmin Park; +Cc: linux-mmc, linux-kernel, Chris Ball, linus.walleij
Hi. Park.
I understand. Thanks for your reply.
2011/8/24 Kyungmin Park <kmpark@infradead.org>:
> Hi,
>
> On Wed, Aug 24, 2011 at 2:09 PM, NamJae Jeon <linkinjeon@gmail.com> 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@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-08-24 15:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-24 5:09 RFC : mmc: mmc_blk_issue_secdiscard_rq() and mmc_blk_issue_discard_rq() effectively merged into one NamJae Jeon
2011-08-24 5:52 ` Kyungmin Park
2011-08-24 15:21 ` NamJae Jeon
2011-08-24 15:21 ` NamJae Jeon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.