From: liaoweixiong <liaoweixiong@allwinnertech.com>
To: Vignesh Raghavendra <vigneshr@ti.com>,
Kees Cook <keescook@chromium.org>,
Anton Vorontsov <anton@enomsg.org>,
Colin Cross <ccross@android.com>, Tony Luck <tony.luck@intel.com>,
Jonathan Corbet <corbet@lwn.net>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Richard Weinberger <richard@nod.at>,
Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Rob Herring <robh@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mtd@lists.infradead.org
Subject: Re: [PATCH v1 11/11] mtd: new support oops logger based on pstore/blk
Date: Thu, 23 Jan 2020 15:03:25 +0800 [thread overview]
Message-ID: <bee57965-6160-0979-68ee-d3841a585df9@allwinnertech.com> (raw)
In-Reply-To: <de3659ad-10bc-f14c-169d-d004c8726316@ti.com>
hi Vignesh Raghavendra,
On 2020/1/23 下午12:24, Vignesh Raghavendra wrote:
> Hi
>
> On 20/01/20 6:33 am, WeiXiong Liao wrote:
> [...]
>> +static inline int mtdpstore_panic_block_isbad(struct mtdpstore_context *cxt,
>> + loff_t off)
>> +{
>> + struct mtd_info *mtd = cxt->mtd;
>> + u64 blknum = div_u64(off, mtd->erasesize);
>> +
>> + return test_bit(blknum, cxt->badmap);
>> +}
>> +
>> +static inline void mtdpstore_mark_used(struct mtdpstore_context *cxt,
>> + loff_t off)
>> +{
>> + u64 zonenum = div_u64(off, cxt->bo_info.dmesg_size);
>> +
>> + pr_debug("mark zone %llu used\n", zonenum);
>
> Please replace pr_*() with dev_*() throughout the patch. Device pointer
> should be available via struct mtd_info
>
OK. I will fix it later. Thank you.
> Regards
> Vignesh
>
>> + set_bit(zonenum, cxt->usedmap);
>> +}
>> +
>> +static inline void mtdpstore_mark_unused(struct mtdpstore_context *cxt,
>> + loff_t off)
>> +{
>> + u64 zonenum = div_u64(off, cxt->bo_info.dmesg_size);
>> +
>> + pr_debug("mark zone %llu unused\n", zonenum);
>> + clear_bit(zonenum, cxt->usedmap);
>> +}
>> +
>> +static inline void mtdpstore_block_mark_unused(struct mtdpstore_context *cxt,
>> + loff_t off)
>> +{
>> + u64 zonenum = div_u64(off, cxt->bo_info.dmesg_size);
>> + u32 zonecnt = cxt->mtd->erasesize / cxt->bo_info.dmesg_size;
>> +
>> + while (zonecnt > 0) {
>> + pr_debug("mark zone %llu unused\n", zonenum);
>> + clear_bit(zonenum, cxt->usedmap);
>> + zonenum++;
>> + zonecnt--;
>> + }
>> +}
>> +
>> +static inline int mtdpstore_is_used(struct mtdpstore_context *cxt, loff_t off)
>> +{
>> + u64 zonenum = div_u64(off, cxt->bo_info.dmesg_size);
>> + u64 blknum = div_u64(off, cxt->mtd->erasesize);
>> +
>> + if (test_bit(blknum, cxt->badmap))
>> + return true;
>> + return test_bit(zonenum, cxt->usedmap);
>> +}
>> +
>> +static int mtdpstore_block_is_used(struct mtdpstore_context *cxt,
>> + loff_t off)
>> +{
>> + u64 zonenum = div_u64(off, cxt->bo_info.dmesg_size);
>> + u32 zonecnt = cxt->mtd->erasesize / cxt->bo_info.dmesg_size;
>> +
>> + while (zonecnt > 0) {
>> + if (test_bit(zonenum, cxt->usedmap))
>> + return true;
>> + zonenum++;
>> + zonecnt--;
>> + }
>> + return false;
>> +}
>> +
>> +static int mtdpstore_is_empty(struct mtdpstore_context *cxt, char *buf,
>> + size_t size)
>> +{
>> + struct mtd_info *mtd = cxt->mtd;
>> + size_t sz;
>> + int i;
>> +
>> + sz = min_t(uint32_t, size, mtd->writesize / 4);
>> + for (i = 0; i < sz; i++) {
>> + if (buf[i] != (char)0xFF)
>> + return false;
>> + }
>> + return true;
>> +}
>> +
>> +static void mtdpstore_mark_removed(struct mtdpstore_context *cxt, loff_t off)
>> +{
>> + u64 zonenum = div_u64(off, cxt->bo_info.dmesg_size);
>> +
>> + pr_debug("mark zone %llu removed\n", zonenum);
>> + set_bit(zonenum, cxt->rmmap);
>> +}
>> +
>> +static void mtdpstore_block_clear_removed(struct mtdpstore_context *cxt,
>> + loff_t off)
>> +{
>> + u64 zonenum = div_u64(off, cxt->bo_info.dmesg_size);
>> + u32 zonecnt = cxt->mtd->erasesize / cxt->bo_info.dmesg_size;
>> +
>> + while (zonecnt > 0) {
>> + clear_bit(zonenum, cxt->rmmap);
>> + zonenum++;
>> + zonecnt--;
>> + }
>> +}
>> +
>> +static int mtdpstore_block_is_removed(struct mtdpstore_context *cxt,
>> + loff_t off)
>> +{
>> + u64 zonenum = div_u64(off, cxt->bo_info.dmesg_size);
>> + u32 zonecnt = cxt->mtd->erasesize / cxt->bo_info.dmesg_size;
>> +
>> + while (zonecnt > 0) {
>> + if (test_bit(zonenum, cxt->rmmap))
>> + return true;
>> + zonenum++;
>> + zonecnt--;
>> + }
>> + return false;
>> +}
>> +
>> +static int mtdpstore_erase_do(struct mtdpstore_context *cxt, loff_t off)
>> +{
>> + struct erase_info erase;
>> + int ret;
>> +
>> + pr_debug("try to erase off 0x%llx\n", off);
>> + erase.len = cxt->mtd->erasesize;
>> + erase.addr = off;
>> + ret = mtd_erase(cxt->mtd, &erase);
>> + if (!ret)
>> + mtdpstore_block_clear_removed(cxt, off);
>> + else
>> + pr_err("erase of region [0x%llx, 0x%llx] on \"%s\" failed\n",
>> + (unsigned long long)erase.addr,
>> + (unsigned long long)erase.len, cxt->bo_info.device);
>> + return ret;
>> +}
>> +
>> +/*
>> + * called while removing file
>> + *
>> + * Avoiding over erasing, do erase only when all zones are removed or unused.
>> + * Ensure to remove when unregister by reading, erasing and wrtiing back.
>> + */
>> +static ssize_t mtdpstore_erase(size_t size, loff_t off)
>> +{
>> + struct mtdpstore_context *cxt = &oops_cxt;
>> +
>> + if (mtdpstore_block_isbad(cxt, off))
>> + return -EIO;
>> +
>> + mtdpstore_mark_unused(cxt, off);
>> +
>> + if (likely(mtdpstore_block_is_used(cxt, off))) {
>> + mtdpstore_mark_removed(cxt, off);
>> + return 0;
>> + }
>> +
>> + /* all zones are unused, erase it */
>> + off = ALIGN_DOWN(off, cxt->mtd->erasesize);
>> + return mtdpstore_erase_do(cxt, off);
>> +}
>> +
>> +/*
>> + * What is securety for mtdpstore?
>> + * As there is no erase for panic case, we should ensure at least one zone
>> + * is writable. Otherwise, panic write will be failed.
>> + * If zone is used, write operation will return -ENEXT, which means that
>> + * pstore/blk will try one by one until get a empty zone. So, it's no need
>> + * to ensure next zone is empty, but at least one.
>> + */
>> +static int mtdpstore_security(struct mtdpstore_context *cxt, loff_t off)
>> +{
>> + int ret = 0, i;
>> + u32 zonenum = (u32)div_u64(off, cxt->bo_info.dmesg_size);
>> + u32 zonecnt = (u32)div_u64(cxt->mtd->size, cxt->bo_info.dmesg_size);
>> + u32 blkcnt = (u32)div_u64(cxt->mtd->size, cxt->mtd->erasesize);
>> + u32 erasesize = cxt->mtd->erasesize;
>> +
>> + for (i = 0; i < zonecnt; i++) {
>> + u32 num = (zonenum + i) % zonecnt;
>> +
>> + /* found empty zone */
>> + if (!test_bit(num, cxt->usedmap))
>> + return 0;
>> + }
>> +
>> + /* If there is no any empty zone, we have no way but to do erase */
>> + off = ALIGN_DOWN(off, erasesize);
>> + while (blkcnt--) {
>> + div64_u64_rem(off + erasesize, cxt->mtd->size, (u64 *)&off);
>> +
>> + if (mtdpstore_block_isbad(cxt, off))
>> + continue;
>> +
>> + ret = mtdpstore_erase_do(cxt, off);
>> + if (!ret) {
>> + mtdpstore_block_mark_unused(cxt, off);
>> + break;
>> + }
>> + }
>> +
>> + if (ret)
>> + pr_err("all blocks bad!\n");
>> + pr_debug("end security\n");
>> + return ret;
>> +}
>> +
>> +static ssize_t mtdpstore_write(const char *buf, size_t size, loff_t off)
>> +{
>> + struct mtdpstore_context *cxt = &oops_cxt;
>> + size_t retlen;
>> + int ret;
>> +
>> + if (mtdpstore_block_isbad(cxt, off))
>> + return -ENEXT;
>> +
>> + /* zone is used, please try next one */
>> + if (mtdpstore_is_used(cxt, off))
>> + return -ENEXT;
>> +
>> + pr_debug("try to write off 0x%llx size %zu\n", off, size);
>> + ret = mtd_write(cxt->mtd, off, size, &retlen, (u_char *)buf);
>> + if (ret < 0 || retlen != size) {
>> + pr_err("write failure at %lld (%zu of %zu written), err %d\n",
>> + off, retlen, size, ret);
>> + return -EIO;
>> + }
>> + mtdpstore_mark_used(cxt, off);
>> +
>> + mtdpstore_security(cxt, off);
>> + return retlen;
>> +}
>> +
>> +/*
>> + * All zones will be read as pstore/blk will read zone one by one when do
>> + * recover.
>> + */
>> +static ssize_t mtdpstore_read(char *buf, size_t size, loff_t off)
>> +{
>> + struct mtdpstore_context *cxt = &oops_cxt;
>> + size_t retlen;
>> + int ret;
>> +
>> + if (mtdpstore_block_isbad(cxt, off))
>> + return -ENEXT;
>> +
>> + pr_debug("try to read off 0x%llx size %zu\n", off, size);
>> + ret = mtd_read(cxt->mtd, off, size, &retlen, (u_char *)buf);
>> + if ((ret < 0 && !mtd_is_bitflip(ret)) || size != retlen) {
>> + pr_err("read failure at %lld (%zu of %zu read), err %d\n",
>> + off, retlen, size, ret);
>> + return -EIO;
>> + }
>> +
>> + if (mtdpstore_is_empty(cxt, buf, size))
>> + mtdpstore_mark_unused(cxt, off);
>> + else
>> + mtdpstore_mark_used(cxt, off);
>> +
>> + mtdpstore_security(cxt, off);
>> + return retlen;
>> +}
>> +
>> +static ssize_t mtdpstore_panic_write(const char *buf, size_t size, loff_t off)
>> +{
>> + struct mtdpstore_context *cxt = &oops_cxt;
>> + size_t retlen;
>> + int ret;
>> +
>> + if (mtdpstore_panic_block_isbad(cxt, off))
>> + return -ENEXT;
>> +
>> + /* zone is used, please try next one */
>> + if (mtdpstore_is_used(cxt, off))
>> + return -ENEXT;
>> +
>> + ret = mtd_panic_write(cxt->mtd, off, size, &retlen, (u_char *)buf);
>> + if (ret < 0 || size != retlen) {
>> + pr_err("panic write failure at %lld (%zu of %zu read), err %d\n",
>> + off, retlen, size, ret);
>> + return -EIO;
>> + }
>> + mtdpstore_mark_used(cxt, off);
>> +
>> + return retlen;
>> +}
>> +
>> +static void mtdpstore_notify_add(struct mtd_info *mtd)
>> +{
>> + int ret;
>> + struct mtdpstore_context *cxt = &oops_cxt;
>> + struct blkoops_info *info = &cxt->bo_info;
>> + unsigned long longcnt;
>> +
>> + if (!strcmp(mtd->name, info->device))
>> + cxt->index = mtd->index;
>> +
>> + if (mtd->index != cxt->index || cxt->index < 0)
>> + return;
>> +
>> + pr_debug("found matching MTD device %s\n", mtd->name);
>> +
>> + if (mtd->size < info->dmesg_size * 2) {
>> + pr_err("MTD partition %d not big enough\n", mtd->index);
>> + return;
>> + }
>> + if (mtd->erasesize < info->dmesg_size) {
>> + pr_err("eraseblock size of MTD partition %d too small\n",
>> + mtd->index);
>> + return;
>> + }
>> + if (unlikely(info->dmesg_size % mtd->writesize)) {
>> + pr_err("record size %lu KB must align to write size %d KB\n",
>> + info->dmesg_size / 1024,
>> + mtd->writesize / 1024);
>> + return;
>> + }
>> + if (unlikely(mtd->size > MTDPSTORE_MAX_MTD_SIZE)) {
>> + pr_err("mtd%d is too large (limit is %d MiB)\n",
>> + mtd->index,
>> + MTDPSTORE_MAX_MTD_SIZE / 1024 / 1024);
>> + return;
>> + }
>> +
>> + longcnt = BITS_TO_LONGS(div_u64(mtd->size, info->dmesg_size));
>> + cxt->rmmap = kcalloc(longcnt, sizeof(long), GFP_KERNEL);
>> + cxt->usedmap = kcalloc(longcnt, sizeof(long), GFP_KERNEL);
>> +
>> + longcnt = BITS_TO_LONGS(div_u64(mtd->size, mtd->erasesize));
>> + cxt->badmap = kcalloc(longcnt, sizeof(long), GFP_KERNEL);
>> +
>> + cxt->bo_dev.total_size = mtd->size;
>> + /* just support dmesg right now */
>> + cxt->bo_dev.flags = BLKOOPS_DEV_SUPPORT_DMESG;
>> + cxt->bo_dev.read = mtdpstore_read;
>> + cxt->bo_dev.write = mtdpstore_write;
>> + cxt->bo_dev.erase = mtdpstore_erase;
>> + cxt->bo_dev.panic_write = mtdpstore_panic_write;
>> +
>> + ret = blkoops_register_device(&cxt->bo_dev);
>> + if (ret) {
>> + pr_err("mtd%d register to blkoops failed\n", mtd->index);
>> + return;
>> + }
>> + cxt->mtd = mtd;
>> + pr_info("Attached to MTD device %d\n", mtd->index);
>> +}
>> +
>> +static int mtdpstore_flush_removed_do(struct mtdpstore_context *cxt,
>> + loff_t off, size_t size)
>> +{
>> + struct mtd_info *mtd = cxt->mtd;
>> + u_char *buf;
>> + int ret;
>> + size_t retlen;
>> + struct erase_info erase;
>> +
>> + buf = kmalloc(mtd->erasesize, GFP_KERNEL);
>> + if (!buf)
>> + return -ENOMEM;
>> +
>> + /* 1st. read to cache */
>> + ret = mtd_read(mtd, off, mtd->erasesize, &retlen, buf);
>> + if (ret || retlen != mtd->erasesize)
>> + goto free;
>> +
>> + /* 2nd. erase block */
>> + erase.len = mtd->erasesize;
>> + erase.addr = off;
>> + ret = mtd_erase(mtd, &erase);
>> + if (ret)
>> + goto free;
>> +
>> + /* 3rd. write back */
>> + while (size) {
>> + unsigned int zonesize = cxt->bo_info.dmesg_size;
>> +
>> + /* remove must clear used bit */
>> + if (mtdpstore_is_used(cxt, off))
>> + mtd_write(mtd, off, zonesize, &retlen, buf);
>> +
>> + off += zonesize;
>> + size -= min_t(unsigned int, zonesize, size);
>> + }
>> +
>> +free:
>> + kfree(buf);
>> + return ret;
>> +}
>> +
>> +static int mtdpstore_flush_removed(struct mtdpstore_context *cxt)
>> +{
>> + struct mtd_info *mtd = cxt->mtd;
>> + int ret;
>> + loff_t off;
>> + u32 blkcnt = (u32)div_u64(mtd->size, mtd->erasesize);
>> +
>> + for (off = 0; blkcnt > 0; blkcnt--, off += mtd->erasesize) {
>> + ret = mtdpstore_block_is_removed(cxt, off);
>> + if (!ret) {
>> + off += mtd->erasesize;
>> + continue;
>> + }
>> +
>> + ret = mtdpstore_flush_removed_do(cxt, off, mtd->erasesize);
>> + if (ret)
>> + return ret;
>> + }
>> + return 0;
>> +}
>> +
>> +static void mtdpstore_notify_remove(struct mtd_info *mtd)
>> +{
>> + struct mtdpstore_context *cxt = &oops_cxt;
>> +
>> + if (mtd->index != cxt->index || cxt->index < 0)
>> + return;
>> +
>> + mtdpstore_flush_removed(cxt);
>> +
>> + blkoops_unregister_device(&cxt->bo_dev);
>> + kfree(cxt->badmap);
>> + kfree(cxt->usedmap);
>> + kfree(cxt->rmmap);
>> + cxt->mtd = NULL;
>> + cxt->index = -1;
>> +}
>> +
>> +static struct mtd_notifier mtdpstore_notifier = {
>> + .add = mtdpstore_notify_add,
>> + .remove = mtdpstore_notify_remove,
>> +};
>> +
>> +static int __init mtdpstore_init(void)
>> +{
>> + int ret;
>> + struct mtdpstore_context *cxt = &oops_cxt;
>> + struct blkoops_info *info = &cxt->bo_info;
>> +
>> + ret = blkoops_info(info);
>> + if (unlikely(ret))
>> + return ret;
>> +
>> + if (strlen(info->device) == 0) {
>> + pr_err("mtd device must be supplied\n");
>> + return -EINVAL;
>> + }
>> + if (!info->dmesg_size) {
>> + pr_err("no recorder enabled\n");
>> + return -EINVAL;
>> + }
>> +
>> + /* Setup the MTD device to use */
>> + ret = kstrtoint((char *)info->device, 0, &cxt->index);
>> + if (ret)
>> + cxt->index = -1;
>> +
>> + register_mtd_user(&mtdpstore_notifier);
>> + return 0;
>> +}
>> +module_init(mtdpstore_init);
>> +
>> +static void __exit mtdpstore_exit(void)
>> +{
>> + unregister_mtd_user(&mtdpstore_notifier);
>> +}
>> +module_exit(mtdpstore_exit);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("WeiXiong Liao <liaoweixiong@allwinnertech.com>");
>> +MODULE_DESCRIPTION("MTD Oops/Panic console logger/driver");
>>
>
next prev parent reply other threads:[~2020-01-23 7:03 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-20 1:03 [PATCH v1 00/11] pstore: support crash log to block and mtd device WeiXiong Liao
2020-01-20 1:03 ` [PATCH v1 01/11] pstore/blk: new support logger for block devices WeiXiong Liao
2020-01-20 1:03 ` [PATCH v1 02/11] blkoops: add blkoops, a warpper for pstore/blk WeiXiong Liao
2020-01-20 1:03 ` [PATCH v1 03/11] pstore/blk: support pmsg recorder WeiXiong Liao
2020-01-20 1:03 ` [PATCH v1 04/11] pstore/blk: blkoops: support console recorder WeiXiong Liao
2020-01-20 1:03 ` [PATCH v1 05/11] pstore/blk: blkoops: support ftrace recorder WeiXiong Liao
2020-01-20 1:03 ` [PATCH v1 06/11] Documentation: pstore/blk: blkoops: create document for pstore_blk WeiXiong Liao
2020-01-21 4:13 ` Randy Dunlap
2020-01-21 5:23 ` liaoweixiong
2020-01-21 6:36 ` Randy Dunlap
2020-01-21 8:19 ` liaoweixiong
2020-01-21 15:34 ` Randy Dunlap
2020-01-22 15:01 ` liaoweixiong
2020-01-22 16:08 ` Randy Dunlap
2020-01-20 1:03 ` [PATCH v1 07/11] pstore/blk: skip broken zone for mtd device WeiXiong Liao
2020-01-20 1:03 ` [PATCH v1 08/11] blkoops: respect for device to pick recorders WeiXiong Liao
2020-01-20 1:03 ` [PATCH v1 09/11] pstore/blk: blkoops: support special removing jobs for dmesg WeiXiong Liao
2020-01-20 1:03 ` [PATCH v1 10/11] blkoops: add interface for dirver to get information of blkoops WeiXiong Liao
2020-01-20 1:03 ` [PATCH v1 11/11] mtd: new support oops logger based on pstore/blk WeiXiong Liao
2020-01-20 10:03 ` Miquel Raynal
2020-01-21 3:36 ` liaoweixiong
2020-01-21 8:48 ` Miquel Raynal
2020-01-22 17:22 ` liaoweixiong
2020-01-22 17:41 ` Miquel Raynal
2020-02-06 13:10 ` liaoweixiong
2020-02-06 15:45 ` Miquel Raynal
2020-02-07 4:13 ` liaoweixiong
2020-02-07 8:41 ` Miquel Raynal
2020-02-07 10:30 ` liaoweixiong
2020-01-23 4:24 ` Vignesh Raghavendra
2020-01-23 7:03 ` liaoweixiong [this message]
2020-02-06 9:13 ` [PATCH v1 00/11] pstore: support crash log to block and mtd device Kees Cook
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=bee57965-6160-0979-68ee-d3841a585df9@allwinnertech.com \
--to=liaoweixiong@allwinnertech.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=anton@enomsg.org \
--cc=ccross@android.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=gregkh@linuxfoundation.org \
--cc=keescook@chromium.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=mchehab+samsung@kernel.org \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--cc=robh@kernel.org \
--cc=tony.luck@intel.com \
--cc=vigneshr@ti.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).