On 2019-01-08 05:47, Kees Cook wrote: > On Mon, Jan 7, 2019 at 4:01 AM liaoweixiong > <liaoweixiong@xxxxxxxxxxxxxxxxx> wrote: >> >> Why should we need pstore_block? >> 1. Most embedded intelligent equipment have no persistent ram, which >> increases costs. We perfer to cheaper solutions, like block devices. >> In fast, there is already a sample for block device logger in driver >> MTD (drivers/mtd/mtdoops.c). >> 2. Do not any equipment have battery, which means that it lost all data >> on general ram if power failure. Pstore has little to do for these >> equipments. > > I'm catching up from being off over the holidays, but I just wanted to > say that I like the general idea here. Thanks for your work very much! :) > >> [PATCH v5] >> On patch 1: >> 1. rename pstore/rom to pstore/blk > > Thanks! The earlier name seemed wrong. :) > >> 2. Do not allocate any memory in the write path of panic. So, use local >> array instead in function romz_recover_dmesg_meta. >> 3. Add C header file "linux/fs.h" to fix implicit declaration of function >> 'filp_open','kernel_read'... >> On patch 3: >> 1. If panic, do not recover pmsg but flush if it is dirty. >> 2. Fix erase pmsg failed. >> On patch 4: >> 1. Create a document for pstore/blk > > Overall, I wonder if more of the ram backend code could get reused for > the blk backend? I'd much rather be able to share that, since much of > the logic is the same. Before I wrote pstore/blk, I specially studied pstore/ram. As your words, much of the logic is the same, you will see some codes similar to pstore/ram. But there is still something different with pstore/ram. For example, block device maybe unready for read/write before kernel finish initializing. So, pstore/blk recovers old data when mount rather than initialize. In addition, pstore/blk could not be used for pstore-console, since block device much slower than ram. In my original design, pstore/blk works like a manager for storage space, don't care if it is ram or block device. So, it may works for persistent ram too. > > I'll get to a more detailed review in the coming days. Thanks for sending this! > > -Kees > >> >> [PATCH v4] >> On patch 1: >> 1. Fix always true condition '(--i >= 0) => (0-u32max >= 0)' in function >> romz_init_zones by defining variable i to 'int' rahter than >> 'unsigned int'. >> 2. To make codes more easily to read, we use macro READ_NEXT_ZONE for >> return value of romz_dmesg_read if it need to read next zone. >> Moveover, we assign READ_NEXT_ZONE -1024 rather than 0. >> 3. Add 'FLUSH_META' to 'enum romz_flush_mode' and rename 'NOT_FLUSH' to >> 'FLUSH_NONE' >> 4. Function romz_zone_write work badly with FLUSH_PART mode as badly >> address and offset to write. >> On patch 3: >> NEW SUPPORT psmg for pstore_rom. >> >> [PATCH v3] >> On patch 1: >> Fix build as module error for undefined 'vfs_read' and 'vfs_write' >> Both of 'vfs_read' and 'vfs_write' haven't be exproted yet, so we use >> 'kernel_read' and 'kernel_write' instead. >> >> [PATCH v2] >> On patch 1: >> Fix build as module error for redefinition of 'romz_unregister' and >> 'romz_register' >> >> [PATCH v1] >> On patch 1: >> Core codes of pstore_rom, which works well on allwinner(sunxi) platform. >> On patch 2: >> A sample for pstore_rom, using general ram rather than block device. >> >> liaoweixiong (4): >> pstore/blk: new support logger for block devices >> pstore/blk: add sample for pstore_blk >> pstore/blk: support pmsg for pstore block >> Documentation: pstore/blk: create document for pstore_blk >> >> Documentation/admin-guide/pstore-block.rst | 226 ++++++ >> MAINTAINERS | 1 + >> fs/pstore/Kconfig | 20 + >> fs/pstore/Makefile | 5 + >> fs/pstore/blkbuf.c | 46 ++ >> fs/pstore/blkzone.c | 1168 ++++++++++++++++++++++++++++ >> include/linux/pstore_blk.h | 62 ++ >> 7 files changed, 1528 insertions(+) >> create mode 100644 Documentation/admin-guide/pstore-block.rst >> create mode 100644 fs/pstore/blkbuf.c >> create mode 100644 fs/pstore/blkzone.c >> create mode 100644 include/linux/pstore_blk.h >> >> -- >> 1.9.1 >> > >
