drivers/mtd/mtdoops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
The read buffer size depends on the MTDOOPS_HEADER_SIZE.
Tested: Changed the header size, it doesn't panic, header is still
read/written correctly.
Signed-off-by: Jean-Marc Eurin <jmeurin@google.com>
---
drivers/mtd/mtdoops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 227df24387df..09a26747f490 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -223,7 +223,7 @@ static void find_next_position(struct mtdoops_context *cxt)
{
struct mtd_info *mtd = cxt->mtd;
int ret, page, maxpos = 0;
- u32 count[2], maxcount = 0xffffffff;
+ u32 count[MTDOOPS_HEADER_SIZE/sizeof(u32)], maxcount = 0xffffffff;
size_t retlen;
for (page = 0; page < cxt->oops_pages; page++) {
--
2.35.0.rc2.247.g8bbb082509-googThe current header consists of 2 32-bit values which makes it difficult and error prone to expand. This creates a structure for the header. Changelog since v1: - Create a header structure to simplify code. - Patches in series. - Patch prefix. Jean-Marc Eurin (2): mtd: mtdoops: Fix the size of the header read buffer. mtd: mtdoops: Create a header structure for the saved mtdoops. drivers/mtd/mtdoops.c | 53 +++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 24 deletions(-) -- 2.35.1.1094.g7c7d902a7c-goog
On Wed, Mar 30, 2022 at 11:28 AM Jean-Marc Eurin <jmeurin@google.com> wrote:
>
> The read buffer size depends on the MTDOOPS_HEADER_SIZE.
>
> Tested: Changed the header size, it doesn't panic, header is still
> read/written correctly.
>
> Signed-off-by: Jean-Marc Eurin <jmeurin@google.com>
> ---
> drivers/mtd/mtdoops.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
> index 227df24387df..09a26747f490 100644
> --- a/drivers/mtd/mtdoops.c
> +++ b/drivers/mtd/mtdoops.c
> @@ -223,7 +223,7 @@ static void find_next_position(struct mtdoops_context *cxt)
> {
> struct mtd_info *mtd = cxt->mtd;
> int ret, page, maxpos = 0;
> - u32 count[2], maxcount = 0xffffffff;
> + u32 count[MTDOOPS_HEADER_SIZE/sizeof(u32)], maxcount = 0xffffffff;
> size_t retlen;
>
> for (page = 0; page < cxt->oops_pages; page++) {
> --
> 2.35.0.rc2.247.g8bbb082509-goog
>
Sorry, please ignore this duplicate patch/email.
Jean-Marc
The read buffer size depends on the MTDOOPS_HEADER_SIZE.
Tested: Changed the header size, it doesn't panic, header is still
read/written correctly.
Signed-off-by: Jean-Marc Eurin <jmeurin@google.com>
---
drivers/mtd/mtdoops.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 227df24387df..09a26747f490 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -223,7 +223,7 @@ static void find_next_position(struct mtdoops_context *cxt)
{
struct mtd_info *mtd = cxt->mtd;
int ret, page, maxpos = 0;
- u32 count[2], maxcount = 0xffffffff;
+ u32 count[MTDOOPS_HEADER_SIZE/sizeof(u32)], maxcount = 0xffffffff;
size_t retlen;
for (page = 0; page < cxt->oops_pages; page++) {
--
2.35.1.1094.g7c7d902a7c-googCreate a dump header to enable the addition of fields without having
to modify the rest of the code.
Signed-off-by: Jean-Marc Eurin <jmeurin@google.com>
---
drivers/mtd/mtdoops.c | 53 +++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 24 deletions(-)
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 09a26747f490..9ca82c2dbf60 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -22,9 +22,6 @@
/* Maximum MTD partition size */
#define MTDOOPS_MAX_MTD_SIZE (8 * 1024 * 1024)
-#define MTDOOPS_KERNMSG_MAGIC 0x5d005d00
-#define MTDOOPS_HEADER_SIZE 8
-
static unsigned long record_size = 4096;
module_param(record_size, ulong, 0400);
MODULE_PARM_DESC(record_size,
@@ -40,6 +37,13 @@ module_param(dump_oops, int, 0600);
MODULE_PARM_DESC(dump_oops,
"set to 1 to dump oopses, 0 to only dump panics (default 1)");
+#define MTDOOPS_KERNMSG_MAGIC 0x5d005d00
+
+struct mtdoops_hdr {
+ u32 seq;
+ u32 magic;
+} __packed;
+
static struct mtdoops_context {
struct kmsg_dumper dump;
@@ -178,16 +182,16 @@ static void mtdoops_write(struct mtdoops_context *cxt, int panic)
{
struct mtd_info *mtd = cxt->mtd;
size_t retlen;
- u32 *hdr;
+ struct mtdoops_hdr *hdr;
int ret;
if (test_and_set_bit(0, &cxt->oops_buf_busy))
return;
/* Add mtdoops header to the buffer */
- hdr = cxt->oops_buf;
- hdr[0] = cxt->nextcount;
- hdr[1] = MTDOOPS_KERNMSG_MAGIC;
+ hdr = (struct mtdoops_hdr *)cxt->oops_buf;
+ hdr->seq = cxt->nextcount;
+ hdr->magic = MTDOOPS_KERNMSG_MAGIC;
if (panic) {
ret = mtd_panic_write(mtd, cxt->nextpage * record_size,
@@ -222,8 +226,9 @@ static void mtdoops_workfunc_write(struct work_struct *work)
static void find_next_position(struct mtdoops_context *cxt)
{
struct mtd_info *mtd = cxt->mtd;
+ struct mtdoops_hdr hdr;
int ret, page, maxpos = 0;
- u32 count[MTDOOPS_HEADER_SIZE/sizeof(u32)], maxcount = 0xffffffff;
+ u32 maxcount = 0xffffffff;
size_t retlen;
for (page = 0; page < cxt->oops_pages; page++) {
@@ -231,32 +236,31 @@ static void find_next_position(struct mtdoops_context *cxt)
continue;
/* Assume the page is used */
mark_page_used(cxt, page);
- ret = mtd_read(mtd, page * record_size, MTDOOPS_HEADER_SIZE,
- &retlen, (u_char *)&count[0]);
- if (retlen != MTDOOPS_HEADER_SIZE ||
+ ret = mtd_read(mtd, page * record_size, sizeof(hdr),
+ &retlen, (u_char *)&hdr);
+ if (retlen != sizeof(hdr) ||
(ret < 0 && !mtd_is_bitflip(ret))) {
printk(KERN_ERR "mtdoops: read failure at %ld (%td of %d read), err %d\n",
- page * record_size, retlen,
- MTDOOPS_HEADER_SIZE, ret);
+ page * record_size, retlen, sizeof(hdr), ret);
continue;
}
- if (count[0] == 0xffffffff && count[1] == 0xffffffff)
+ if (hdr.seq == 0xffffffff && hdr.magic == 0xffffffff)
mark_page_unused(cxt, page);
- if (count[0] == 0xffffffff || count[1] != MTDOOPS_KERNMSG_MAGIC)
+ if (hdr.seq == 0xffffffff || hdr.magic != MTDOOPS_KERNMSG_MAGIC)
continue;
if (maxcount == 0xffffffff) {
- maxcount = count[0];
+ maxcount = hdr.seq;
maxpos = page;
- } else if (count[0] < 0x40000000 && maxcount > 0xc0000000) {
- maxcount = count[0];
+ } else if (hdr.seq < 0x40000000 && maxcount > 0xc0000000) {
+ maxcount = hdr.seq;
maxpos = page;
- } else if (count[0] > maxcount && count[0] < 0xc0000000) {
- maxcount = count[0];
+ } else if (hdr.seq > maxcount && hdr.seq < 0xc0000000) {
+ maxcount = hdr.seq;
maxpos = page;
- } else if (count[0] > maxcount && count[0] > 0xc0000000
+ } else if (hdr.seq > maxcount && hdr.seq > 0xc0000000
&& maxcount > 0x80000000) {
- maxcount = count[0];
+ maxcount = hdr.seq;
maxpos = page;
}
}
@@ -287,8 +291,9 @@ static void mtdoops_do_dump(struct kmsg_dumper *dumper,
if (test_and_set_bit(0, &cxt->oops_buf_busy))
return;
- kmsg_dump_get_buffer(&iter, true, cxt->oops_buf + MTDOOPS_HEADER_SIZE,
- record_size - MTDOOPS_HEADER_SIZE, NULL);
+ kmsg_dump_get_buffer(&iter, true,
+ cxt->oops_buf + sizeof(struct mtdoops_hdr),
+ record_size - sizeof(struct mtdoops_hdr), NULL);
clear_bit(0, &cxt->oops_buf_busy);
if (reason != KMSG_DUMP_OOPS) {
--
2.35.1.1094.g7c7d902a7c-googHi Jean-Marc,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mtd/mtd/next]
[also build test WARNING on mtd/mtd/fixes linux/master linus/master v5.17 next-20220330]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Jean-Marc-Eurin/mtd-mtdoops-Structure-the-header-of-the-dumped-oops/20220331-023808
base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next
config: x86_64-randconfig-a015 (https://download.01.org/0day-ci/archive/20220331/202203310648.it4f2xXD-lkp@intel.com/config)
compiler: gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/0d39801219fd826554caf69402424346799810d5
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jean-Marc-Eurin/mtd-mtdoops-Structure-the-header-of-the-dumped-oops/20220331-023808
git checkout 0d39801219fd826554caf69402424346799810d5
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/mtd/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:29,
from drivers/mtd/mtdoops.c:10:
drivers/mtd/mtdoops.c: In function 'find_next_position':
>> include/linux/kern_levels.h:5:18: warning: format '%d' expects argument of type 'int', but argument 4 has type 'long unsigned int' [-Wformat=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
include/linux/printk.h:418:11: note: in definition of macro 'printk_index_wrap'
418 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~
drivers/mtd/mtdoops.c:243:4: note: in expansion of macro 'printk'
243 | printk(KERN_ERR "mtdoops: read failure at %ld (%td of %d read), err %d\n",
| ^~~~~~
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
11 | #define KERN_ERR KERN_SOH "3" /* error conditions */
| ^~~~~~~~
drivers/mtd/mtdoops.c:243:11: note: in expansion of macro 'KERN_ERR'
243 | printk(KERN_ERR "mtdoops: read failure at %ld (%td of %d read), err %d\n",
| ^~~~~~~~
drivers/mtd/mtdoops.c:243:59: note: format string is defined here
243 | printk(KERN_ERR "mtdoops: read failure at %ld (%td of %d read), err %d\n",
| ~^
| |
| int
| %ld
vim +5 include/linux/kern_levels.h
314ba3520e513a Joe Perches 2012-07-30 4
04d2c8c83d0e3a Joe Perches 2012-07-30 @5 #define KERN_SOH "\001" /* ASCII Start Of Header */
04d2c8c83d0e3a Joe Perches 2012-07-30 6 #define KERN_SOH_ASCII '\001'
04d2c8c83d0e3a Joe Perches 2012-07-30 7
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Jean-Marc,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mtd/mtd/next]
[also build test WARNING on mtd/mtd/fixes linux/master linus/master v5.17 next-20220330]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Jean-Marc-Eurin/mtd-mtdoops-Structure-the-header-of-the-dumped-oops/20220331-023808
base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next
config: x86_64-randconfig-a014 (https://download.01.org/0day-ci/archive/20220331/202203310819.kOiVSg8W-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0f6d9501cf49ce02937099350d08f20c4af86f3d)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/0d39801219fd826554caf69402424346799810d5
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jean-Marc-Eurin/mtd-mtdoops-Structure-the-header-of-the-dumped-oops/20220331-023808
git checkout 0d39801219fd826554caf69402424346799810d5
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/mtd/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/mtd/mtdoops.c:244:39: warning: format specifies type 'int' but the argument has type 'unsigned long' [-Wformat]
page * record_size, retlen, sizeof(hdr), ret);
^~~~~~~~~~~
include/linux/printk.h:446:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/printk.h:418:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
~~~~ ^~~~~~~~~~~
1 warning generated.
vim +244 drivers/mtd/mtdoops.c
225
226 static void find_next_position(struct mtdoops_context *cxt)
227 {
228 struct mtd_info *mtd = cxt->mtd;
229 struct mtdoops_hdr hdr;
230 int ret, page, maxpos = 0;
231 u32 maxcount = 0xffffffff;
232 size_t retlen;
233
234 for (page = 0; page < cxt->oops_pages; page++) {
235 if (mtd_block_isbad(mtd, page * record_size))
236 continue;
237 /* Assume the page is used */
238 mark_page_used(cxt, page);
239 ret = mtd_read(mtd, page * record_size, sizeof(hdr),
240 &retlen, (u_char *)&hdr);
241 if (retlen != sizeof(hdr) ||
242 (ret < 0 && !mtd_is_bitflip(ret))) {
243 printk(KERN_ERR "mtdoops: read failure at %ld (%td of %d read), err %d\n",
> 244 page * record_size, retlen, sizeof(hdr), ret);
245 continue;
246 }
247
248 if (hdr.seq == 0xffffffff && hdr.magic == 0xffffffff)
249 mark_page_unused(cxt, page);
250 if (hdr.seq == 0xffffffff || hdr.magic != MTDOOPS_KERNMSG_MAGIC)
251 continue;
252 if (maxcount == 0xffffffff) {
253 maxcount = hdr.seq;
254 maxpos = page;
255 } else if (hdr.seq < 0x40000000 && maxcount > 0xc0000000) {
256 maxcount = hdr.seq;
257 maxpos = page;
258 } else if (hdr.seq > maxcount && hdr.seq < 0xc0000000) {
259 maxcount = hdr.seq;
260 maxpos = page;
261 } else if (hdr.seq > maxcount && hdr.seq > 0xc0000000
262 && maxcount > 0x80000000) {
263 maxcount = hdr.seq;
264 maxpos = page;
265 }
266 }
267 if (maxcount == 0xffffffff) {
268 cxt->nextpage = cxt->oops_pages - 1;
269 cxt->nextcount = 0;
270 }
271 else {
272 cxt->nextpage = maxpos;
273 cxt->nextcount = maxcount;
274 }
275
276 mtdoops_inc_counter(cxt);
277 }
278
--
0-DAY CI Kernel Test Service
https://01.org/lkp
© 2016 - 2026 Red Hat, Inc.