Re: [RFC/PATCH 0/5 v2] mtd:ubi: Read disturb and Data retention handling
On Thu, 2014-11-13 at 14:13 +0200, Tanya Brokhman wrote:
> > In your solution you have to do more work maintaining the counters and
> > writing them. With read solution you do more work reading data.
>
> But the maintaining work is minimal here. ++the counter on every read is
> all that is required and verify it's value. O(1)...
Let's consider the R/O FS on top of UBI case. Fastmap will only be
updated when there are erase operations, which may only be cause by
scrubbing in this case. IOW, fastmap will be updated extremely rarely.
And suppose there is no clean unmount ever happening.
Will we always lose erase counters and set them to half the threshold
all the time? Even if it was Threshold-1 before, it becomes Threshold/2
after power cut?
Don't we actually want to write the read counters when they change
significantly enough?
> I know... We got the threshold value (that is exposed in my patches as a
> define you just missed it) from NAND manufacturer asking to take into
> consideration the temperature the device will operate at. I know its
> still an estimation but so is the program/erase threshold. Since it was
> set by manufacturer - I think its the best one we can hope for.
I wonder how constant is the threshold.
* Does it change with time, as eraseblock becomes more worn out. Say,
the PEB resource is 1 erase cycles. Will the threshold be the same
for PEB at 0 erase cycles and at 5000 erase cycles?
* Does it depend on eraseblock?
* Does it depend on the I/O in other eraseblocks?
Just wonder how pessimistic is the threshold number manufacturers give.
Just curious to learn more about this number, and have an idea about how
reliable is it.
> > You will end up scrubbing a lot earlier than needed. Here comes the
> > performance loss too (and energy). And you will eventually end up
> > scrubbing too late.
>
> I don't see why I would end up scrubbing too late?
Well, one example - see above, you lose the read counters often, always
reset to threshold/2, end up reading more than the threshold.
The other doubt is that the threshold you use is actually the right one
for a worst case usage scenario of the end product. But probably it is
about just learning more about this threshold value.
> I can't guarantee it wont bit-flip, I don't think any one could but I
> can say that with my implementation the chance of bit-flip is reduced.
That was my point. There is already a solution for the problem you are
trying to solve. It is implemented. And it covers not just the problem
you are solving, but the other problems of NAND.
So probably what is missing is some kind of better analysis or
experimental prove that the solution which is already implemented (let's
call it "periodic read") is defective.
May be I should expand a bit more on why the periodic read solution does
not look bad to me.
If the ECC is strong enough for the flash chip in question, then
bit-flips will accumulate slowly enough. First one bit-flip, then 2,
then 3, etc. All you need to do is to make your read period good enough
to make sure no PEB accumulates too many bit-flips.
E.g., modern ECCs cover 8 or more bit-flips.
And the other compelling point here that this will cover all other NAND
effects. All of them lead to more bit-flips at the end, right? And you
just fix bit-flips when they come. You do not care why they came. You
just deal with them.
And what is very nice is that you do not need to implement anything, or
you implement very little.
> In an endless loop - read page 3 of PEB-A.
> This will effect near by pages (say 4 and 2 for simplicity). But if I
> scrub the whole PEB according to read-counter I will save data of pages
> 2 and 4.
> If I do nothing: when reading eventually page 4 it will produce
> bit-flips that may not be fixable.
This is quite artificial example, but yes, if you read the same page in
a tight loop, you may cause bit flips fast enough, faster than your
periodic read task starts reading your media.
But first of all, how realistic is this scenario? I am sure not,
especially if there is an FS on top of UBI and the data are cached, so
the second read actually reads from RAM.
Secondly, can this scenario be covered by simpler means? Say, UBI could
watch the read ratio, and if it grows, trigger the scrubber task
earlier?
> > I understand the whole customer orientation concept. But for me so far
> > the solution does not feel like something suitable to a customer I could
> > imagine. I mean, if I think about me as a potential customer, I would
> > just want my data to be safe and covered from all the NAND effects.
>
> I'm not sure that at the moment "all NAND effects" can be covered.
I explained how I see it above in this e-mail. In short: read all data
often enough ("enough" is defined by your product), and you are done.
All "NAND effects" lead to bit-flips, you fix bit-flips faster than they
become hard errors, and you are done.
--
To unsubscribe from this list: send the line "unsubscribe
Re: [RFC/PATCH 0/5 v2] mtd:ubi: Read disturb and Data retention handling
[Sort of off-topic] On Wed, 2014-11-12 at 14:01 +0100, Richard Weinberger wrote: > Tanya stated that the read counters must not get lost. I understood that this is more of "we try not to lose them, but if we lose, we can deal with this". > But it can happen that you lose the fastmap. Fastmap is optional. And new data structure would be kind of optional too. > I.e. if you boot an older kernel it will delete the fastmap. If you run > out of PEBs which can be used by fastmap, fastmap has to delete the current > fastmap. > Same for too many write errors, etc... It would be cool to document this in more details, say in the web site. If someone uses fastmap, they probably need to know exactly when it could "disappear", in order to try avoiding these conditions. > If we add the read-counters to fastmap we'd have to change the fastmap > on-flash layout too. But this is not the end of the world. Fastmap is still an experimental feature, and I personally consider it as "not yet proved to be ready for production", because I did not hear success stories yet. It does not mean there are no success stories. And this is just my perception, I may be wrong. So while not touching on-flash format is always a good goal, we may be less resistant about fastmap. > (Unless we do very hacky tricks) > Also writing a fastmap is not cheap, we have to stop all IO. So, saving the > read-counter will > be expensive and an performance problem. For me this one sounds like a strong point. We do not really want to make fastmap change more often. Thanks, Artem. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V6] UBI: Extend UBI layer debug/messaging capabilities
On Tue, 2014-11-11 at 20:49 +0100, Richard Weinberger wrote: > > I am OK with removing function names from warnings and errors, though. > > But they were there since the very beginning, and changing this is a > > separate subject, so I'd prefer someone else to submit a corresponding > > patch. > > If we keep the function names away from ubi_msg() I'm perfectly fine. :-) OK, I'm pushing the patch I sent. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/5 v2] mtd:ubi: Read disturb and Data retention handling
On Tue, 2014-11-11 at 22:39 +0100, Richard Weinberger wrote: > Please don't (ab)use fastmap. If you really need persistent read-counters use > an internal UBI volume. Just like you, I do not think the proposed solution is the right answer to the problem, at least so far. But if we imagine that Tanya proves that the counters is the right thing, storing them in fastmap would be the first thing which comes to mind. Just calling this an abuse without explaining (even if this is right) is not very collaborative. Let me see why would that be an "abuse"... Probably because of the nature of the data. Fastmap contains data which only changes in case of writes (well, more precisely, erases, but those usually go are related to writes). Read counters are completely opposite - they stay constant when we write and change when we read. Putting them all to the same on-flash area is possible, but is it optimal? I wouldn't be so sure, I see cons. and pros. Any other reasons? Artem. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/5 v2] mtd:ubi: Read disturb and Data retention handling
On Tue, 2014-11-11 at 22:36 +0200, Tanya Brokhman wrote: > Unfortunately none. This is done for a new device that we received just > now. The development was done on a virtual machine with nandsim. Testing > was more of stability and regression OK. So the implementation is theory-driven and misses the experimental prove. This means that building a product based on this implementation has certain amount of risk involved. And from where I am, the theoretical base for the solution also does not look very strong. > > The advantages of the "read all periodically" approach were: > > > > 1. Simple, no modifications needed > > 2. No need to write if the media is read-only, except when scrubbing > > happens. > > 3. Should cover all the NAND effects, including the "radiation" one. > > Disadvantages (as I see it): > 1. performance hit: when do you trigger the "read-all"? will effect > performance Right. We do not know how often, just like we do not know how often and how much (read counter threshold) in your proposal. Performance - sure, matter of experiment, just like the performance of your solution. And as I notice, energy too (read - battery life). In your solution you have to do more work maintaining the counters and writing them. With read solution you do more work reading data. The promise that reading may be done in background, when there is no other I/O. > 2. finds bitflips only when they are present instead of preventing them > from happening But is this true? I do not see how is this true in your case. Yo want to scrub by threshold, which is a theoretical value with very large deviation from the real one. And there may be no real one even - the real one depends on the erase block, it depends on the I/O patterns, and it depends on the temperature. You will end up scrubbing a lot earlier than needed. Here comes the performance loss too (and energy). And you will eventually end up scrubbing too late. I do not see how your solution provides any hard guarantee. Please, explain how do you guarantee that my PEB does not bit-rot earlier than read counter reaches the threshold? It may bit-rot earlier because it is close to be worn out, or because of just higher temperature, or because it has a nano-defect. > Perhaps our design is an overkill for this and not covering 100% of te > usecases. But it was requested by our customers to handle read-disturb > and data retention specifically (as in "prevent" and not just "fix"). > This is due to a new NAND device that should operate in high temperature > and last for ~15-20 years. I understand the whole customer orientation concept. But for me so far the solution does not feel like something suitable to a customer I could imagine. I mean, if I think about me as a potential customer, I would just want my data to be safe and covered from all the NAND effects. I would not want counters, I'd want the result. And in the proposed solution I would not see how I'd get the guaranteed result. But of course I do not know the customer requirements that you've got. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] UBI: Extend UBI layer debug/messaging capabilities - cosmetics
On Sun, 2014-11-09 at 13:06 +0200, Tanya Brokhman wrote: > Some cosmetic fixes to the patch "UBI: Extend UBI layer debug/messaging > capabilities". > > Signed-off-by: Tanya Brokhman > --- Pushed this patch, but without the hunk which changes the printing helpers. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V6] UBI: Extend UBI layer debug/messaging capabilities
On Tue, 2014-11-11 at 13:25 +0100, Richard Weinberger wrote:
> Am 11.11.2014 um 13:03 schrieb Artem Bityutskiy:
> > On Tue, 2014-11-11 at 09:15 +0100, Richard Weinberger wrote:
> >>> Do we really want the function name in every log message?
> >>> IMHO this is not wise except for pure debug logs.
> >>
> >> BTW: Why UBI-X? This looks odd. Either use UBIX or ubiX.
> >
> > How about something like this (untested):
> >
> >
> > From: Artem Bityutskiy
> > Date: Tue, 11 Nov 2014 13:56:34 +0200
> > Subject: [PATCH] UBI: clean-up printing helpers
> >
> > Let's prefix UBI messages with 'ubiX' instead of 'UBI-X' - this is more
> > consistent with the way we name UBI devices.
> >
> > Also, commit "32608703 UBI: Extend UBI layer debug/messaging capabilities"
> > added the function name print to 'ubi_msg()' - lets revert this change,
> > since
> > these messages are supposed to be just informative messages, and not
> > debugging
> > messages.
>
> What is the benefit of having the function name still in ubi_warn() and
> ubi_err()?
The benefit is that it is easier to find the source code where the
message comes from. And not necessarily because the message is
"cryptic", but because you may want to check the possible reasons of the
problem from the code.
> e.g.
> [ 95.511825] ubi0 error: ubi_attach_mtd_dev: mtd0 is already attached to
> ubi0
>
> If the log message is so cryptic that you need to lookup it in the source to
> understand it,
> we better fix the message.
UBI messages are usually not cryptic, and I think we did try to keep
them user-friendly. If you hit a cryptic error or warning message, do
not hesitate to improve it please. Debug messages are often cryptic.
I am OK with removing function names from warnings and errors, though.
But they were there since the very beginning, and changing this is a
separate subject, so I'd prefer someone else to submit a corresponding
patch.
> > -#define ubi_msg(ubi, fmt, ...) pr_notice("UBI-%d: %s:" fmt "\n", \
> > - ubi->ubi_num, __func__, ##__VA_ARGS__)
> > +#define ubi_msg(ubi, fmt, ...) pr_notice("ubi%d: " fmt "\n", \
> > +ubi->ubi_num, ##__VA_ARGS__)
>
> We could even use UBI_NAME_STR here. :-)
OK, how about this (untested)
From: Artem Bityutskiy
Date: Tue, 11 Nov 2014 13:56:34 +0200
Subject: [PATCH v2] UBI: clean-up printing helpers
Let's prefix UBI messages with 'ubiX' instead of 'UBI-X' - this is more
consistent with the way we name UBI devices.
Also, commit "32608703 UBI: Extend UBI layer debug/messaging capabilities"
added the function name print to 'ubi_msg()' - lets revert this change, since
these messages are supposed to be just informative messages, and not debugging
messages.
Signed-off-by: Artem Bityutskiy
---
drivers/mtd/ubi/ubi.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index f80ffab..ee7ac0b 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -50,13 +50,13 @@
#define UBI_NAME_STR "ubi"
/* Normal UBI messages */
-#define ubi_msg(ubi, fmt, ...) pr_notice("UBI-%d: %s:" fmt "\n", \
-ubi->ubi_num, __func__, ##__VA_ARGS__)
+#define ubi_msg(ubi, fmt, ...) pr_notice(UBI_NAME_STR "%d: " fmt "\n", \
+ubi->ubi_num, ##__VA_ARGS__)
/* UBI warning messages */
-#define ubi_warn(ubi, fmt, ...) pr_warn("UBI-%d warning: %s: " fmt "\n", \
+#define ubi_warn(ubi, fmt, ...) pr_warn(UBI_NAME_STR "%d warning: %s: " fmt
"\n", \
ubi->ubi_num, __func__, ##__VA_ARGS__)
/* UBI error messages */
-#define ubi_err(ubi, fmt, ...) pr_err("UBI-%d error: %s: " fmt "\n", \
+#define ubi_err(ubi, fmt, ...) pr_err(UBI_NAME_STR "%d error: %s: " fmt "\n", \
ubi->ubi_num, __func__, ##__VA_ARGS__)
/* Background thread name pattern */
--
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V6] UBI: Extend UBI layer debug/messaging capabilities
On Tue, 2014-11-11 at 09:15 +0100, Richard Weinberger wrote:
> > Do we really want the function name in every log message?
> > IMHO this is not wise except for pure debug logs.
>
> BTW: Why UBI-X? This looks odd. Either use UBIX or ubiX.
How about something like this (untested):
From: Artem Bityutskiy
Date: Tue, 11 Nov 2014 13:56:34 +0200
Subject: [PATCH] UBI: clean-up printing helpers
Let's prefix UBI messages with 'ubiX' instead of 'UBI-X' - this is more
consistent with the way we name UBI devices.
Also, commit "32608703 UBI: Extend UBI layer debug/messaging capabilities"
added the function name print to 'ubi_msg()' - lets revert this change, since
these messages are supposed to be just informative messages, and not debugging
messages.
Signed-off-by: Artem Bityutskiy
---
drivers/mtd/ubi/ubi.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index f80ffab..7a92283 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -50,13 +50,13 @@
#define UBI_NAME_STR "ubi"
/* Normal UBI messages */
-#define ubi_msg(ubi, fmt, ...) pr_notice("UBI-%d: %s:" fmt "\n", \
-ubi->ubi_num, __func__, ##__VA_ARGS__)
+#define ubi_msg(ubi, fmt, ...) pr_notice("ubi%d: " fmt "\n", \
+ubi->ubi_num, ##__VA_ARGS__)
/* UBI warning messages */
-#define ubi_warn(ubi, fmt, ...) pr_warn("UBI-%d warning: %s: " fmt "\n", \
+#define ubi_warn(ubi, fmt, ...) pr_warn("ubi%d warning: %s: " fmt "\n", \
ubi->ubi_num, __func__, ##__VA_ARGS__)
/* UBI error messages */
-#define ubi_err(ubi, fmt, ...) pr_err("UBI-%d error: %s: " fmt "\n", \
+#define ubi_err(ubi, fmt, ...) pr_err("ubi%d error: %s: " fmt "\n", \
ubi->ubi_num, __func__, ##__VA_ARGS__)
/* Background thread name pattern */
--
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V6] UBI: Extend UBI layer debug/messaging capabilities
On Mon, 2014-11-10 at 09:57 -0800, Joe Perches wrote: > On Mon, 2014-11-10 at 18:37 +0100, Richard Weinberger wrote: > > Am 03.11.2014 um 14:58 schrieb Tanya Brokhman: > > > If there is more then one UBI device mounted, there is no way to > > > distinguish between messages from different UBI devices. > > > Add device number to all ubi layer message types. > > Adding "error" and "warning" to the message logs is > duplicative to the KERN_ logging information. > > > > Changes from V5: > > > - Added ptr verification @ ubi_err/ubi_msg/ubi_warn > > > Removed extra printing of ubi number > > > Removed new messages. > > Did you all ever look at what I posted? > > https://lkml.org/lkml/2014/10/14/280 > > smaller code, consistent prefixing, consistent with > typical kernel style, etc. No, I did not, because Tanya is someone who actually using and developing that stuff, and you are more of being on the watch for the kernel in general. So naturally, I use my limited time to first check her stuff. Sorry for this. But Let me change small things in Tanya's patch first. The if you'd be kind enough to implement your changes on top of that, that'd be perfect. Artem. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] UBI: Extend UBI layer debug/messaging capabilities - cosmetics
On Mon, 2014-11-10 at 14:53 +0200, Tanya Brokhman wrote:
> On 11/10/2014 2:18 PM, Artem Bityutskiy wrote:
> > On Sun, 2014-11-09 at 13:06 +0200, Tanya Brokhman wrote:
> >>
> >> /* Normal UBI messages */
> >> #define ubi_msg(ubi, fmt, ...) pr_notice("UBI-%d: %s:" fmt "\n", \
> >> -ubi->ubi_num, __func__,
> >> ##__VA_ARGS__)
> >> + (ubi ? ubi->ubi_num : UBI_MAX_DEVICES), \
> >> + __func__, ##__VA_ARGS__)
> >> /* UBI warning messages */
> >> #define ubi_warn(ubi, fmt, ...) pr_warn("UBI-%d warning: %s: " fmt "\n",
> >> \
> >> - ubi->ubi_num, __func__,
> >> ##__VA_ARGS__)
> >> + (ubi ? ubi->ubi_num : UBI_MAX_DEVICES), \
> >> + __func__, ##__VA_ARGS__)
> >> /* UBI error messages */
> >> #define ubi_err(ubi, fmt, ...) pr_err("UBI-%d error: %s: " fmt "\n", \
> >> - ubi->ubi_num, __func__,
> >> ##__VA_ARGS__)
> >> + (ubi ? ubi->ubi_num : UBI_MAX_DEVICES), \
> >> + __func__, ##__VA_ARGS__)
> >
> > Why did you make these changes? It is preferable to not add another 'if'
> > statement to this macro to handle one or 2 cases - much bloat, little
> > gain.
> >
> > Could we please avoid this?
>
> I just wanted to be on the safe side and prevent this macro being called
> with ubi=NULL that may crash the system. If you still prefer the "if"
> removed will do.
On the other hand, these are macros, and this if gets duplicated in many
places and translate into few additional assembly instructions per
message.
> > The warning looks pretty poor, so I do not mind to remove it, but I
> > thought your patch is about adding a parameter, but you mix different
> > kinds of things there. Please, be stricter to the similar UBIFS patch
> > which you was going to send.
>
> Now I'm confused. I added this msg as part of the patch you already
> pushed to your branch but later you requested NOT to add additional msgs
> and if required add it in a different patch. So this was added by me and
> now removed by me - as per your request.
This comment of mine just repeats that request. It talks about being
stricter in the future patches and not add/remove messages. It does not
request to modify this patch. IOW, this change is OK, but please, let's
make sure we do not have them in the UBIFS patch.
> > How about just turning this into a debug message, not removing?
>
> Same here. Removing this because *you* requested it.
> Quoting you from V5:
> "Yes, please, remove these messages or turn them into debugging messages.
> And yes, these should have been added in a separate patch."
OK, just asking.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] UBI: Extend UBI layer debug/messaging capabilities - cosmetics
On Sun, 2014-11-09 at 13:06 +0200, Tanya Brokhman wrote:
>
> /* Normal UBI messages */
> #define ubi_msg(ubi, fmt, ...) pr_notice("UBI-%d: %s:" fmt "\n", \
> -ubi->ubi_num, __func__,
> ##__VA_ARGS__)
> + (ubi ? ubi->ubi_num : UBI_MAX_DEVICES), \
> + __func__, ##__VA_ARGS__)
> /* UBI warning messages */
> #define ubi_warn(ubi, fmt, ...) pr_warn("UBI-%d warning: %s: " fmt "\n", \
> - ubi->ubi_num, __func__, ##__VA_ARGS__)
> + (ubi ? ubi->ubi_num : UBI_MAX_DEVICES), \
> + __func__, ##__VA_ARGS__)
> /* UBI error messages */
> #define ubi_err(ubi, fmt, ...) pr_err("UBI-%d error: %s: " fmt "\n", \
> - ubi->ubi_num, __func__, ##__VA_ARGS__)
> + (ubi ? ubi->ubi_num : UBI_MAX_DEVICES), \
> + __func__, ##__VA_ARGS__)
Why did you make these changes? It is preferable to not add another 'if'
statement to this macro to handle one or 2 cases - much bloat, little
gain.
Could we please avoid this?
>
> - if (!ubi->free.rb_node || (ubi->free_count - ubi->beb_rsvd_pebs < 1))
> {
> - ubi_warn(ubi, "Can't get peb for fastmap:anchor=%d,
> free_cnt=%d, reserved=%d",
> -anchor, ubi->free_count, ubi->beb_rsvd_pebs);
> + if (!ubi->free.rb_node || (ubi->free_count - ubi->beb_rsvd_pebs < 1))
> goto out;
The warning looks pretty poor, so I do not mind to remove it, but I
thought your patch is about adding a parameter, but you mix different
kinds of things there. Please, be stricter to the similar UBIFS patch
which you was going to send.
> - if (kthread_should_stop()) {
> - ubi_msg(ubi, "background thread \"%s\" should stop,
> PID %d",
> - ubi->bgt_name, task_pid_nr(current));
> + if (kthread_should_stop())
> break;
> - }
How about just turning this into a debug message, not removing?
Artem.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/5 v2] mtd:ubi: Read disturb and Data retention handling
On Sun, 2014-11-02 at 15:30 +0200, Tanya Brokhman wrote: > > If NAND why not use ECC to monitor for disturb? > > We don't want just to monitor, we want to prevent cases where ecc cant > be fixed. You said it yourself later on "BCH ECC will tell you if bits > have changed and will correct up to 5". The goal is to prevent more then > 5 errors that can't be fixed. > > NAND is a great storage unit, but you have to follow the rules. Please > refer to Micron datasheet MT29F2G08ABAEAH4 page 100. NAND is made up of > blocks(2048 in this case), each block has a number of pages. The block > is the smallest erasable unit and the only way to change 0's to 1's. > Pages are the smallest programmable unit and can only change 1's to 0's. > P/E cycling (100,000 in this case) wears out the block. We provide > 64bytes of spare area for BCH ECC and NAND management. BCH ECC will > tell you if bits have changed and will correct up to 5. > > > > Read disturb is a recoverable failure. It doesn't affect the cells in the > > page you are reading it affects the cells on either side of the page you > > are reading. P/E cycling for this device is 100,000. You can program once > > and read many many times. > > > > Data retention is the loss of charge on the cells. Technically you can > > only change a 0 to 1 by erasing the whole block. However, data retention > > is the loss of charge in a cell over time. In this case data retention is > > 10 years. > > Data retention gets worse as temperature goes up. > > Exactly! We're aware of all you described above. This is exactly why we > need to handle both read disturb and data retention. Hi Tanya, just a friendly notice: did you notice that you drop all the CCs in the reply? Even the person you replied to was not in "To". I guess it is worth checking your e-mail client's settings. Jeff, my main concern about the patches is whether they really address NAND problems, and whether the complexity they introduce are worth it. The counter-approach is to just read the entire flash periodically, and just scrub the PEBs (physical eraseblocks) which have have enough bit-flips (more than a configured threshold per ECC unit, say 1 or 2). I tried to explain my concerns in here: http://lists.infradead.org/pipermail/linux-mtd/2014-November/056385.html http://lists.infradead.org/pipermail/linux-mtd/2014-November/056386.html Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Sun, 2014-11-02 at 19:14 +0200, Tanya Brokhman wrote: > >> +ubi_err(ubi, "self-check failed for PEB %d", pnum); > >> +ubi_msg(ubi, "hex dump of the %d-%d region", > >> + offset, offset + len); > >> print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1, buf, > len, 1); > >> err = -EINVAL; > >> error: > > > > Artem, I know you have tried to align the message code in different > lines, maybe > > you can check if you lose this one. > > > > hmmm... not sure I understand what is wrong here It is more of a nit-pick, but we try to be consistent. Here is how we align split messages: ubi_msg(ubi, "blah", XYZ) and not ubi_msg(ubi, "blah", XYZ) So we first use few tabs, and then some spaces to align. You use just tabs. Sometimes the second line is aligned, sometimes it goes a bit further. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/5 v2] mtd:ubi: Read disturb and Data retention handling
On Sun, 2014-11-02 at 15:25 +0200, Tanya Brokhman wrote: > On 10/31/2014 5:39 PM, Richard Weinberger wrote: > > Am 31.10.2014 um 16:34 schrieb Richard Weinberger: > >> Hi Tanya, > >> > >> Am 31.10.2014 um 14:12 schrieb Tanya Brokhman: > >>> Hi Richard > >>> > >>> On 10/29/2014 2:00 PM, Richard Weinberger wrote: > Tanya, > > Am 29.10.2014 um 12:03 schrieb Tanya Brokhman: > > 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. > > But it does not really matter if the counters are a way to high or too > low? > It does also not matter if a re-read of adjacent PEBs is issued too > often. > It won't hurt. > > > 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 > > Correct. > > > - kernel updates the counters on each read operation > > Yeah, that's a plain simple in kernel counter.. > > > - on powerdown: save the updated kernel counters back to userspace > > Correct. The counters can also be saved once a day by cron. > If one or two save operations are missed it won't hurt either. > > > 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. > > Why? > We both agree on the fact that the counters don't have to be exact. > Maybe I'm wrong but to my understanding they are just a rough indicator > that sometime later UBI has to check for bitrot/flips. > >>> > >>> The idea is to prevent data loss, to prevent errors while reading, > >>> because we might hit errors we can't fix. So although the > >>> read_disturb_threshold is a rough estimation based on > >>> statistics, we can't ignore it and need to stay close to the calculated > >>> statistics. > >>> > >>> Its really the same as wear-leveling. You have a limitation that each peb > >>> can be erased limited number of times. This erase-limit is also an > >>> estimation based on statistics > >>> collected by the card vendor. But you do want to know the exact number of > >>> erase counter to prevent erasing the block extensively. > >> > >> So you have to update the EC-Header every time we read a PEB...? > >> > >>> > > > 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. > > Does it hurt if these counters are lost upon an upgrade? > Why do we need them for ever? > If they start after an upgrade from 0 again heavily read PEBs will > quickly gain a high counter and will be checked. > >>> > >>> yes, we do need the ACCURATE counters and cant loose them. For example: > >>> we have a heavily read block. It was read from 100 times when the > >>> read-threshold is 101. Meaning, the 101 > >>> read will most probably fail. > >> > >> You are trying me to tell that the NAND is that crappy that it will die > >> after 100 reads? I really hope this was just a bad example. > >> You *will* loose counters unless you update the EC-Header upon every read, > >> which is also not sane at all. > >> > >>> You do a SW upgrade, and set the read-counter for this block as 0 and > >>> don't scrubb it. Next time you try reading from it (since it's heavily > >>> read from block), you'll get errors. If > >>> you're lucky, ecc will fx them for you, but its not guarantied. > >>> > > And of course these counters can be preserved. One can also place them > into a UBI static volume. > Or use a sane upgrade process... > >>> > >>> "Sane upgrade" means that in order to support read-disturb we twist the > >>> users hand into implementing not a trivial logic in userspace. > >>> > > As I wrote in my last mail we could also create a new internal UBI > volume to store these counters. > Then you can have the logic in kernel but don't have to change the UBI > on-disk layout. > > > 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 > >>>
Re: [PATCH V6] UBI: Extend UBI layer debug/messaging capabilities
On Mon, 2014-11-03 at 15:58 +0200, Tanya Brokhman wrote: > If there is more then one UBI device mounted, there is no way to > distinguish between messages from different UBI devices. > Add device number to all ubi layer message types. > > The R/O block driver messages were replaced by pr_* since > ubi_device structure is not used by it. > > Amended a bit by Artem. > > Signed-off-by: Tanya Brokhman > Signed-off-by: Artem Bityutskiy Oh, the entire patch-set again... this means we need to review it again! Please, have a mercy! :-) It is huge! Please, send a small patch against the one which I pushed already. Thanks! Artem. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/5 v2] mtd: ubi: Read disturb infrastructure
Hi, let me summarize. 1. To handle the read disturb problem you selected the read counters solution. PEB (physical erase block) read counter is the trigger for scrubbing. When the PEB read counter reaches a threshold value - we scrub. The threshold value is set to 10 by default. Users may it via sysfs. Read-counters are stored on the flash media, in the fastmap structure. 2. To handle the data retention problem you selected the time-stamps approach. Each PEB gets a time-stamp. Time-stamp gets updated when the PEB is erased. When PEB becomes old enough (by comparing to the current system time), it gets scrubbed. The threshold for "old enough" is 120 days by default. Users may change it via sysfs. Timestamps are stored on the flash media, in the erase counter header of the PEB. For both, the read-disturb and data retention problems to be taken care of, user-space has to periodically trigger scanning, which will go through all PEBs, check the read counter and the timestamp, and scrub if needed. Hopefully I got right. I have concerns. This is rather complex design, and it is not clear why just doing full flash read from time to time is not good enough. Why the complexity is worth it. In this case the trigger for scrubbing is a bit-flip. It indicates that there is a problem, and this is a reliable trigger. Is the 10 reads threshold a reliable trigger? What if it is 10 times larger for me, or smaller? Is the 120 days threshold a reliable trigger? What if it is much larger for me? Do you think it is even possible to get the thresholds right? Let me re-emphasize: bit-flip is an objective, reliable reason to scrub. Thresholds are more of a pessimistic prediction. No? Let's think of someone having a R/O storage with video files. Compare: 1. Do full media read every 120 days in background with low priority (possibly implemented as UBI service). And because it is R/O, cut the power all you want. 2. Have UBI write data to media all the time to maintain the read counters. Lose your fastmap and have slow mounts on power cuts. Have extra work on boot-time. Spend more RAM. Why 2 is better than 1? Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Fri, 2014-10-24 at 11:33 +0800, hujianyang wrote:
> > if (len == 0) {
> > - pr_warn("UBI warning: empty 'mtd=' parameter - ignored\n");
> > + pr_err("UBI warning: empty 'mtd=' parameter - ignored\n");
> > return 0;
> > }
>
> Why the last 'pr_warn()' need to be changed into 'pr_err()'? I looked up your
> V1 and V2 patches, I think it's not your purpose.
Well-spotted, thanks.
> > -static int check_av(const struct ubi_volume *vol,
> > +static int check_av(const struct ubi_device *ubi, const struct ubi_volume
> > *vol,
> > const struct ubi_ainf_volume *av)
> > {
> > int err;
>
> This patch add 'struct ubi_device *' for 3 functions. We can get 'ubi_device'
> from
> 'ubi_volume'. So I think it's because when we call these functions, the
> '->ubi'
> pointer of 'ubi_volume' is not initialized, am I right? This patch use
> 'vol->ubi'
> to indicate a 'struct ubi_device *' pointer in some places, I think you are
> sure
> of using them.
Yeah, let's remove the unneeded argument indeed.
> > - ubi_msg("attached mtd%d (name \"%s\", size %llu MiB) to ubi%d",
> > - mtd->index, mtd->name, ubi->flash_size >> 20, ubi_num);
> > - ubi_msg("PEB size: %d bytes (%d KiB), LEB size: %d bytes",
> > + ubi_msg(ubi, "attached mtd%d (name \"%s\", size %llu MiB)",
> > + mtd->index, mtd->name, ubi->flash_size >> 20);
> > + ubi_msg(ubi, "PEB size: %d bytes (%d KiB), LEB size: %d bytes",
> > ubi->peb_size, ubi->peb_size >> 10, ubi->leb_size);
>
> We have the parameter 'ubi_num' for log in some functions like
> 'ubi_attach_mtd_dev'
> before. This patch remove 'ubi_num' in upper changes but keep it in other
> changes.
> Do we have a discussed rule to deal with this situation? It's not a big
> problem~
Well, printing 'ubi_num' explicitely is not needed anymore, so it would
be good to make the code consistent and remove it from other places,
where it is not needed.
> > - if (kthread_should_stop())
> > + if (kthread_should_stop()) {
> > + ubi_msg(ubi, "background thread \"%s\" should stop, PID
> > %d",
> > + ubi->bgt_name, task_pid_nr(current));
> > break;
> > + }
> >
> > if (try_to_freeze())
> > continue;
>
> Here are two new adding messages. Maybe a separate patch is better? Just a
> suggestion.
Yes, please, remove these messages or turn them into debugging messages.
And yes, these should have been added in a separate patch.
> > @@ -1415,8 +1418,9 @@ int ubi_self_check_all_ff(struct ubi_device *ubi, int
> > pnum, int offset, int len)
> > return 0;
> >
> > fail:
> > - ubi_err("self-check failed for PEB %d", pnum);
> > - ubi_msg("hex dump of the %d-%d region", offset, offset + len);
> > + ubi_err(ubi, "self-check failed for PEB %d", pnum);
> > + ubi_msg(ubi, "hex dump of the %d-%d region",
> > +offset, offset + len);
> > print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1, buf, len, 1);
> > err = -EINVAL;
> > error:
>
> Artem, I know you have tried to align the message code in different lines,
> maybe
> you can check if you lose this one.
Yeah, lets' correct this too.
Thanks for ferview Hujianyang!
Tanya, would you send a follow-up patch these?
Artem.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Mon, 2014-10-20 at 19:57 +0300, Tanya Brokhman wrote:
> If there is more then one UBI device mounted, there is no way to
> distinguish between messages from different UBI devices.
> Add device number to all ubi layer message types.
>
> The R/O block driver messages were replaced by pr_* since
> ubi_device structure is not used by it.
>
> Signed-off-by: Tanya Brokhman
Pushed to linux-ubifs.git, thanks.
You did not get indentations right, though, so I amended your patch.
Also, some lines were split unnecessarily. My amendments are in the diff
below. Please, take this into account for the similar UBIFS patch which
you was going to send. Thank you!
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index 3da5df1..9d2e16f 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -671,7 +671,7 @@ static int early_erase_peb(struct ubi_device *ubi,
* erase counters internally.
*/
ubi_err(ubi, "erase counter overflow at PEB %d, EC %d",
- pnum, ec);
+ pnum, ec);
return -EINVAL;
}
@@ -1637,7 +1637,7 @@ static int self_check_ai(struct ubi_device *ubi, struct
ubi_attach_info *ai)
err = ubi_io_read_vid_hdr(ubi, aeb->pnum, vidh, 1);
if (err && err != UBI_IO_BITFLIPS) {
ubi_err(ubi, "VID header is not OK (%d)",
- err);
+ err);
if (err > 0)
err = -EIO;
return err;
@@ -1691,7 +1691,7 @@ static int self_check_ai(struct ubi_device *ubi, struct
ubi_attach_info *ai)
if (av->last_data_size != be32_to_cpu(vidh->data_size)) {
ubi_err(ubi, "bad last_data_size %d",
- av->last_data_size);
+ av->last_data_size);
goto bad_vid_hdr;
}
}
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index 657ff08..6b6bce2 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -117,7 +117,7 @@ static int __init ubiblock_set_param(const char *val,
if (len == UBIBLOCK_PARAM_LEN) {
pr_err("UBI: block: parameter \"%s\" is too long, max. is %d\n",
- val, UBIBLOCK_PARAM_LEN);
+ val, UBIBLOCK_PARAM_LEN);
return -EINVAL;
}
@@ -446,7 +446,7 @@ int ubiblock_create(struct ubi_volume_info *vi)
/* Must be the last step: anyone can call file ops from now on */
add_disk(dev->gd);
dev_info(disk_to_dev(dev->gd), "created from ubi%d:%d(%s)",
- dev->ubi_num, dev->vol_id, vi->name);
+dev->ubi_num, dev->vol_id, vi->name);
return 0;
out_free_queue:
@@ -518,7 +518,7 @@ static int ubiblock_resize(struct ubi_volume_info *vi)
if ((sector_t)disk_capacity != disk_capacity) {
mutex_unlock(&devices_mutex);
dev_warn(disk_to_dev(dev->gd), "the volume is too big (%d
LEBs), cannot resize",
- vi->size);
+vi->size);
return -EFBIG;
}
@@ -527,7 +527,7 @@ static int ubiblock_resize(struct ubi_volume_info *vi)
if (get_capacity(dev->gd) != disk_capacity) {
set_capacity(dev->gd, disk_capacity);
dev_info(disk_to_dev(dev->gd), "resized to %lld bytes",
- vi->used_bytes);
+vi->used_bytes);
}
mutex_unlock(&dev->dev_mutex);
mutex_unlock(&devices_mutex);
@@ -596,7 +596,7 @@ static int __init ubiblock_create_from_param(void)
desc = open_volume_desc(p->name, p->ubi_num, p->vol_id);
if (IS_ERR(desc)) {
pr_err("UBI: block: can't open volume, err=%ld\n",
- PTR_ERR(desc));
+ PTR_ERR(desc));
ret = PTR_ERR(desc);
break;
}
@@ -607,7 +607,7 @@ static int __init ubiblock_create_from_param(void)
ret = ubiblock_create(&vi);
if (ret) {
pr_err("UBI: block: can't add '%s' volume, err=%d\n",
- vi.name, ret);
+ vi.name, ret);
break;
}
}
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 802f29e..3405be4 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -960,8 +960,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
ubi->fm_disabled = 1;
}
- ubi_msg(ubi, "default fastmap pool size: %d",
- ubi->fm_pool.max_
Re: [PATCH v4] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Tue, 2014-10-14 at 12:31 -0700, Joe Perches wrote:
> On Tue, 2014-10-14 at 16:18 -0300, Ezequiel Garcia wrote:
> > On 14 Oct 12:13 PM, Joe Perches wrote:
> > > On Tue, 2014-10-14 at 15:47 -0300, Ezequiel Garcia wrote:
> []
> > > > When the gendisk is not available, a simple pr_{} would work.
> > >
> > > Or maybe combine these in the ubi_ calls passing
> > > NULL when there is no struct ubi_device *
> []
> > Isn't this excessive obfuscation? What's the benefit of it?
>
> Single error message type.
Well, these are 2 separate drivers just living in the same folder, so
they do not have to have to share message functions.
Artem.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Tue, 2014-10-14 at 09:05 -0700, Joe Perches wrote: > It's pretty trivial when all the lines are already > being touched. OK, but then the same change should done in UBIFS, because it's ubifs_msg() and so on macros are consistent with UBI macros. So I think if this is done, then it is done separately for both UBI and UBIFS. Artem. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Tue, 2014-10-14 at 11:39 -0300, Ezequiel Garcia wrote: > Please use some pr_fmt for this. Something like this before the headers > should be enough: > > #define pr_fmt(fmt) "UBI: block:" fmt Sinc ubiblock is a device, there should be a 'struct device' somewhere, so probably dev_printk() and other dev_*() printing functions would be a better choice? -- Artem -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Tue, 2014-10-14 at 07:33 -0700, Joe Perches wrote:
> If you are going to change all the ubi_ calls,
> can you also please add a terminating newline to all
> the uses for consistency with all the other
> pr_/dev_/_ calls?
I get the consistency argument.
On the other hand, this is about printing a single line. It is gets
prefixed (with "UBI: ") automatically, why wouldn't we append the
newline character automatically too?
In the generic functions this is for flexibility: rarely, people to want
to print a multi-line message with those. The first line will be
prefixed, the following line won't be prefixed.
We do not need that flexibility. And I think that adding hundreds of
'\n's just for the sake of consistency to be not very attractive option.
IOW, I do not support this suggestion.
> > > /* UBI error messages */
> > > -#define ubi_err(fmt, ...) pr_err("UBI error: %s: " fmt "\n", \
> > > - __func__, ##__VA_ARGS__)
> > > +#define ubi_err(ubi, fmt, ...) pr_err("UBI-%d error: %s: " fmt "\n",
> > > \
> > > + ubi->ubi_num, __func__, ##__VA_ARGS__)
>
> Converting these macros to functions using "%pV"
> will save quite a bit of text space by removing
> a lot of "UBI-%d : " duplication.
These were added before '%pV' existed, I think. I never used this printk
extension, but if it results in a more concise code, sounds like a good
idea. But I'd do this separately.
> Using ubi_notice instead of ubi_msg would be a
> lot more standard too.
Yes, this could be an OK separate nicification, I think, if someone is
willing to do this work. I would not put this item to my TODO list,
since this is a lot of changes for with little gain. But I would accept
such a patch, sure.
Thanks!
--
Artem.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Mon, 2014-10-13 at 18:37 +0300, Artem Bityutskiy wrote: > On Mon, 2014-10-06 at 14:01 +0300, Tanya Brokhman wrote: > > If there is more then one UBI device mounted, there is no way to > > distinguish between messages from different UBI devices. > > Add device number to all ubi layer message types. > > Looks good to me, pushed to the master branch of the linux-ubifs.git > tree. Later, when the merge window is closed, I'll merge this patch to > the linux-next branch too. Tanya, sorry, I was not careful enough, I merged it and tested against the Linuses head, it is fine. But it does not apply the the linux-ubifs.git tree. There are conflicts. But more importantly, you did not get the 'block.c' right. There we use the same printing macros, but we do not have 'struct ubi_info' there at all. Please, enable the R/O block layer feature and try to compile, it'll fail. The block driver is in 'drivers/mtd/ubi', but it is kind of a separate driver - it does not access the internal UBI data structures. I guess the solution would be to just use pr_* functions there instead. CCing Ezequiel. Please, submit a patch against the 'linux-next' branch of this tree: git://git.infradead.org/linux-ubifs.git Artem. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Mon, 2014-10-06 at 14:01 +0300, Tanya Brokhman wrote: > If there is more then one UBI device mounted, there is no way to > distinguish between messages from different UBI devices. > Add device number to all ubi layer message types. Looks good to me, pushed to the master branch of the linux-ubifs.git tree. Later, when the merge window is closed, I'll merge this patch to the linux-next branch too. Thanks! -- Artem -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Fri, 2014-10-03 at 17:50 +0200, Rafał Miłecki wrote:
> On 3 October 2014 17:27, Artem Bityutskiy wrote:
> > Yes, I guess a single patch is indeed OK. I have few nit-picks, though.
> >
> > On Tue, 2014-09-30 at 18:13 +0300, Tanya Brokhman wrote:
> >> - ubi_err("'ubi_io_read_ec_hdr()' returned unknown code %d",
> >> err);
> >> + ubi_err(ubi,
> >> + "'ubi_io_read_ec_hdr()' returned unknown code %d", err);
> >> return -EINVAL;
> >
> > I think it is fine if the line is long in these cases, let's keep the
> > message on the same line, this split does not contribute to better
> > readability, quite the opposite, in my opinion.
> >
> > One line:
> > ubi_err(ubi, "long line")
> >
> > Multiple lines:
> > ubi_err(ubi, "long line,
> > parameters)
>
> You should discuss that with checkpatch team, because ARAIR it will
> complain about "long line" with any other parameter in the same line.
I respect checkpatch.pl, and uniformity / consistency, but in this
particular case I put my maintainer hat on and say that for this kernel
subsystem it is fine, because the maintainer will be more efficient in
maintaining this code when the code is a bit mere readable for him.
Artem.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/5] mtd: ubi: Read disturb infrastructure
On Thu, 2014-10-02 at 16:05 +0200, Richard Weinberger wrote: > The question is, do we really need all this values on-flash? Good question, this is why I requested some kind of design description, which would clearly explain the problem. Just off-the top of my head, may be it could be enough to keep the counters only in the UBI headers, may be it is enough to work with some kind of averages. But it is hard to tell without clearly understanding the problem we are solving. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mtd: ubi: Extend UBI layer debug/messaging capabilities
Yes, I guess a single patch is indeed OK. I have few nit-picks, though.
On Tue, 2014-09-30 at 18:13 +0300, Tanya Brokhman wrote:
> - ubi_err("'ubi_io_read_ec_hdr()' returned unknown code %d", err);
> + ubi_err(ubi,
> + "'ubi_io_read_ec_hdr()' returned unknown code %d", err);
> return -EINVAL;
I think it is fine if the line is long in these cases, let's keep the
message on the same line, this split does not contribute to better
readability, quite the opposite, in my opinion.
One line:
ubi_err(ubi, "long line")
Multiple lines:
ubi_err(ubi, "long line,
parameters)
> - ubi_msg("\"delete\" compatible internal volume
> %d:%d found, will remove it",
> + ubi_msg(ubi,
> +"\"delete\" compatible internal volume %d:%d
> found, will remove it",
> vol_id, lnum);
Ditto.
> - ubi_msg("read-only compatible internal volume %d:%d
> found, switch to read-only mode",
> + ubi_msg(ubi,
> + "read-only compatible internal volume %d:%d
> found, switch to read-only mode",
> vol_id, lnum);
Ditto. And so on, in many places.
> - ubi_err("bad compat %d", vidh->compat);
> + ubi_err(ubi,
> + "bad compat %d", vidh->compat);
And for consistency, places like this would be:
ubi_err(ubi, bad compat %d",
vidh->compat);
> if (av->used_ebs != be32_to_cpu(vidh->used_ebs)) {
> - ubi_err("bad used_ebs %d", av->used_ebs);
> + ubi_err(ubi,
> + "bad used_ebs %d", av->used_ebs);
Ditto for all the similar cases.
> - ubi_msg("volume %d (\"%s\") re-sized from %d to %d LEBs", vol_id,
> + ubi_msg(ubi, "volume %d (\"%s\") re-sized from %d to %d LEBs",
> + vol_id,
> vol->name, old_reserved_pebs, vol->reserved_pebs);
Please, in cases like this, try to pack more arguments to the second
line, and move those which do not fit there to the third line. So this
would be like:
ubi_msg(ubi, "volume %d (\"%s\") re-sized from %d to %d LEBs",
vol_id, vol->name, old_reserved_pebs,
vol->reserved_pebs);
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Sun, 2014-09-28 at 11:24 +0300, Tanya Brokhman wrote: > This is how I first implemented this patch. But then I cam across > some > ubi_err prints during the init process (in build.c ubi_init()) when > we > still don't have the ubi structure, so we need to pass some number to > the message. I overcame this by using UBI_MAX_DEVICES. Well, may be having two sets of wrappers would make sense then? Or if there are few of those, just using 'printk()' would be a solution? -- Best Regards, Artem Bityutskiy -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Sun, 2014-09-28 at 10:01 +0200, Richard Weinberger wrote: > Tanya, > > Am 28.09.2014 08:36, schrieb Tanya Brokhman: > > If there is more then one UBI device mounted, there is no way to > > distinguish between messages from different UBI devices. > > Add device number to all ubi layer message types. > > > > > > Signed-off-by: Tanya Brokhman > > Artem's mail is dedekind1@ not dedeking1@. :) Yeah, it is not after a King named "Dede" :-) I just liked the Richard Dedekind's theorem back in the University days :-) > Anyway, instead of passing ubi->ubi_num down to every log function you can > just pass the ubi object itself and let the log function access ->ubi_num. Yes, then we potentially may add more than just the device number to the messages, if we ever need. -- Best Regards, Artem Bityutskiy -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/5] mtd: ubi: Read disturb infrastructure
On Sun, 2014-09-28 at 09:37 +0300, Tanya Brokhman wrote: > The need for performing read disturb is determined according to new > statistics collected per eraseblock: > - read counter: incremented at each read operation > reset at each erase > - last erase time stamp: updated at each erase > > This patch adds the infrastructure for the above statistics Would you please provide some kind of high level description for this stuff. What is the problem at hand, how is it solved. Right off-the top of my head I have the following comment. Adding more fields to 'struct ubi_wl_entry' should be well-justified. These objects are per-PEB, so there may be really a lot of them, and they may consume a lot of memory. Increasing the size of the object may be affect the memory consumption a lot. So I wonder if these read counters solve a big enough problem, so that they are worth having in this data structure. So I am really missing the bigger picture here. -- Best Regards, Artem Bityutskiy -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mtd: ubi: Extend UBI layer debug/messaging capabilities
On Sun, 2014-09-28 at 09:36 +0300, Tanya Brokhman wrote: > If there is more then one UBI device mounted, there is no way to > distinguish between messages from different UBI devices. > Add device number to all ubi layer message types. Hi, the goal looks legit to me, but the patch is so large that I do not think that I can really review it in this form. a) A patch which changes the macros (ubi_err(), etc) b) A set of patches which do not change messages at all, but add the 'ubi' parameter to the places where it is missing. c) A patch which changes the messages. So a) will be the most important patch for the reviewer. b) - more or less mechanical changes of a similar kind. c) - the same. Also, if you add a parameter to 'ubi_err()' and the other printing wrappers, add 'ubi' there, not 'ubi_num'. This will allow to prefix messages with vary different things, not just the device number in the future. So the calls would look like ubi_err(ubi, "inconsistent used_ebs"); Once this is done, the series should be more reviewable. The next thing I'd check is whether we really need to change all the messages, or most of them, or we actually need to change only a small part of them. In the former case, it is OK to do what you do, I guess. In the latter case we probably better off with introducing a separate set of printing macros and leave the existing ones as they are. Thanks! -- Best Regards, Artem Bityutskiy -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 2/2] mtd: msm_nand: Add initial msm nand driver support.
On Mon, 2011-04-18 at 09:32 +0200, Matthieu CASTET wrote: > Hi, > > Murali Nalajala a écrit : > > Add initial msm nand driver support for Qualcomm MSM platforms. > > This driver is capable of handling both 2k and 4k page support > > nand devices. > > > > This driver was originally developed by Arve Hjønnevåg at google. > > Its source is available at > > http://android.git.kernel.org/?p=kernel/msm.git under > > android-msm-2.6.35 branch. > > > > CC: Dima Zavin > > CC: Brian Swetland > > CC: Arve Hjønnevåg > > Signed-off-by: Murali Nalajala > > --- > > Changes in V2 > > * Turn most of the pr_info() calls into pr_debug(). > > > > drivers/mtd/devices/Kconfig| 11 + > > drivers/mtd/devices/Makefile |1 + > > drivers/mtd/devices/msm_nand.c | 1597 > > > > drivers/mtd/devices/msm_nand.h | 77 ++ > > 4 files changed, 1686 insertions(+), 0 deletions(-) > > create mode 100644 drivers/mtd/devices/msm_nand.c > > create mode 100644 drivers/mtd/devices/msm_nand.h > > > > For the record, I am not sure it is a great idea to make nand drivers > independent of the nand layer. This is not acceptable. Nand drivers should use the nand_base framework. If the framework is limiting, buggy, slow, etc for your case (which is quite possible) - the framework should be improved. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mtd: msm_nand: Add initial msm nand driver support.
> Can i export this variable now? I do not think so, we do not export random variables. > What is your suggestion? But I already did provide you suggestions - kill this global variable completely. I provided the reasons: 1. It is error prone to hide functions behind something which looks like a constant. 2. Multiple controllers with different base addresses wouldn't work - your driver has to be able to at least theoretically handle multiple controllers with different bases. -- Best Regards, Artem Bityutskiy (Битюцкий Артём) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mtd: msm_nand: Add initial msm nand driver support.
On Sat, 2011-04-16 at 11:20 +0300, Artem Bityutskiy wrote: > > > +#define MSM_NAND_REG(off) (msm_nand_phys + (off)) > > > + > > > +#define MSM_NAND_FLASH_CMDMSM_NAND_REG(0x) > > > +#define MSM_NAND_ADDR0MSM_NAND_REG(0x0004) > > > > > > Could you please make the macros to take the "struct msm_nand_chip > > > *chip" argument instead, and store the pase address there. Do not hide > > > the fact that those macros are actually functions, not constant - this > > > is error prone. > > > > > > Besides, I'm do not know your HW, but if you have several controllers > > > with various base addresses - your driver won't work. > > > > you are correct, we have multiple controllers, which breaks this logic > > in future. > > So then make your macros to accept the base address as an argument > instead please. Or better make macros to be constants, and always use something like base + MSM_NAND_FLASH_CMD - this is the standard approach. -- Best Regards, Artem Bityutskiy (Битюцкий Артём) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mtd: msm_nand: Add initial msm nand driver support.
On Fri, 2011-04-15 at 20:20 +0530, Murali Nalajala wrote: > On 4/15/2011 3:21 PM, Artem Bityutskiy wrote: > > On Tue, 2011-03-01 at 06:17 +0530, Murali Nalajala wrote: > >> +#define pr_fmt(fmt) "%s:" fmt, __func__ > >> + > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> + > >> +#include "msm_nand.h" > >> + > >> +unsigned long msm_nand_phys; > > > > No global variables like this please. Here is how you use them: > > > > +extern unsigned long msm_nand_phys; > > extern declaration in the 'C' file causing warning. We are in plan to > upload the OneNAND changes soon which makes use of the same .h file. So you say that OneNAND (an independent driver) is going to use this variable? Are you also going to export it? > > +#define MSM_NAND_REG(off) (msm_nand_phys + (off)) > > + > > +#define MSM_NAND_FLASH_CMDMSM_NAND_REG(0x) > > +#define MSM_NAND_ADDR0MSM_NAND_REG(0x0004) > > > > Could you please make the macros to take the "struct msm_nand_chip > > *chip" argument instead, and store the pase address there. Do not hide > > the fact that those macros are actually functions, not constant - this > > is error prone. > > > > Besides, I'm do not know your HW, but if you have several controllers > > with various base addresses - your driver won't work. > > you are correct, we have multiple controllers, which breaks this logic > in future. So then make your macros to accept the base address as an argument instead please. -- Best Regards, Artem Bityutskiy (Битюцкий Артём) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mtd: msm_nand: Add initial msm nand driver support.
On Tue, 2011-03-01 at 06:17 +0530, Murali Nalajala wrote:
> +#define pr_fmt(fmt) "%s:" fmt, __func__
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "msm_nand.h"
> +
> +unsigned long msm_nand_phys;
No global variables like this please. Here is how you use them:
+extern unsigned long msm_nand_phys;
+#define MSM_NAND_REG(off) (msm_nand_phys + (off))
+
+#define MSM_NAND_FLASH_CMDMSM_NAND_REG(0x)
+#define MSM_NAND_ADDR0MSM_NAND_REG(0x0004)
Could you please make the macros to take the "struct msm_nand_chip
*chip" argument instead, and store the pase address there. Do not hide
the fact that those macros are actually functions, not constant - this
is error prone.
Besides, I'm do not know your HW, but if you have several controllers
with various base addresses - your driver won't work.
> + pr_info("Save cfg0 = %x cfg1 = %x\n", chip->cfg0, chip->cfg1);
> + pr_info("cfg0: cw/page=%d ud_sz=%d ecc_sz=%d spare_sz=%d "
> + "num_addr_cycles=%d\n", (chip->cfg0 >> 6) & 7,
> + (chip->cfg0 >> 9) & 0x3ff, (chip->cfg0 >> 19) & 15,
> + (chip->cfg0 >> 23) & 15, (chip->cfg0 >> 27) & 7);
Please, revise all your pr_info() calls and turn most of them into
dev_dbg() or pr_debug. Your driver should be completely silent by
default, except of error messages and some information messages when it
is initialized.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/2] mtd: nand: Allow NAND chip ids to be included standalone.
On Wed, 2011-04-06 at 10:08 +0530, Murali Nalajala wrote: > On 3/22/2011 1:41 PM, Artem Bityutskiy wrote: > > On Tue, 2011-03-22 at 12:11 +0530, Murali Nalajala wrote: > >> Still you have any issues/comments? > > > > No, not so far, just have not time now to look closer. > > > > > Did anybody got a chance to go through the patch? Could you start with trying to get an ack or reviewed-by from some maintainer form Codeaurora, e.g., David Brown? Unfortunately, MTD community does not have enough people who'd actively review other's work. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/2] mtd: nand: Allow NAND chip ids to be included standalone.
On Tue, 2011-03-22 at 12:11 +0530, Murali Nalajala wrote: > Still you have any issues/comments? No, not so far, just have not time now to look closer. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/2] mtd: nand: Allow NAND chip ids to be included standalone.
On Mon, 2011-03-21 at 12:09 +0530, Murali Nalajala wrote: > On 3/8/2011 11:11 PM, Murali Nalajala wrote: > > On 3/8/2011 12:50 AM, Artem Bityutskiy wrote: > >> On Tue, 2011-03-08 at 05:38 +0530, Murali Nalajala wrote: > >>> Lets non-standard NAND drivers take advantage of known NAND > >>> chip information. > >>> > >>> The initial development of msm nand driver, driver uses the supported > >>> NAND devices information as a hardcoded table. Remove the existing > >>> hardcoded supported flash device table and read the flash device > >>> information from the flash id table which are part of the mtd subsystem. > >> > >> Why this initial version should be upstream? Why wouldn't you make it > >> "standard" first? > >> > > > > Currently we are not fully using the MTD nand subsystem. That's the > > intention author has introduced "non-standard" here!!! > > > > Thanks, > > Murali N > > > > Any reviews comments on this change? No, I think you should "sell" your driver better than that. Indeed, Q: "Why this initial version should be upstream? Why wouldn't you make it "standard" first?" A: Currently we are not fully using the MTD nand subsystem. That's the intention author has introduced "non-standard" here!!! -- Best Regards, Artem Bityutskiy (Артём Битюцкий) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/2] mtd: nand: Allow NAND chip ids to be included standalone.
On Tue, 2011-03-08 at 05:38 +0530, Murali Nalajala wrote: > Lets non-standard NAND drivers take advantage of known NAND > chip information. > > The initial development of msm nand driver, driver uses the supported > NAND devices information as a hardcoded table. Remove the existing > hardcoded supported flash device table and read the flash device > information from the flash id table which are part of the mtd subsystem. Why this initial version should be upstream? Why wouldn't you make it "standard" first? -- Best Regards, Artem Bityutskiy (Артём Битюцкий) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mtd: nand: Allow NAND chip ids to be included standalone.
On Tue, 2011-03-01 at 06:17 +0530, Murali Nalajala wrote: > Lets non-standard NAND drivers take advantage of known NAND > chip information. > > Signed-off-by: Dima Zavin > Signed-off-by: Murali Nalajala Please, make the description more descriptive. Please, make it explain which problem you are solving and how. What do you mean by non-standard drivers? Why they are non-standard? -- Best Regards, Artem Bityutskiy (Артём Битюцкий) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] mtd: msm_nand: Add initial msm nand driver support.
On Thu, 2011-01-06 at 09:39 -0800, Daniel Walker wrote: > On Wed, 2011-01-05 at 22:39 +0200, Artem Bityutskiy wrote: > > On Wed, 2011-01-05 at 09:12 -0800, Daniel Walker wrote: > > > On Wed, 2011-01-05 at 10:41 +0200, Artem Bityutskiy wrote: > > > > On Fri, 2010-12-31 at 14:24 +0530, Murali Nalajala wrote: > > > > > From: Arve Hjønnevåg > > > > > > > > > > Add initial msm nand driver support for Qualcomm MSM and QSD > > > > > platforms. > > > > > This driver is currently capable of handling 2K page nand devices. > > > > > > > > > > This driver is originally > > > > > developed by Google and its source is available at > > > > > http://android.git.kernel.org/?p=kernel/experimental.git > > > > > > > > > > CC: Brian Swetland > > > > > Signed-off-by: Arve Hjønnevåg > > > > > Signed-off-by: Murali Nalajala > > > > > > > > Pushed to l2-mtd-2.6.git. > > > > > > This patch had incorrect authorship .. Can you replace it with the > > > second one that was sent ? > > > > Yes, I did that actually, noticed the second one later. > > Is it too late for you to drop this ? It's got some additional issues > that we need some time to address .. Sure, I do not merge stuff to upstream, I barely take it to my tree to make sure David Woodhouse does not miss it. Everything he does not merge to mtd tree is automatically dropped from my l2-mtd then. -- Best Regards, Artem Bityutskiy (Битюцкий Артём) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] mtd: msm_nand: Add initial msm nand driver support.
On Wed, 2011-01-05 at 09:12 -0800, Daniel Walker wrote: > On Wed, 2011-01-05 at 10:41 +0200, Artem Bityutskiy wrote: > > On Fri, 2010-12-31 at 14:24 +0530, Murali Nalajala wrote: > > > From: Arve Hjønnevåg > > > > > > Add initial msm nand driver support for Qualcomm MSM and QSD platforms. > > > This driver is currently capable of handling 2K page nand devices. > > > > > > This driver is originally > > > developed by Google and its source is available at > > > http://android.git.kernel.org/?p=kernel/experimental.git > > > > > > CC: Brian Swetland > > > Signed-off-by: Arve Hjønnevåg > > > Signed-off-by: Murali Nalajala > > > > Pushed to l2-mtd-2.6.git. > > This patch had incorrect authorship .. Can you replace it with the > second one that was sent ? Yes, I did that actually, noticed the second one later. -- Best Regards, Artem Bityutskiy (Битюцкий Артём) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] mtd: msm_nand: Add initial msm nand driver support.
On Fri, 2010-12-31 at 14:24 +0530, Murali Nalajala wrote: > From: Arve Hjønnevåg > > Add initial msm nand driver support for Qualcomm MSM and QSD platforms. > This driver is currently capable of handling 2K page nand devices. > > This driver is originally > developed by Google and its source is available at > http://android.git.kernel.org/?p=kernel/experimental.git > > CC: Brian Swetland > Signed-off-by: Arve Hjønnevåg > Signed-off-by: Murali Nalajala Pushed to l2-mtd-2.6.git. -- Best Regards, Artem Bityutskiy (Битюцкий Артём) -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
