diff mbox

about the _dtype_ parameter

Message ID 1231168447.6608.26.camel@localhost.localdomain
State New, archived
Headers show

Commit Message

Artem Bityutskiy Jan. 5, 2009, 3:14 p.m. UTC
On Mon, 2009-01-05 at 22:59 +0800, xiaochuan xu wrote:
> Hi,
> 
> >> In my test, I found that the third parameter of ubi_wl_get_peb()
> >> function _dtype_ is always UBI_SHORTTERM.
> 
> >Which test?
> I write a temporary file in the ubifs with O_TRUNC flag over and over.
> I see that all the @dtype variables are equal to 2 not 3.
> why does this happen?

Hmm, indeed, UBIFS uses UBI_SHORTTERM in 'reserve_space()', which looks
wrong - thanks for noticing. Could you please try the attached patch?

> Even though, as you say, most of the data are type of UBI_UNKNOWN,
> the @dtype variable seems to make less sense in the current implemenation.
> If this is ture, I think it's necessary to implement a
> dynamic-data-type-classification
> policy.

Agree. I'll be happy to merge such an improvement, but I do not have
plans to do this myself.

Patch:

Comments

xiaochuan-xu Jan. 12, 2009, 9:26 a.m. UTC | #1
Hi Artem,

>> I write a temporary file in the ubifs with O_TRUNC flag over and over.
>> I see that all the @dtype variables are equal to 2 not 3.
>> why does this happen?

> Patch:
> 
> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> index 10ae25b..6a885c5 100644
> --- a/fs/ubifs/journal.c
> +++ b/fs/ubifs/journal.c
> @@ -208,7 +208,7 @@ again:
>  offs = 0;
> 
> out:
> - err = ubifs_wbuf_seek_nolock(wbuf, lnum, offs, UBI_SHORTTERM);
> + err = ubifs_wbuf_seek_nolock(wbuf, lnum, offs, wbuf->dtype);
>  if (err)
>  goto out_unlock;
> 

I'v applied this patch, but the @dtype variables are still equal to UBI_SHORTTERM (2)! :-(
Are there some more bugs?

Best regards,
Xiaochuan-Xu
Artem Bityutskiy Jan. 13, 2009, 9:41 a.m. UTC | #2
On Mon, 2009-01-12 at 17:26 +0800, xiaochuan-xu wrote:
> Hi Artem,
> 
> >> I write a temporary file in the ubifs with O_TRUNC flag over and over.
> >> I see that all the @dtype variables are equal to 2 not 3.
> >> why does this happen?
> 
> > Patch:
> > 
> > diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> > index 10ae25b..6a885c5 100644
> > --- a/fs/ubifs/journal.c
> > +++ b/fs/ubifs/journal.c
> > @@ -208,7 +208,7 @@ again:
> >  offs = 0;
> > 
> > out:
> > - err = ubifs_wbuf_seek_nolock(wbuf, lnum, offs, UBI_SHORTTERM);
> > + err = ubifs_wbuf_seek_nolock(wbuf, lnum, offs, wbuf->dtype);
> >  if (err)
> >  goto out_unlock;
> > 
> 
> I'v applied this patch, but the @dtype variables are still equal to UBI_SHORTTERM (2)! :-(
> Are there some more bugs?

Tried the latest ubifs-2.6 tree and @dtype is always UBI_UNKNOWN (3).
This is also strange though, I'll look at this.
Artem Bityutskiy Jan. 14, 2009, 12:48 p.m. UTC | #3
On Thu, 2009-01-15 at 01:02 +0800, xiaochuanxv wrote:
> Sorry, a mistake in writing: 
> 
> (Suppose wr-ratio represent the ratio of erasures due to wear-leveling
> to erasure total physical erase-block erasures. )
> I found that when most of the @dtype is equal to UBI_SHORTTERM, the
> wr-ratio is approximately equal to 0.1%, whereas is greater than
>  
> 0.4% 
> 
> when @dtype is always UBI_UNKNOWN. It seem that the data type call-back
> policy does NOT make any sense in the current implementation.

Hmm, interesting observation. I would suggest you to add statistics
support to UBI. Introduce a counter for the amount of times UBI moves
PEB data, and expose it via sysfs. You may probably add other counters
like number of erasures and so on.

Then it will be easier to do testing and investigations.
xiaochuan-xu Jan. 14, 2009, 4:37 p.m. UTC | #4
Hi,

On Tue, 2009-01-13 at 11:41 +0200, Artem Bityutskiy wrote:
> 
> Tried the latest ubifs-2.6 tree and @dtype is always UBI_UNKNOWN (3).
> This is also strange though, I'll look at this.

Unfortunately, if the @dtype is always UBI_UNKNOWN, this result in a
great increase in the physical erase-block erasures due to wear-leveling
processes.

(Suppose wr-ratio represent the ratio of erasures due to wear-leveling
to erasure total physical erase-block erasures. )
I found that when most of the @dtype is equal to UBI_SHORTTERM, the
wr-ratio is approximately equal to 0.1%, whereas is greater than 4% when
@dtype is always UBI_UNKNOWN. It seem that the data type call-back
policy does NOT make any sense in the current implementation.

FYI, An simple way is of making all data type UBI_SHORTTERM only,
although a bit of data are long term data. Go further and say, @dtype
can be got rid off from UBI clients (e.g., UBIFS), or classifying the
data type as TWO type(UBI_SHORTTERM and UBI_LOANGTERM).
xiaochuan-xu Jan. 14, 2009, 5:02 p.m. UTC | #5
Sorry, a mistake in writing: 

(Suppose wr-ratio represent the ratio of erasures due to wear-leveling
to erasure total physical erase-block erasures. )
I found that when most of the @dtype is equal to UBI_SHORTTERM, the
wr-ratio is approximately equal to 0.1%, whereas is greater than
 
0.4% 

when @dtype is always UBI_UNKNOWN. It seem that the data type call-back
policy does NOT make any sense in the current implementation.
diff mbox

Patch

diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 10ae25b..6a885c5 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -208,7 +208,7 @@  again:
 	offs = 0;
 
 out:
-	err = ubifs_wbuf_seek_nolock(wbuf, lnum, offs, UBI_SHORTTERM);
+	err = ubifs_wbuf_seek_nolock(wbuf, lnum, offs, UBI_UNKNOWN);
 	if (err)
 		goto out_unlock;