| Message ID | 1368634865-4186-1-git-send-email-dirk.behme@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2013-05-15 at 18:21 +0200, Dirk Behme wrote: > From: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com> > > UBIfs adapts it‘s structures according to the minimum writeSize reported > by the MTD device. For NOR flash devices, this is normally 1. For NOR > devices with internal hardware-ECC it might be more, such as the S-Die > flash chips of Micron which can use an internal ECC if the chip is > accessed in 32 byte chunks. > > The UBIfs mount process checks and compares the writeSize set in the > image and the writeSize reported from the /dev/mtd . UBIfs will fail > to mount if the values differ. It should, though, not be a problem to > mount an image which was created with a writeSize larger than that of > the MTD, if it is larger by an integer factor. > > This commit changes the check in a way so it will allow the image > writeSize to be larger than that of the MTD by an integer factor. > It will allow to create images and deploy them on different devices > using different writeSizes. writeSize found using values of 1, 8 and 32. > > Signed-off-by: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com> Hi Dirk, first of all, sorry for long delay. In UBIFS we have 2 flash parameters which are relevant to this discussion: 1. min_io_size - minimum size of the I/O buffer, comes from mtd->writesize 2. max_write_size - optimzl size of the I/O buffer, comes from mtd->writebufsize Take a look at io.c. So it looks like what you actually want to do is to make sure you have the right max_write_size, in which case UBIFS will do most of its writes in max_wirte_size units. E.g., make it to be 32. This can be done by teaching your flash driver to export correct 'mtd->writebufsize'. My guess is that you do not do this, and it defaults to 'mtd->writesize'.
Hello. No, I do not want to touch the max_write_size or writebufsize. The max_write_size is the size which can be transferred in one piece to the chip and does not modify the UBIfs data structures. The min size is relevant and can cause images to become incompatible with a specific NOR chip. If the NOR driver should accidentally choose a writesize which is too small, this will not cause any compatibility issues or data loss. It will only reduce write performance. Without the tolerance patch, you cannot mount one UBSfs image on different NOR devices with different writesize values of 1 or 32. Achim. -----Original Message----- From: Artem Bityutskiy [mailto:dedekind1@gmail.com] Sent: Monday, July 01, 2013 9:55 AM To: Dirk Behme Cc: linux-mtd@lists.infradead.org; Behme Dirk (CM-AI/PJ-CF32); Dahlhoff Achim (CM-AI/PJ-CF32) Subject: Re: [PATCH] UBIFS: add tolerance to use variable writesize On Wed, 2013-05-15 at 18:21 +0200, Dirk Behme wrote: > From: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com> > > UBIfs adapts it‘s structures according to the minimum writeSize reported > by the MTD device. For NOR flash devices, this is normally 1. For NOR > devices with internal hardware-ECC it might be more, such as the S-Die > flash chips of Micron which can use an internal ECC if the chip is > accessed in 32 byte chunks. > > The UBIfs mount process checks and compares the writeSize set in the > image and the writeSize reported from the /dev/mtd . UBIfs will fail > to mount if the values differ. It should, though, not be a problem to > mount an image which was created with a writeSize larger than that of > the MTD, if it is larger by an integer factor. > > This commit changes the check in a way so it will allow the image > writeSize to be larger than that of the MTD by an integer factor. > It will allow to create images and deploy them on different devices > using different writeSizes. writeSize found using values of 1, 8 and 32. > > Signed-off-by: Achim Dahlhoff <Achim.Dahlhoff@de.bosch.com> Hi Dirk, first of all, sorry for long delay. In UBIFS we have 2 flash parameters which are relevant to this discussion: 1. min_io_size - minimum size of the I/O buffer, comes from mtd->writesize 2. max_write_size - optimzl size of the I/O buffer, comes from mtd->writebufsize Take a look at io.c. So it looks like what you actually want to do is to make sure you have the right max_write_size, in which case UBIFS will do most of its writes in max_wirte_size units. E.g., make it to be 32. This can be done by teaching your flash driver to export correct 'mtd->writebufsize'. My guess is that you do not do this, and it defaults to 'mtd->writesize'. -- Best Regards, Artem Bityutskiy
On Mon, 2013-07-01 at 10:32 +0200, Dahlhoff Achim (CM-AI/PJ-CF32) wrote: > Hello. > > No, I do not want to touch the max_write_size or writebufsize. The > max_write_size is the size which can be transferred in one piece to > the chip and does not modify the UBIfs data structures. Not sure I understand what you mean. Max_write_size, which is the same as writebufsize, despite the name, is the _optimal_ I/O size. Sometimes UBI/UBIFS _will_ write in smaller chunks. But it will try to write in 'max_write_size'. Sounds like what you need - you flash does allow 1-byte writes, but will be more efficient when writes are 32-bytes instead. > The min size is relevant and can cause images to become incompatible > with a specific NOR chip. If the NOR driver should accidentally choose > a writesize which is too small, this will not cause any compatibility > issues or data loss. It will only reduce write performance. Right, increasing min_io_size breaks compatibility. Although I believe this is fixable, and I already suggested people to fix this several times in the past. But increasing max_write_size should not lead to incompatibilites. > > Without the tolerance patch, you cannot mount one UBSfs image on > different NOR devices with different writesize values of 1 or 32. I still think that you should just change writebufsize in your driver. May be you could find some other view-angle to show my why you think I am wrong?
On 01.07.2013 11:33, Artem Bityutskiy wrote: > On Mon, 2013-07-01 at 10:32 +0200, Dahlhoff Achim (CM-AI/PJ-CF32) wrote: >> Hello. >> >> No, I do not want to touch the max_write_size or writebufsize. The >> max_write_size is the size which can be transferred in one piece to >> the chip and does not modify the UBIfs data structures. > > Not sure I understand what you mean. Max_write_size, which is the same > as writebufsize, despite the name, is the _optimal_ I/O size. Sometimes > UBI/UBIFS _will_ write in smaller chunks. But it will try to write in > 'max_write_size'. > > Sounds like what you need - you flash does allow 1-byte writes, but will > be more efficient when writes are 32-bytes instead. > >> The min size is relevant and can cause images to become incompatible >> with a specific NOR chip. If the NOR driver should accidentally choose >> a writesize which is too small, this will not cause any compatibility >> issues or data loss. It will only reduce write performance. > > Right, increasing min_io_size breaks compatibility. Although I believe > this is fixable, and I already suggested people to fix this several > times in the past. > > But increasing max_write_size should not lead to incompatibilites. >> >> Without the tolerance patch, you cannot mount one UBSfs image on >> different NOR devices with different writesize values of 1 or 32. > > I still think that you should just change writebufsize in your driver. > May be you could find some other view-angle to show my why you think I > am wrong? I'm no expert on this, but let me try to explain what I've understood from Achim: We are talking about UBIFS images which are created on the PC ("mkfs.ubifs" and "ubinize") and then written to real physical NOR devices on the target hardware. Different NOR devices can have different writesize values of 1, 8 or 32. On the target, this is reported from the physical NOR chip and used there for the UBIFS image. But, if the UBIFS image is created on the PC, the PC tools take the writesize as a parameter (--min-io-size) due to the unknown hardware device. This writesize is recorded in the UBIFS created on the PC, e.g. writesize == 1. Now, this UBIFS image created on the PC should be written on the target to a real physical NOR device. This reports e.g. a writesize == 32. While writing the UBIFS image to the NOR device works, mounting it fails. Due to the mismatch in the writesize: The UBIFS image has recored 1, while the physical NOR device reports 32. As technically there is no reason why this shouldn't work, the patch we are discussing here tries to fix that. I hope above is roughly correct ;) Best regards Dirk
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c index 4c37607..69dd794 100644 --- a/fs/ubifs/sb.c +++ b/fs/ubifs/sb.c @@ -351,6 +351,7 @@ static int validate_sb(struct ubifs_info *c, struct ubifs_sb_node *sup) { long long max_bytes; int err = 1, min_leb_cnt; + int chk_tmp; if (!c->key_hash) { err = 2; @@ -362,9 +363,16 @@ static int validate_sb(struct ubifs_info *c, struct ubifs_sb_node *sup) goto failed; } - if (le32_to_cpu(sup->min_io_size) != c->min_io_size) { + /* + * Allow a min_io_size mismatch in the way that + * the size in the superblock (the image) is larger by + * an integer factor. If image-IOsize mod real-IOsize + * is zero, it should be ok to mount this. + */ + chk_tmp = le32_to_cpu(sup->min_io_size); + if (chk_tmp != c->min_io_size && ((chk_tmp%c->min_io_size) != 0)) { ubifs_err("min. I/O unit mismatch: %d in superblock, %d real", - le32_to_cpu(sup->min_io_size), c->min_io_size); + chk_tmp, c->min_io_size); goto failed; }