Hi Richard
On 10/27/2014 10:56 AM, Richard Weinberger wrote:
Tanya,
Am 27.10.2014 um 09:41 schrieb Tanya Brokhman:
So, your patch addresses the following issue:
We need to re-read a PEB after a specific time (to detect bit rot) or after N
reads (to detect read disturb issues).
Is this correct?
Not exactly... We need to scrub a PEB that is being frequently read from in
order to prevent bit-flip errors that might occur due to read-disturb
This is what I meant with "after N reads". :)
Currently users of UBI do this by having cron jobs which read the complete UBI
volume
and then cause scrub work.
The draw back of this is that only UBI payload will be read and not all data
like EC and VID headers.
I understand that you want to fix this issue.
Not sure I completely understand what this crons do but the last patch in the
series does something similar.
The cron job reads the complete UBI volume. i.e. dd=/dev/ubi0_X of=/dev/null.
It will trigger scrub work
for bit-flipping PEBs. Is the poor men variant of your feature.
According to my opinion it is not a good idea to store read counters and
timestamps into the UBI/Fastmap on-disk layout.
Both the read counters and timestamps don't have to be exact values.
Why not? Storing last_erase_timestamp doesn't increase the memory consumption
on NAND since I used reserved bytes in the ec_header. I agree that the RAM is
increased but I couldn't
find any other way to have these statistics saved.
read_counters can be saved ONLY as part of fastmap unfortunately because of the
erase-before-write limitation.
Please explain in detail why those counters have to be exact.
I was not complaining about RAM consumption. But I think we should change the
on-disk layout only for very
serious reasons.
What about this idea?
Add a userspace interface which allows UBI to expose read counters and last
access timestamps.
Where will you save those?
In a plain file? As I said, the counters don't have to be exact. If you lose
one cycle, who cares....
The counters and timestamps are only a rough estimate.
i.e. the userspace daemon dumps all this informations from UBI and stores them
to a file (or a static UBI volume).
Upon system boot it restores them.
A userspace daemon (let's name it ubihealthd) then can decide whether it is
time to trigger a re-read of a PEB.
Not a re-read - scrub. read-disturb is fixed by erasing the PEB.
It will trigger a scrub work if bit-flipps happen. But what I was trying to
say, this all can be done perfectly fine
in userspace.
This daemon can also store and load the timestamp values and counters from and
to UBI. If it misses these meta data some times due to a
power cut it won't hurt.
Not sure i follow. How is this better then doing this from the kernel? you do
have to store the timestamps and the read_counters somewhere and they are both
updated in the ubi
layer. I must be missing something here. Could you please elaborate on your
idea?
If it can be done in userspace, do it in userspace. We have to make sure that
the kernel stays maintainable.
We really don't want to add new complexity which is not really needed.
We could also add another internal UBI volume which can carry these data.
I'm afraid I have to disagree with this idea. First of all having a dedicated
volume for this data is an overkill. Its not a sufficient amount of data to
reserve a volume for. and
what about the PEBs that belong to this volume? Taking this feature out of the
UBI layer is just complicated, feels wrong from design perspective, and I don't
see the benefit of
it. Basically, its very similar to the wear-leveling but for "reads" instead of
"writes".
But adding this data to fastmap is a better idea? fastmap is also just another
internal volume.
All in all, I like the idea but changing/extending the on-disk layout is
overkill IMHO.
Why? Without addressing this issues we can't have devices with life span of
more then ~5 years (and we need to). And this is very similar to wear-leveling
and erase counters. So
why is read-counters and erase_timestamp is an overkill?
I'm working on your idea of changing the fastmap layout to save all the read
disturb data at the end of it and not integrated into fastmap existing data
structures (as is done in
this version of the code). But as I see it, fastmap has to be updates as well.
I meant that adding these data to the on-disk layout is overkill. I like your
feature but not the part
where you extend the on-disk layout. In my opinion most of it can be done
without storing this data into fastmap
or other UBI internal on-disk data structures.
As I said, the counters don't have to be exact. Let a daemon handle and persist
them.
I'll try to address all you comments in one place.
You're right that the read counters don't have to be exact but they do
have to reflect the real state.
Regarding your idea of saving them to a file, or somehow with userspace
involved; This is doable, but such solution will depend on user space
implementation:
- one need to update kernel with correct read counters (saved somewhere
in userspace)
- it is required on every boot.
- saving the counters back to userspace should be periodically triggered
as well.
So the minimal workflow for each boot life cycle will be:
- on boot: update kernel with correct values from userspace
- kernel updates the counters on each read operation
- on powerdown: save the updated kernel counters back to userspace
The read-disturb handling is based on kernel updating and monitoring
read counters. Taking this out of the kernel space will result in an
incomplete and very fragile solution for the read-disturb problem since
the dependency in userspace is just too big.
Another issue to consider is that each SW upgrade will result in loosing
the counters saved in userspace and reset all. Otherwise, system upgrade
process will also have to be updated.
The read counters are very much like the ec counters used for
wear-leveling; One is updated on each erase, other on each read; One is
used to handle issues caused by frequent writes (erase operations), the
other handle issues caused by frequent reads.
So how are the two different? Why isn't wear-leveling (and erase
counters) handled by userspace? My guess that the decision to
encapsulate the wear-leveling into the kernel was due to the above
mentioned reasons.
Thanks,
Tanya Brokhman
--
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
