Re: [PATCH v6 0/5] Micron SLC NAND filling block
On Tue, 02 Jun 2020 10:59:46 +0200 Bean Huo wrote: > On Tue, 2020-06-02 at 09:48 +0200, Boris Brezillon wrote: > > Hi Bean, > > > > On Mon, 01 Jun 2020 23:10:43 +0200 > > Bean Huo wrote: > > > > > Hi Richard > > > would you please help us confirm below question?? > > > > Miquel suggested an approach that would allow us to deal with both > > JFFS2 > > and UBI/UBIFS without having any FS/wear-leveling specific code at > > the > > NAND level, but you decided to ignore his comments. Sorry but there's > > nothing we can do to help you if you don't listen to our > > recommendations. > > Expose this issue to FS layer, it is not good idea. that will impact > more code, and involve duplicated code. Sorry but as far as I'm concerned, you've lost the right to have your word in such design choices a long time ago. You can't deliberately lie to us for several weeks/months and expect us to trust you (your judgment) after that. Back to the actual proposal, it's something that came from a discussion we had with Miquel and Richard. It's certainly not perfect, but neither is the option of hardcoding a quirk for JFFS2/UBI/UBIFS in the Micron NAND driver. BTW, I think you completely occluded Miquel's suggestion to have a generic implementation at the MTD level for users who don't care about the pattern that's written to those 'soon-to-be-erased' blocks. See, that's one of the things I'm complaining about. You seem to ignore (don't know if it's deliberate or not) some of the suggestions we do. > > > > I've been quite disappointed by your behavior in the past, and it > > > continues. Recently you've taken Miquel's patches and claimed > > ownership > did you seem my recent patch? you can ignore that see this. I don't understand what you mean here, sorry. > > > > on them (probably not intentionally, but still) while you were > > clearly > > unable to rework your original series the way I suggested (which > > Miquel > > did after seeing you would never send new versions). > > seriously? Yes, seriously! > > > And when Miquel > > suggested a change to the implementation he had done based on the > > discussion we had with Richard, you decided to ignore it and pursue > > in > > the original direction. So, quite frankly, I'm really not convinced > > you > > can conduct such a change. > > > > As Miquel mentioned, we need richard's final comfirmation, > If he agrees with this proposal, I give up my current patch. > Actually, you need more than Richard's blessing. Miquel has to agree on the NAND changes, and even if I can't block the solution, I think I can at least give my opinion: anything that involves FS/wear-leveling specific code at the NAND level should be avoided. Given the discussion we had regarding JFFS2 and the cleanmarkers, I don't think we can come up with a solution that's safe for every users, hence the proposal to empower users with this responsibility.
Re: [PATCH v6 0/5] Micron SLC NAND filling block
On Tue, 2020-06-02 at 09:48 +0200, Boris Brezillon wrote: > Hi Bean, > > On Mon, 01 Jun 2020 23:10:43 +0200 > Bean Huo wrote: > > > Hi Richard > > would you please help us confirm below question?? > > Miquel suggested an approach that would allow us to deal with both > JFFS2 > and UBI/UBIFS without having any FS/wear-leveling specific code at > the > NAND level, but you decided to ignore his comments. Sorry but there's > nothing we can do to help you if you don't listen to our > recommendations. Expose this issue to FS layer, it is not good idea. that will impact more code, and involve duplicated code. > > I've been quite disappointed by your behavior in the past, and it > continues. Recently you've taken Miquel's patches and claimed > ownership did you seem my recent patch? you can ignore that see this. > on them (probably not intentionally, but still) while you were > clearly > unable to rework your original series the way I suggested (which > Miquel > did after seeing you would never send new versions). seriously? > And when Miquel > suggested a change to the implementation he had done based on the > discussion we had with Richard, you decided to ignore it and pursue > in > the original direction. So, quite frankly, I'm really not convinced > you > can conduct such a change. > As Miquel mentioned, we need richard's final comfirmation, If he agrees with this proposal, I give up my current patch.
Re: [PATCH v6 0/5] Micron SLC NAND filling block
Hi Bean, On Mon, 01 Jun 2020 23:10:43 +0200 Bean Huo wrote: > Hi Richard > would you please help us confirm below question?? Miquel suggested an approach that would allow us to deal with both JFFS2 and UBI/UBIFS without having any FS/wear-leveling specific code at the NAND level, but you decided to ignore his comments. Sorry but there's nothing we can do to help you if you don't listen to our recommendations. I've been quite disappointed by your behavior in the past, and it continues. Recently you've taken Miquel's patches and claimed ownership on them (probably not intentionally, but still) while you were clearly unable to rework your original series the way I suggested (which Miquel did after seeing you would never send new versions). And when Miquel suggested a change to the implementation he had done based on the discussion we had with Richard, you decided to ignore it and pursue in the original direction. So, quite frankly, I'm really not convinced you can conduct such a change. Regards, Boris
Re: [PATCH v6 0/5] Micron SLC NAND filling block
Hello Bean Huo wrote on Mon, 01 Jun 2020 23:10:43 +0200: > Hi Richard > would you please help us confirm below question?? > > Thanks, > Bean > > On Thu, 2020-05-28 at 16:14 +0200, Bean Huo wrote: > > hi, Richard > > > > > > On Mon, 2020-05-25 at 14:18 +0200, Bean Huo wrote: > > > After submission of patch V1 [1] and V2 [2], we stopped its update > > > since we get > > > stuck in the solution on how to avoid the power-loss issue in case > > > power-cut > > > hits the block filling. In the v1 and v2, to avoid this issue, we > > > always damaged > > > page0, page1, this's based on the hypothesis that NAND FS is UBIFS. > > > This > > > FS-specifical code is unacceptable in the MTD layer. Also, it > > > cannot > > > cover all > > > NAND based file system. Based on the current discussion, seems that > > > re-write all > > > first 15 page from page0 is a satisfactory solution. > > > > > This patch has overwrite page0~page14, damage EC and VID header > > boths. > > I know this is safe for UBIFS, even fastmap is enabled (you fixed > > this in (ubi: fastmap: Correctly handle interrupted erasures in > > EBA)). > > Now, how about jffs2? > > > > > > Thanks, > > Bean > > > FYI, Bean already askes me privately and here was my answer. Feel free to comment. ---8<--- I'm not sure we are synced on this issue, because it is clearly not addressed in your patchset. Quoting Richard, the ubifs log uses a fixed range of LEBs. It replays them upon mount and checks whether they are empty, new or outdated refs it assumes that interrupted erase got detecred by UBI and such a LEB will just contain 0xFF bytes. Rewriting the page before erase basically fails this assumption. For JFFS2, the problem is the clean marker. You cannot destroy the payload while keeping the clean marker which says "this block is fine and contain data". Also, if you destroy the clean marker, you need to take care of not turning the block being discovered as "bad" at reboot time if a power cut happens before the erasure. All of this is not impossible but: - we need to write specific code for each user - we don't want to create more problems that we already have I cannot give you more details because this is not something that I master. Ask Richard directly if you need more details on this. --->8--- Cheers, Miquèl
Re: [PATCH v6 0/5] Micron SLC NAND filling block
Hi Richard would you please help us confirm below question?? Thanks, Bean On Thu, 2020-05-28 at 16:14 +0200, Bean Huo wrote: > hi, Richard > > > On Mon, 2020-05-25 at 14:18 +0200, Bean Huo wrote: > > After submission of patch V1 [1] and V2 [2], we stopped its update > > since we get > > stuck in the solution on how to avoid the power-loss issue in case > > power-cut > > hits the block filling. In the v1 and v2, to avoid this issue, we > > always damaged > > page0, page1, this's based on the hypothesis that NAND FS is UBIFS. > > This > > FS-specifical code is unacceptable in the MTD layer. Also, it > > cannot > > cover all > > NAND based file system. Based on the current discussion, seems that > > re-write all > > first 15 page from page0 is a satisfactory solution. > > This patch has overwrite page0~page14, damage EC and VID header > boths. > I know this is safe for UBIFS, even fastmap is enabled (you fixed > this in (ubi: fastmap: Correctly handle interrupted erasures in > EBA)). > Now, how about jffs2? > > > Thanks, > Bean >
Re: [PATCH v6 0/5] Micron SLC NAND filling block
hi, Richard On Mon, 2020-05-25 at 14:18 +0200, Bean Huo wrote: > After submission of patch V1 [1] and V2 [2], we stopped its update > since we get > stuck in the solution on how to avoid the power-loss issue in case > power-cut > hits the block filling. In the v1 and v2, to avoid this issue, we > always damaged > page0, page1, this's based on the hypothesis that NAND FS is UBIFS. > This > FS-specifical code is unacceptable in the MTD layer. Also, it cannot > cover all > NAND based file system. Based on the current discussion, seems that > re-write all > first 15 page from page0 is a satisfactory solution. This patch has overwrite page0~page14, damage EC and VID header boths. I know this is safe for UBIFS, even fastmap is enabled (you fixed this in (ubi: fastmap: Correctly handle interrupted erasures in EBA)). Now, how about jffs2? Thanks, Bean
[PATCH v6 0/5] Micron SLC NAND filling block
From: Bean Huo Hi, On planar 2D Micron NAND devices when a block erase command is issued, occasionally even though a block erase operation completes and returns a pass status, the flash block may not be completely erased. Subsequent operations to this block on very rare cases can result in subtle failures or corruption. These extremely rare cases should nevertheless be considered. This patchset is to address this potential issue. After submission of patch V1 [1] and V2 [2], we stopped its update since we get stuck in the solution on how to avoid the power-loss issue in case power-cut hits the block filling. In the v1 and v2, to avoid this issue, we always damaged page0, page1, this's based on the hypothesis that NAND FS is UBIFS. This FS-specifical code is unacceptable in the MTD layer. Also, it cannot cover all NAND based file system. Based on the current discussion, seems that re-write all first 15 page from page0 is a satisfactory solution. Meanwhile, I borrowed one idea from Miquel Raynal patchset [3], in which keeps a recode of programmed pages, base on it, for most of the cases, we don't need to read every page to see if current erasing block is a partially programmed block. Changelog: v5 - v6: 1. Fix a misleading-indentation in patch 5/5 (Reported-by: kbuild test robot ) 2. Rebase patch to git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next v4 - v5: 1. Add Miquel Raynal Authorship and SoB in 4/5 and 5/5 (Miquel Raynal) 2. Change commit message in 5/5. (Steve deRosier) 3. delete unused variable max_bitflips in 4/5 v3 - v4: 1. In the patch 4/5, change to directly use ecc.strength to judge the page is a empty page or not, rather than max_bitflips < mtd->bitflip_threshold 2. In the patch 5/5, for the powerloss case, from the next time boot up, lots of page will be programmed from >page15 address, if still using first_p as GENMASK() bitmask starting position, writtenp will be always 0. fix it by changing its bitmask starting at bit position 0. v2 - v3: 1. Rebase patch to the latest MTD git tree 2. Add a record that keeps tracking the programmed pages in the first 16 pages 3. Change from program odd pages, damage page 0 and page 1, to program all first 15 pages 4. Address issues which exist in the V2. v1 - v2: 1. Rebased V1 to latest Linux kernel. 2. Add erase preparation function pointer in nand_manufacturer_ops. [1] https://www.spinics.net/lists/linux-mtd/msg04112.html [2] https://www.spinics.net/lists/linux-mtd/msg04450.html [3] https://www.spinics.net/lists/linux-mtd/msg13083.html Bean Huo (3): mtd: rawnand: group all NAND specific ops into new nand_chip_ops mtd: rawnand: Add {pre,post}_erase hooks in nand_chip_ops mtd: rawnand: Introduce a new function nand_check_is_erased_page() Miquel Raynal (2): mtd: rawnand: Add write_oob hook in nand_chip_ops mtd: rawnand: micron: Micron SLC NAND filling block drivers/mtd/nand/raw/internals.h | 3 +- drivers/mtd/nand/raw/nand_base.c | 87 ++ drivers/mtd/nand/raw/nand_hynix.c| 2 +- drivers/mtd/nand/raw/nand_macronix.c | 10 +-- drivers/mtd/nand/raw/nand_micron.c | 104 ++- include/linux/mtd/rawnand.h | 40 +++ 6 files changed, 211 insertions(+), 35 deletions(-) -- 2.17.1