Re: about the _dtype_ parameter

Re: about the _dtype_ parameter

[Artem Bityutskiy]

Hi,

On Sun, 2009-01-04 at 01:07 +0800, 许小川 wrote:
> In my test, I found that the third parameter of ubi_wl_get_peb()
> function _dtype_ is always UBI_SHORTTERM.

Which test?

> How does the UBIFS classify the data type?

Did you grep for this fs/ubifs? I think mostly we use UBI_UNKNOWN.

> and would we better classify these UBI_SHORTTERM data more
> fine granularity? Which is good for efficient wear-leveling scheme
> IMO.

May be. I've never done any good experiments for this.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: about the _dtype_ parameter

[Artem Bityutskiy]

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:

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;

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: about the _dtype_ parameter

[xiaochuan-xu]

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

Re: about the _dtype_ parameter

[Artem Bityutskiy]

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.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: about the _dtype_ parameter

[xiaochuanxv]

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).


-- 
Best regards,
Xiaochuan Xu (许小川)

Re: about the _dtype_ parameter

[xiaochuanxv]

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.

-- 
Best regards,
Xiaochuan Xu (许小川)

Re: about the _dtype_ parameter

[Artem Bityutskiy]

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.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)