use case for register_pstore_blk?
Subsystems (3)
Hi WeiXiong, hi Kees,
what is the use case for the code added in commit 17639f67c1d6 ("pstore/blk: Introduce backend for block devices").
This still doesn't have a user, and the API looks really odd to me. By our normal kernel rules we should not add new exports without users and this should probably be reverted for the 5.9 release.
Quoted 6 lines · Christoph Hellwig
Hi WeiXiong, hi Kees,
what is the use case for the code added in commit
17639f67c1d6("pstore/blk: Introduce backend for block devices").This still doesn't have a user, and the API looks really odd to me.
pstore is a beast. :) The API is there so that a blk device can declare its direct support of pstore (specifically, to provide a panic_write handler).
The MTD device does this, but yes, that's a good point, there isn't a blk device user of that entry point yet.
Quoted 2 lines
By our normal kernel rules we should not add new exports without users and this should probably be reverted for the 5.9 release.
I don't want to revert the entire patch (I'm still using __register_pstore_blk by way of pstore/blk's "best_effort" option), but I wouldn't object to something like this:
fs/pstore/blk.c +0 -34
@@ -419,27 +419,6 @@ static int __register_pstore_blk(struct pstore_blk_info *info)return ret; } -/** -* register_pstore_blk() - register block device to pstore/blk -* -* @info: details on the desired block device interface -* -* Return: -* * 0 - OK -* * Others - something error. -*/ -int register_pstore_blk (struct pstore_blk_info *info) -{ -int ret; - -mutex_lock (&pstore_blk_lock); - ret =__register_pstore_blk (info); -mutex_unlock (&pstore_blk_lock); - -return ret; -} -EXPORT_SYMBOL_GPL (register_pstore_blk); -static void __unregister_pstore_blk (unsigned int major) {struct pstore_device_info dev = { .read = psblk_generic_blk_read };
@@ -454,19 +433,6 @@ static void __unregister_pstore_blk(unsigned int major) } } -/** -* unregister_pstore_blk() - unregister block device from pstore/blk -* -* @major: the major device number of device -*/ -void unregister_pstore_blk (unsigned int major) -{ -mutex_lock (&pstore_blk_lock); -__unregister_pstore_blk (major); -mutex_unlock (&pstore_blk_lock); -} -EXPORT_SYMBOL_GPL (unregister_pstore_blk); -/* get information of pstore/blk */ int pstore_blk_get_config (struct pstore_blk_config *info) {
include/linux/pstore_blk.h +0 -3
@@ -46,9 +46,6 @@ struct pstore_blk_info {sector_t start_sect; }; -int register_pstore_blk (struct pstore_blk_info *info); -void unregister_pstore_blk (unsigned int major); -/** * struct pstore_device_info - back-end pstore/blk driver structure. *
Signature · 1 line
Kees Cook
Quoted 15 lines · Kees Cook
Quoted 6 lines · Christoph Hellwig
Hi WeiXiong, hi Kees,
what is the use case for the code added in commit
17639f67c1d6("pstore/blk: Introduce backend for block devices").This still doesn't have a user, and the API looks really odd to me.
pstore is a beast. :) The API is there so that a blk device can declare its direct support of pstore (specifically, to provide a
panic_writehandler).The MTD device does this, but yes, that's a good point, there isn't a blk device user of that entry point yet.
Quoted 6 lines
Quoted 2 lines
By our normal kernel rules we should not add new exports without users and this should probably be reverted for the 5.9 release.
I don't want to revert the entire patch (I'm still using
__register_pstore_blkby way of pstore/blk's "best_effort" option), but I wouldn't object to something like this:
That is a minimum. But the MTD support is also completely and utterly broken as well, by declaring on-stack struct file and struct file_operations instances instead of actually reusing the block layer code. This kind of stuff really needs more coordination with the block layer and maintainers instead of the crap that ended up being merged.
Quoted 15 lines · Kees Cook
Quoted 6 lines · Christoph Hellwig
Hi WeiXiong, hi Kees,
what is the use case for the code added in commit
17639f67c1d6("pstore/blk: Introduce backend for block devices").This still doesn't have a user, and the API looks really odd to me.
pstore is a beast. :) The API is there so that a blk device can declare its direct support of pstore (specifically, to provide a
panic_writehandler).The MTD device does this, but yes, that's a good point, there isn't a blk device user of that entry point yet.
Quoted 6 lines
Quoted 2 lines
By our normal kernel rules we should not add new exports without users and this should probably be reverted for the 5.9 release.
I don't want to revert the entire patch (I'm still using
__register_pstore_blkby way of pstore/blk's "best_effort" option), but I wouldn't object to something like this:
FYI, I'd be much happier with this, which removes the crazy block device autoprobe, and the completely broken block device read/write path as well:
Documentation/admin-guide/pstore-blk.rst +1 -7
@@ -151,13 +151,7 @@ otherwise KMSG_DUMP_MAX. Configurations for driver ------------------------- -Only a block device driver cares about these configurations. A block device -driver uses ``register_pstore_blk`` to register to pstore/blk. - -.. kernel-doc:: fs/pstore/blk.c - :identifiers: register_pstore_blk - -A non-block device driver uses ``register_pstore_device`` with +Device drivers use ``register_pstore_device`` with ``struct pstore_device_info`` to register to pstore/blk. .. kernel-doc:: fs/pstore/blk.c
fs/pstore/blk.c +5 -292
319 lines not on this page · raw message
include/linux/pstore_blk.h +0 -27
@@ -22,33 +22,6 @@typedef int (*pstore_blk_panic_write_op )(const char *buf,sector_t start_sect,sector_t sects); -/** -* struct pstore_blk_info - pstore/blk registration details -* -* @major: Which major device number to support with pstore/blk -* @flags: The supported PSTORE_FLAGS_* from linux/pstore.h. -* @panic_write:The write operation only used for the panic case. -* This can be NULL, but is recommended to avoid losing -* crash data if the kernel's IO path or work queues are -* broken during a panic. -* @devt: The dev_t that pstore/blk has attached to. -* @nr_sects: Number of sectors on @devt. -* @start_sect: Starting sector on @devt. -*/ -struct pstore_blk_info { -unsigned int major; -unsigned int flags; -pstore_blk_panic_write_op panic_write; - -/* Filled in by pstore/blk after registration. */ -dev_t devt; -sector_t nr_sects; -sector_t start_sect; -}; - -int register_pstore_blk (struct pstore_blk_info *info); -void unregister_pstore_blk (unsigned int major); -/** * struct pstore_device_info - back-end pstore/blk driver structure. *
Looking at this more: in addition to the block code being totally broken, there is really no point in mtdpstore even using this code. It does nothing but minimal parameter validation to just pass it on to the pstore zone interface. IMHO writing the mtd code directly to the zone interface makes a whole lot more sense even if we grow a non-broken block backend at some point. Something like this:
Documentation/admin-guide/pstore-blk.rst deleted +0 -243
@@ -1,243 +0,0 @@ -.. SPDX-License-Identifier: GPL-2.0 - -pstore block oops/panic logger -============================== - -Introduction ------------- - -pstore block (pstore/blk) is an oops/panic logger that writes its logs to a -block device and non-block device before the system crashes. You can get -these log files by mounting pstore filesystem like:: - - mount -t pstore pstore /sys/fs/pstore - - -pstore block concepts ---------------------- - -pstore/blk provides efficient configuration method for pstore/blk, which -divides all configurations into two parts, configurations for user and -configurations for driver. - -Configurations for user determine how pstore/blk works, such as pmsg_size, -kmsg_size and so on. All of them support both Kconfig and module parameters, -but module parameters have priority over Kconfig. - -Configurations for driver are all about block device and non-block device, -such as total_size of block device and read/write operations. - -Configurations for user ------------------------ - -All of these configurations support both Kconfig and module parameters, but -module parameters have priority over Kconfig. - -Here is an example for module parameters:: - - pstore_blk.blkdev=179:7 pstore_blk.kmsg_size=64 - -The detail of each configurations may be of interest to you. - -blkdev -~~~~~~ - -The block device to use. Most of the time, it is a partition of block device. -It's required for pstore/blk. It is also used for MTD device. - -It accepts the following variants for block device: - -1. <hex_major><hex_minor> device number in hexadecimal represents itself; no - leading 0x, for example b302. -#. /dev/<disk_name> represents the device number of disk -#. /dev/<disk_name><decimal> represents the device number of partition - device - number of disk plus the partition number -#. /dev/<disk_name>p<decimal> - same as the above; this form is used when disk - name of partitioned disk ends with a digit. -#. PARTUUID=00112233-4455-6677-8899-AABBCCDDEEFF represents the unique id of - a partition if the partition table provides it. The UUID may be either an - EFI/GPT UUID, or refer to an MSDOS partition using the format SSSSSSSS-PP, - where SSSSSSSS is a zero-filled hex representation of the 32-bit - "NT disk signature", and PP is a zero-filled hex representation of the - 1-based partition number. -#. PARTUUID=<UUID>/PARTNROFF=<int> to select a partition in relation to a - partition with a known unique id. -#. <major>:<minor> major and minor number of the device separated by a colon. - -It accepts the following variants for MTD device: - -1. <device name> MTD device name. "pstore" is recommended. -#. <device number> MTD device number. - -kmsg_size -~~~~~~~~~ - -The chunk size in KB for oops/panic front-end. It **MUST** be a multiple of 4. -It's optional if you do not care oops/panic log. - -There are multiple chunks for oops/panic front-end depending on the remaining -space except other pstore front-ends. - -pstore/blk will log to oops/panic chunks one by one, and always overwrite the -oldest chunk if there is no more free chunk. - -pmsg_size -~~~~~~~~~ - -The chunk size in KB for pmsg front-end. It **MUST** be a multiple of 4. -It's optional if you do not care pmsg log. - -Unlike oops/panic front-end, there is only one chunk for pmsg front-end. - -Pmsg is a user space accessible pstore object. Writes to */dev/pmsg0* are -appended to the chunk. On reboot the contents are available in -*/sys/fs/pstore/pmsg-pstore-blk-0*. - -console_size -~~~~~~~~~~~~ - -The chunk size in KB for console front-end. It **MUST** be a multiple of 4. -It's optional if you do not care console log. - -Similar to pmsg front-end, there is only one chunk for console front-end. - -All log of console will be appended to the chunk. On reboot the contents are -available in */sys/fs/pstore/console-pstore-blk-0*. - -ftrace_size -~~~~~~~~~~~ - -The chunk size in KB for ftrace front-end. It **MUST** be a multiple of 4. -It's optional if you do not care console log. - -Similar to oops front-end, there are multiple chunks for ftrace front-end -depending on the count of cpu processors. Each chunk size is equal to -ftrace_size / processors_count. - -All log of ftrace will be appended to the chunk. On reboot the contents are -combined and available in */sys/fs/pstore/ftrace-pstore-blk-0*. - -Persistent function tracing might be useful for debugging software or hardware -related hangs. Here is an example of usage:: - - # mount -t pstore pstore /sys/fs/pstore - # mount -t debugfs debugfs /sys/kernel/debug/ - # echo 1 > /sys/kernel/debug/pstore/record_ftrace - # reboot -f - [...] - # mount -t pstore pstore /sys/fs/pstore - # tail /sys/fs/pstore/ftrace-pstore-blk-0 - CPU:0 ts:5914676 c0063828 c0063b94 call_cpuidle <- cpu_startup_entry+0x1b8/0x1e0 - CPU:0 ts:5914678 c039ecdc c006385c cpuidle_enter_state <- call_cpuidle+0x44/0x48 - CPU:0 ts:5914680 c039e9a0 c039ecf0 cpuidle_enter_freeze <- cpuidle_enter_state+0x304/0x314 - CPU:0 ts:5914681 c0063870 c039ea30 sched_idle_set_state <- cpuidle_enter_state+0x44/0x314 - CPU:1 ts:5916720 c0160f59 c015ee04 kernfs_unmap_bin_file <- __kernfs_remove+0x140/0x204 - CPU:1 ts:5916721 c05ca625 c015ee0c __mutex_lock_slowpath <- __kernfs_remove+0x148/0x204 - CPU:1 ts:5916723 c05c813d c05ca630 yield_to <- __mutex_lock_slowpath+0x314/0x358 - CPU:1 ts:5916724 c05ca2d1 c05ca638 __ww_mutex_lock <- __mutex_lock_slowpath+0x31c/0x358 - -max_reason -~~~~~~~~~~ - -Limiting which kinds of kmsg dumps are stored can be controlled via -the ``max_reason`` value, as defined in include/linux/kmsg_dump.h's -``enum kmsg_dump_reason``. For example, to store both Oopses and Panics, -``max_reason`` should be set to 2 (KMSG_DUMP_OOPS), to store only Panics -``max_reason`` should be set to 1 (KMSG_DUMP_PANIC). Setting this to 0 -(KMSG_DUMP_UNDEF), means the reason filtering will be controlled by the -``printk.always_kmsg_dump`` boot param: if unset, it'll be KMSG_DUMP_OOPS, -otherwise KMSG_DUMP_MAX. - -Configurations for driver -------------------------- - -Only a block device driver cares about these configurations. A block device -driver uses ``register_pstore_blk`` to register to pstore/blk. - -.. kernel-doc:: fs/pstore/blk.c - :identifiers: register_pstore_blk - -A non-block device driver uses ``register_pstore_device`` with -``struct pstore_device_info`` to register to pstore/blk. - -.. kernel-doc:: fs/pstore/blk.c - :identifiers: register_pstore_device - -.. kernel-doc:: include/linux/pstore_blk.h - :identifiers: pstore_device_info - -Compression and header ----------------------- - -Block device is large enough for uncompressed oops data. Actually we do not -recommend data compression because pstore/blk will insert some information into -the first line of oops/panic data. For example:: - - Panic: Total 16 times - -It means that it's OOPS|Panic for the 16th time since the first booting. -Sometimes the number of occurrences of oops|panic since the first booting is -important to judge whether the system is stable. - -The following line is inserted by pstore filesystem. For example:: - - Oops#2 Part1 - -It means that it's OOPS for the 2nd time on the last boot. - -Reading the data ----------------- - -The dump data can be read from the pstore filesystem. The format for these -files is ``dmesg-pstore-blk-[N]`` for oops/panic front-end, -``pmsg-pstore-blk-0`` for pmsg front-end and so on. The timestamp of the -dump file records the trigger time. To delete a stored record from block -device, simply unlink the respective pstore file. - -Attentions in panic read/write APIs ------------------------------------ - -If on panic, the kernel is not going to run for much longer, the tasks will not -be scheduled and most kernel resources will be out of service. It -looks like a single-threaded program running on a single-core computer. - -The following points require special attention for panic read/write APIs: - -1. Can **NOT** allocate any memory. - If you need memory, just allocate while the block driver is initializing - rather than waiting until the panic. -#. Must be polled, **NOT** interrupt driven. - No task schedule any more. The block driver should delay to ensure the write - succeeds, but NOT sleep. -#. Can **NOT** take any lock. - There is no other task, nor any shared resource; you are safe to break all - locks. -#. Just use CPU to transfer. - Do not use DMA to transfer unless you are sure that DMA will not keep lock. -#. Control registers directly. - Please control registers directly rather than use Linux kernel resources. - Do I/O map while initializing rather than wait until a panic occurs. -#. Reset your block device and controller if necessary. - If you are not sure of the state of your block device and controller when - a panic occurs, you are safe to stop and reset them. - -pstore/blk supports psblk_blkdev_info(), which is defined in -*linux/pstore_blk.h*, to get information of using block device, such as the -device number, sector count and start sector of the whole disk. - -pstore block internals ----------------------- - -For developer reference, here are all the important structures and APIs: - -.. kernel-doc:: fs/pstore/zone.c - :internal: - -.. kernel-doc:: include/linux/pstore_zone.h - :internal: - -.. kernel-doc:: fs/pstore/blk.c - :export: - -.. kernel-doc:: include/linux/pstore_blk.h - :internal:
MAINTAINERS +0 -1
@@ -14037,7 +14037,6 @@ M: Tony Luck <[email protected]> S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/pstore F: Documentation/admin-guide/ramoops.rst -F: Documentation/admin-guide/pstore-blk.rst F: Documentation/devicetree/bindings/reserved-memory/ramoops.txt F: drivers/acpi/apei/erst.c F: drivers/firmware/efi/efi-pstore.c
drivers/mtd/Kconfig +34 -1
@@ -172,7 +172,7 @@ config MTD_OOPS config MTD_PSTORE tristate "Log panic/oops to an MTD buffer based on pstore" - depends on PSTORE_BLK + select PSTORE_ZONE help This enables panic and oops messages to be logged to a circular buffer in a flash partition where it can be read back as files after
@@ -180,6 +180,39 @@ config MTD_PSTORE If unsure, say N. +config MTD_PSTORE_DEVICE + string "device identifier" + depends on MTD_PSTORE + default "" + help + Which mtd device should be used for pstore/mtd. + + NOTE that, both Kconfig and module parameters can configure + pstore/mtd, but module parameters have priority over Kconfig. + +config MTD_PSTORE_KMSG_SIZE + int "Size in Kbytes of kmsg dump log to store" + depends on MTD_PSTORE + default 64 + help + This just sets size of kmsg dump (oops, panic, etc) log for + pstore/mtd. The size is in KB and must be a multiple of 4. + + NOTE that, both Kconfig and module parameters can configure + pstore/mtd, but module parameters have priority over Kconfig. + +config MTD_PSTORE_MAX_REASON + int "Maximum kmsg dump reason to store" + depends on MTD_PSTORE + default 2 + help + The maximum reason for kmsg dumps to store. The default is + 2 (KMSG_DUMP_OOPS), see include/linux/kmsg_dump.h's + enum kmsg_dump_reason for more details. + + NOTE that, both Kconfig and module parameters can configure + pstore/mtd, but module parameters have priority over Kconfig. + config MTD_SWAP tristate "Swap on MTD device support" depends on MTD && SWAP
drivers/mtd/mtdpstore.c +90 -46
292 lines not on this page · raw message
fs/pstore/Kconfig +0 -102
105 lines not on this page · raw message
fs/pstore/Makefile +0 -3
@@ -15,6 +15,3 @@ obj-$(CONFIG_PSTORE_RAM) += ramoops.o pstore_zone-objs += zone.o obj-$(CONFIG_PSTORE_ZONE) += pstore_zone.o - -pstore_blk-objs += blk.o -obj-$(CONFIG_PSTORE_BLK) += pstore_blk.o
fs/pstore/blk.c deleted +0 -517
517 lines not on this page · raw message
include/linux/pstore_blk.h deleted +0 -118
118 lines not on this page · raw message
Quoted 6 lines · Christoph Hellwig
Looking at this more: in addition to the block code being totally broken, there is really no point in mtdpstore even using this code. It does nothing but minimal parameter validation to just pass it on to the pstore zone interface. IMHO writing the mtd code directly to the zone interface makes a whole lot more sense even if we grow a non-broken block backend at some point. Something like this:
I really don't like this, sorry. I'm using the pstore/blk bits myself already, and I don't want that removed. Additionally I really don't want the structures open-coded in the MTD driver. The whole point was to encapsulate it. I've spent a lot of time clawing pstore back from the brink of open-coded argument and member explosion. :)
I'm fine to drop the exported register_pstore_blk() API until someone actually uses it, but I want to keep pstore/blk itself and the existing separation between pstore backing devices and pstore storage logic so that configuration happens at the storage level, not the backing device level. My intent, for example, is to migrate ramoops to pstore/zone, etc, and remove all the ramoops-specific configuration which is common to pstore/zone.
Signature · 1 line
Kees Cook
The problem with the block code is that it is completely broken. It uses on-stack structures where it can't, it pokes into internals of the block device read/write path for absolutely not reason, and it uses name_to_dev_t which must not be used in new code.
Or in other words: it is a complete piece of crap full of layering violations that should never have been merged in that form.
It also does not happen to share code with the mtd case.
Quoted 14 lines · Kees Cook
Quoted 6 lines · Christoph Hellwig
Looking at this more: in addition to the block code being totally broken, there is really no point in mtdpstore even using this code. It does nothing but minimal parameter validation to just pass it on to the pstore zone interface. IMHO writing the mtd code directly to the zone interface makes a whole lot more sense even if we grow a non-broken block backend at some point. Something like this:
I really don't like this, sorry. I'm using the pstore/blk bits myself already, and I don't want that removed. Additionally I really don't want the structures open-coded in the MTD driver. The whole point was to encapsulate it. I've spent a lot of time clawing pstore back from the brink of open-coded argument and member explosion. :)
Quoted 10 lines
I'm fine to drop the exported
register_pstore_blk()API until someone actually uses it, but I want to keep pstore/blk itself and the existing separation between pstore backing devices and pstore storage logic so that configuration happens at the storage level, not the backing device level. My intent, for example, is to migrate ramoops to pstore/zone, etc, and remove all the ramoops-specific configuration which is common to pstore/zone.Signature · 1 line
Kees Cook
---end quoted text---
Quoted 1 line · Christoph Hellwig
The problem with the block code is that it is completely broken.
This seems like hyperbole.
Quoted 1 line
It uses on-stack structures where it can't,
Do you mean the ones in pstore_blk_init() and pstore_blk_exit()? Those are fine -- they're what provide the "driverless" module-param loaded "best_effort" target. Do you seem something unsafe about it?
Quoted 2 lines
it pokes into internals of the block device read/write path for absolutely no reason,
Do you mean psblk_generic_blk_read() and psblk_generic_blk_write()? These are for writing to the block device... I'm happy to adjust this if you can show me the better API. (This was being developed in the middle of the iov_iter changes, so perhaps I missed a more appropriate way to do things.)
Quoted 1 line
and it uses
name_to_dev_twhich must not be used in new code.
What?
include/linux/mount.h:
extern dev_t name_to_dev_t(const char *name);
init/do_mounts.c:
/*
* Convert a name into device number. We accept the following
* variants:
...
* If name doesn't have fall into the categories above, we return * (0,0). * block_class is used to check if something is a disk name. If the * disk * name contains slashes, the device name has them replaced with * bangs.
*/dev_t name_to_dev_t(const char *name)
There are no comments about it being deprecated.
And even I had guessed to double-check, there isn't even a hit on lkml about it that I can easily find:
https://lore.kernel.org/lkml/?q=name_to_dev_t+%22new+code%22
https://lore.kernel.org/lkml/?q=name_to_dev_t+deprecated
Where did this happen, where was it documented, and what should be used instead?
Quoted 2 lines
Or in other words: it is a complete piece of crap full of layering violations that should never have been merged in that form.
Gee thanks. I obviously don't agree with you.
Quoted 1 line
It also does not happen to share code with the mtd case.
What? Yes it does: it explicitly uses the pstore/blk configuration callback to get the details configured at boot to identify and configure the backing device. This is specifically designed this way to avoid repeating the mistake of having per-backing-device configuration that is essentially only actually used by the pstore storage layer. i.e. the very thing I'm trying to get away from in ramoops, efi-pstore, etc: storage configuration is tied to the pstore storage layer (i.e. pstore/blk and pstore/zone), not the specific backing device (i.e. MTD, blk, RAM, NVRAM, EFI variables, etc).
So, yes, I think it'd be fine to drop the unused EXPORTs, and I welcome corrections to the generic read/write routines, I very specifically do not want to rip out having a block device as a backing device, nor do I want to revert the configuration management to being backing device specific.
Signature · 1 line
Kees Cook
Quoted 5 lines · Kees Cook
Do you mean
psblk_generic_blk_read()andpsblk_generic_blk_write()? These are for writing to the block device... I'm happy to adjust this if you can show me the better API. (This was being developed in the middle of theiov_iterchanges, so perhaps I missed a more appropriate way to do things.)
The problem is that you use library function for file systems, and then call them on an instance you don't own, and you're feeding crap into them like your own methods.
Now given that as far as I can tell you require 4k aligned reads and writes anyway, there doesn't seem to be any need for this whole page cache dance to start with, and you could just do the completely trivial bios submission. But if for some reason you need to use the page cache, that needs to be done through fs/block_dev.c APIs instead of through the side.
Quoted 7 lines
Quoted 2 lines
and it uses
name_to_dev_twhich must not be used in new code.What?
include/linux/mount.h:
externdev_tname_to_dev_t(const char *name);
It used to be a private API, but then the Chromium folks just exported it in e6e20a7a5f3f49bfee518d5c6849107398d83912 without consulting any relevant maintainer. And now we have this mess :(
Quoted 2 lines
Where did this happen, where was it documented, and what should be used instead?
Use blkdev_get_by_path only. If you look at blkdev_get_by_dev it has this very explicit comment:
* Use it ONLY if you really do not have anything better - i.e. when
* you are behind a truly sucky interface and all you are given is a
* device number. _Never_ to be used for internal purposes. If you
* ever need it - reconsider your API.
Quoted 11 lines
Quoted 1 line
It also does not happen to share code with the mtd case.
What? Yes it does: it explicitly uses the pstore/blk configuration callback to get the details configured at boot to identify and configure the backing device. This is specifically designed this way to avoid repeating the mistake of having per-backing-device configuration that is essentially only actually used by the pstore storage layer. i.e. the very thing I'm trying to get away from in ramoops, efi-pstore, etc: storage configuration is tied to the pstore storage layer (i.e. pstore/blk and pstore/zone), not the specific backing device (i.e. MTD, blk, RAM, NVRAM, EFI variables, etc).
Sharing the config is trivial. But it shared nothing in the actual I/O path, which is entirely different for mtd vs block. And the sad part is that with fs/pstore/zone.c you have added the right abstraction, one that totally makes sense. But only when used by the block and mtd drivers directly. Adding another pointless layer of obsfucation and dead code that doesn't share anything does not help.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/