diff options
author | Artem Bityutskiy <Artem.Bityutskiy@nokia.com> | 2007-04-04 06:59:11 -0400 |
---|---|---|
committer | David Woodhouse <dwmw2@infradead.org> | 2007-04-17 14:05:48 -0400 |
commit | 10731f83009e2556f98ffa5c7c2cbffe66dacfb3 (patch) | |
tree | 8b3f3c446e82e06ab1794af7e1c04bf300f3b023 /fs | |
parent | 7f762ab24ca2215b69a1395b5b58877f8282a089 (diff) |
[JFFS2] fix buffer sise calculations in jffs2_get_inode_nodes()
In read inode we have an optimization which prevents one
min. I/O unit (e.g. NAND page) to be read more then once.
Namely, at the beginning we do not know which node type we read,
so we read so we assume we read the directory entry, because it
has the smallest node header. When we read it, we read up to the
next min. I/O unit, just because if later we'll need to read more,
we already have this data.
If it turns out to be that the node is not directory entry, and
we need more data, and we did not read it because it sits in the
next min. I/O unit, we read the whole next (or several next)
min. I/O unit(s). And if it happens to be that we read a data node,
and we've read part of its data, we calculate partial CRC.
So if later we need to check data CRC, we'll only read the rest
of the data from further min. I/O units and continue CRC checking.
This code was a bit messy and buggy. The bug was that it assumed
relatively large min. I/O unit, so that the largest node header
could overlap only one min. I/O unit boundary.
This parch clean-ups the code a bit and fixes this bug.
The patch was not tested on flash with small min. I/O unit, like
NOR-ECC, nut it was tested on NAND with 512 bytes NAND page, so
it at least does not break NAND. It was also tested with mtdram
so it should not break NOR.
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/jffs2/readinode.c | 98 |
1 files changed, 33 insertions, 65 deletions
diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c index 717a48cf7df2..1298848336b8 100644 --- a/fs/jffs2/readinode.c +++ b/fs/jffs2/readinode.c | |||
@@ -421,49 +421,38 @@ static inline int read_unknown(struct jffs2_sb_info *c, struct jffs2_raw_node_re | |||
421 | * negative error code on failure. | 421 | * negative error code on failure. |
422 | */ | 422 | */ |
423 | static int read_more(struct jffs2_sb_info *c, struct jffs2_raw_node_ref *ref, | 423 | static int read_more(struct jffs2_sb_info *c, struct jffs2_raw_node_ref *ref, |
424 | int right_size, int *rdlen, unsigned char *buf, unsigned char *bufstart) | 424 | int needed_len, int *rdlen, unsigned char *buf) |
425 | { | 425 | { |
426 | int right_len, err, len; | 426 | int err, to_read = needed_len - *rdlen; |
427 | size_t retlen; | 427 | size_t retlen; |
428 | uint32_t offs; | 428 | uint32_t offs; |
429 | 429 | ||
430 | if (jffs2_is_writebuffered(c)) { | 430 | if (jffs2_is_writebuffered(c)) { |
431 | right_len = c->wbuf_pagesize - (bufstart - buf); | 431 | int rem = to_read % c->wbuf_pagesize; |
432 | if (right_size + (int)(bufstart - buf) > c->wbuf_pagesize) | ||
433 | right_len += c->wbuf_pagesize; | ||
434 | } else | ||
435 | right_len = right_size; | ||
436 | 432 | ||
437 | if (*rdlen == right_len) | 433 | if (rem) |
438 | return 0; | 434 | to_read += c->wbuf_pagesize - rem; |
435 | } | ||
439 | 436 | ||
440 | /* We need to read more data */ | 437 | /* We need to read more data */ |
441 | offs = ref_offset(ref) + *rdlen; | 438 | offs = ref_offset(ref) + *rdlen; |
442 | if (jffs2_is_writebuffered(c)) { | ||
443 | bufstart = buf + c->wbuf_pagesize; | ||
444 | len = c->wbuf_pagesize; | ||
445 | } else { | ||
446 | bufstart = buf + *rdlen; | ||
447 | len = right_size - *rdlen; | ||
448 | } | ||
449 | 439 | ||
450 | dbg_readinode("read more %d bytes\n", len); | 440 | dbg_readinode("read more %d bytes\n", to_read); |
451 | 441 | ||
452 | err = jffs2_flash_read(c, offs, len, &retlen, bufstart); | 442 | err = jffs2_flash_read(c, offs, to_read, &retlen, buf + *rdlen); |
453 | if (err) { | 443 | if (err) { |
454 | JFFS2_ERROR("can not read %d bytes from 0x%08x, " | 444 | JFFS2_ERROR("can not read %d bytes from 0x%08x, " |
455 | "error code: %d.\n", len, offs, err); | 445 | "error code: %d.\n", to_read, offs, err); |
456 | return err; | 446 | return err; |
457 | } | 447 | } |
458 | 448 | ||
459 | if (retlen < len) { | 449 | if (retlen < to_read) { |
460 | JFFS2_ERROR("short read at %#08x: %zu instead of %d.\n", | 450 | JFFS2_ERROR("short read at %#08x: %zu instead of %d.\n", |
461 | offs, retlen, len); | 451 | offs, retlen, to_read); |
462 | return -EIO; | 452 | return -EIO; |
463 | } | 453 | } |
464 | 454 | ||
465 | *rdlen = right_len; | 455 | *rdlen += to_read; |
466 | |||
467 | return 0; | 456 | return 0; |
468 | } | 457 | } |
469 | 458 | ||
@@ -486,27 +475,9 @@ static int jffs2_get_inode_nodes(struct jffs2_sb_info *c, struct jffs2_inode_inf | |||
486 | 475 | ||
487 | dbg_readinode("ino #%u\n", f->inocache->ino); | 476 | dbg_readinode("ino #%u\n", f->inocache->ino); |
488 | 477 | ||
489 | if (jffs2_is_writebuffered(c)) { | ||
490 | /* | ||
491 | * If we have the write buffer, we assume the minimal I/O unit | ||
492 | * is c->wbuf_pagesize. We implement some optimizations which in | ||
493 | * this case and we need a temporary buffer of size = | ||
494 | * 2*c->wbuf_pagesize bytes (see comments in read_dnode()). | ||
495 | * Basically, we want to read not only the node header, but the | ||
496 | * whole wbuf (NAND page in case of NAND) or 2, if the node | ||
497 | * header overlaps the border between the 2 wbufs. | ||
498 | */ | ||
499 | len = 2*c->wbuf_pagesize; | ||
500 | } else { | ||
501 | /* | ||
502 | * When there is no write buffer, the size of the temporary | ||
503 | * buffer is the size of the larges node header. | ||
504 | */ | ||
505 | len = sizeof(union jffs2_node_union); | ||
506 | } | ||
507 | |||
508 | /* FIXME: in case of NOR and available ->point() this | 478 | /* FIXME: in case of NOR and available ->point() this |
509 | * needs to be fixed. */ | 479 | * needs to be fixed. */ |
480 | len = sizeof(union jffs2_node_union) + c->wbuf_pagesize; | ||
510 | buf = kmalloc(len, GFP_KERNEL); | 481 | buf = kmalloc(len, GFP_KERNEL); |
511 | if (!buf) | 482 | if (!buf) |
512 | return -ENOMEM; | 483 | return -ENOMEM; |
@@ -516,8 +487,6 @@ static int jffs2_get_inode_nodes(struct jffs2_sb_info *c, struct jffs2_inode_inf | |||
516 | if (!valid_ref && f->inocache->ino != 1) | 487 | if (!valid_ref && f->inocache->ino != 1) |
517 | JFFS2_WARNING("Eep. No valid nodes for ino #%u.\n", f->inocache->ino); | 488 | JFFS2_WARNING("Eep. No valid nodes for ino #%u.\n", f->inocache->ino); |
518 | while (valid_ref) { | 489 | while (valid_ref) { |
519 | unsigned char *bufstart; | ||
520 | |||
521 | /* We can hold a pointer to a non-obsolete node without the spinlock, | 490 | /* We can hold a pointer to a non-obsolete node without the spinlock, |
522 | but _obsolete_ nodes may disappear at any time, if the block | 491 | but _obsolete_ nodes may disappear at any time, if the block |
523 | they're in gets erased. So if we mark 'ref' obsolete while we're | 492 | they're in gets erased. So if we mark 'ref' obsolete while we're |
@@ -533,32 +502,31 @@ static int jffs2_get_inode_nodes(struct jffs2_sb_info *c, struct jffs2_inode_inf | |||
533 | /* | 502 | /* |
534 | * At this point we don't know the type of the node we're going | 503 | * At this point we don't know the type of the node we're going |
535 | * to read, so we do not know the size of its header. In order | 504 | * to read, so we do not know the size of its header. In order |
536 | * to minimize the amount of flash IO we assume the node has | 505 | * to minimize the amount of flash IO we assume the header is |
537 | * size = JFFS2_MIN_NODE_HEADER. | 506 | * of size = JFFS2_MIN_NODE_HEADER. |
538 | */ | 507 | */ |
508 | len = JFFS2_MIN_NODE_HEADER; | ||
539 | if (jffs2_is_writebuffered(c)) { | 509 | if (jffs2_is_writebuffered(c)) { |
510 | int end, rem; | ||
511 | |||
540 | /* | 512 | /* |
541 | * We treat 'buf' as 2 adjacent wbufs. We want to | 513 | * We are about to read JFFS2_MIN_NODE_HEADER bytes, |
542 | * adjust bufstart such as it points to the | 514 | * but this flash has some minimal I/O unit. It is |
543 | * beginning of the node within this wbuf. | 515 | * possible that we'll need to read more soon, so read |
516 | * up to the next min. I/O unit, in order not to | ||
517 | * re-read the same min. I/O unit twice. | ||
544 | */ | 518 | */ |
545 | bufstart = buf + (ref_offset(ref) % c->wbuf_pagesize); | 519 | end = ref_offset(ref) + len; |
546 | /* We will read either one wbuf or 2 wbufs. */ | 520 | rem = end % c->wbuf_pagesize; |
547 | len = c->wbuf_pagesize - (bufstart - buf); | 521 | if (rem) |
548 | if (JFFS2_MIN_NODE_HEADER + (int)(bufstart - buf) > c->wbuf_pagesize) { | 522 | end += c->wbuf_pagesize - rem; |
549 | /* The header spans the border of the first wbuf */ | 523 | len = end - ref_offset(ref); |
550 | len += c->wbuf_pagesize; | ||
551 | } | ||
552 | } else { | ||
553 | bufstart = buf; | ||
554 | len = JFFS2_MIN_NODE_HEADER; | ||
555 | } | 524 | } |
556 | 525 | ||
557 | dbg_readinode("read %d bytes at %#08x(%d).\n", len, ref_offset(ref), ref_flags(ref)); | 526 | dbg_readinode("read %d bytes at %#08x(%d).\n", len, ref_offset(ref), ref_flags(ref)); |
558 | 527 | ||
559 | /* FIXME: point() */ | 528 | /* FIXME: point() */ |
560 | err = jffs2_flash_read(c, ref_offset(ref), len, | 529 | err = jffs2_flash_read(c, ref_offset(ref), len, &retlen, buf); |
561 | &retlen, bufstart); | ||
562 | if (err) { | 530 | if (err) { |
563 | JFFS2_ERROR("can not read %d bytes from 0x%08x, " "error code: %d.\n", len, ref_offset(ref), err); | 531 | JFFS2_ERROR("can not read %d bytes from 0x%08x, " "error code: %d.\n", len, ref_offset(ref), err); |
564 | goto free_out; | 532 | goto free_out; |
@@ -570,7 +538,7 @@ static int jffs2_get_inode_nodes(struct jffs2_sb_info *c, struct jffs2_inode_inf | |||
570 | goto free_out; | 538 | goto free_out; |
571 | } | 539 | } |
572 | 540 | ||
573 | node = (union jffs2_node_union *)bufstart; | 541 | node = (union jffs2_node_union *)buf; |
574 | 542 | ||
575 | /* No need to mask in the valid bit; it shouldn't be invalid */ | 543 | /* No need to mask in the valid bit; it shouldn't be invalid */ |
576 | if (je32_to_cpu(node->u.hdr_crc) != crc32(0, node, sizeof(node->u)-4)) { | 544 | if (je32_to_cpu(node->u.hdr_crc) != crc32(0, node, sizeof(node->u)-4)) { |
@@ -596,7 +564,7 @@ static int jffs2_get_inode_nodes(struct jffs2_sb_info *c, struct jffs2_inode_inf | |||
596 | case JFFS2_NODETYPE_DIRENT: | 564 | case JFFS2_NODETYPE_DIRENT: |
597 | 565 | ||
598 | if (JFFS2_MIN_NODE_HEADER < sizeof(struct jffs2_raw_dirent)) { | 566 | if (JFFS2_MIN_NODE_HEADER < sizeof(struct jffs2_raw_dirent)) { |
599 | err = read_more(c, ref, sizeof(struct jffs2_raw_dirent), &len, buf, bufstart); | 567 | err = read_more(c, ref, sizeof(struct jffs2_raw_dirent), &len, buf); |
600 | if (unlikely(err)) | 568 | if (unlikely(err)) |
601 | goto free_out; | 569 | goto free_out; |
602 | } | 570 | } |
@@ -616,7 +584,7 @@ static int jffs2_get_inode_nodes(struct jffs2_sb_info *c, struct jffs2_inode_inf | |||
616 | case JFFS2_NODETYPE_INODE: | 584 | case JFFS2_NODETYPE_INODE: |
617 | 585 | ||
618 | if (JFFS2_MIN_NODE_HEADER < sizeof(struct jffs2_raw_inode)) { | 586 | if (JFFS2_MIN_NODE_HEADER < sizeof(struct jffs2_raw_inode)) { |
619 | err = read_more(c, ref, sizeof(struct jffs2_raw_inode), &len, buf, bufstart); | 587 | err = read_more(c, ref, sizeof(struct jffs2_raw_inode), &len, buf); |
620 | if (unlikely(err)) | 588 | if (unlikely(err)) |
621 | goto free_out; | 589 | goto free_out; |
622 | } | 590 | } |
@@ -635,7 +603,7 @@ static int jffs2_get_inode_nodes(struct jffs2_sb_info *c, struct jffs2_inode_inf | |||
635 | 603 | ||
636 | default: | 604 | default: |
637 | if (JFFS2_MIN_NODE_HEADER < sizeof(struct jffs2_unknown_node)) { | 605 | if (JFFS2_MIN_NODE_HEADER < sizeof(struct jffs2_unknown_node)) { |
638 | err = read_more(c, ref, sizeof(struct jffs2_unknown_node), &len, buf, bufstart); | 606 | err = read_more(c, ref, sizeof(struct jffs2_unknown_node), &len, buf); |
639 | if (unlikely(err)) | 607 | if (unlikely(err)) |
640 | goto free_out; | 608 | goto free_out; |
641 | } | 609 | } |