From: Namjae Jeon <linkinjeon@kernel.org>
To: "Kohada.Tetsuhiro@dc.MitsubishiElectric.co.jp"
<Kohada.Tetsuhiro@dc.mitsubishielectric.co.jp>
Cc: Sungjong Seo <sj1557.seo@samsung.com>,
"Mori.Takahiro@ab.MitsubishiElectric.co.jp"
<Mori.Takahiro@ab.mitsubishielectric.co.jp>,
"Motai.Hirotaka@aj.MitsubishiElectric.co.jp"
<Motai.Hirotaka@aj.mitsubishielectric.co.jp>,
Namjae Jeon <namjae.jeon@samsung.com>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"kohada.t2@gmail.com" <kohada.t2@gmail.com>
Subject: Re: [PATCH] exfat: optimize dir-cache
Date: Wed, 27 May 2020 20:29:52 +0900 [thread overview]
Message-ID: <CAKYAXd_oG6dc7CNiHszKmhabHd2zrN_VOaNYaWRPES=7hRu+pA@mail.gmail.com> (raw)
In-Reply-To: <TY1PR01MB15784E70CEACDA05F688AE6790B10@TY1PR01MB1578.jpnprd01.prod.outlook.com>
2020-05-27 17:00 GMT+09:00,
Kohada.Tetsuhiro@dc.MitsubishiElectric.co.jp
<Kohada.Tetsuhiro@dc.mitsubishielectric.co.jp>:
> Thank you for your comment.
>
> >> + for (i = 0; i < es->num_bh; i++) {
> >> + if (es->modified)
> >> + exfat_update_bh(es->sb, es->bh[i], sync);
> >
> > Overall, it looks good to me.
> > However, if "sync" is set, it looks better to return the result of
> exfat_update_bh().
> > Of course, a tiny modification for exfat_update_bh() is also required.
>
> I thought the same, while creating this patch.
> However this patch has changed a lot and I didn't add any new error
> checking.
> (So, the same behavior will occur even if an error occurs)
>
> >> +struct exfat_dentry *exfat_get_dentry_cached(
> >> + struct exfat_entry_set_cache *es, int num) {
> >> + int off = es->start_off + num * DENTRY_SIZE;
> >> + struct buffer_head *bh = es->bh[EXFAT_B_TO_BLK(off, es->sb)];
> >> + char *p = bh->b_data + EXFAT_BLK_OFFSET(off, es->sb);
> >
> > In order to prevent illegal accesses to bh and dentries, it would be
> better to check validation for num and bh.
>
> There is no new error checking for same reason as above.
>
> I'll try to add error checking to this v2 patch.
> Or is it better to add error checking in another patch?
The latter:)
Thanks!
>
> BR
> ---
> Kohada Tetsuhiro <Kohada.Tetsuhiro@dc.MitsubishiElectric.co.jp>
next prev parent reply other threads:[~2020-05-27 11:29 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20200520075735epcas1p269372d222e25f3fd51b7979f5b7cdc61@epcas1p2.samsung.com>
2020-05-20 7:56 ` [PATCH] exfat: optimize dir-cache Tetsuhiro Kohada
2020-05-26 2:36 ` Sungjong Seo
2020-05-27 8:00 ` Kohada.Tetsuhiro
2020-05-27 11:29 ` Namjae Jeon [this message]
2020-05-27 14:25 ` Sungjong Seo
2020-05-28 0:12 ` Tetsuhiro Kohada
2020-05-28 5:05 ` Namjae Jeon
2020-05-28 4:42 ` Namjae Jeon
2020-06-01 12:08 ` Sungjong Seo
2020-06-02 6:33 ` Namjae Jeon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKYAXd_oG6dc7CNiHszKmhabHd2zrN_VOaNYaWRPES=7hRu+pA@mail.gmail.com' \
--to=linkinjeon@kernel.org \
--cc=Kohada.Tetsuhiro@dc.mitsubishielectric.co.jp \
--cc=Mori.Takahiro@ab.mitsubishielectric.co.jp \
--cc=Motai.Hirotaka@aj.mitsubishielectric.co.jp \
--cc=kohada.t2@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=namjae.jeon@samsung.com \
--cc=sj1557.seo@samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).