| Message ID | 1380376516-30144-2-git-send-email-richard@nod.at |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, 2013-09-28 at 15:55 +0200, Richard Weinberger wrote: > If no free PEBs are available refill_wl_user_pool() must not > return with -ENOSPC immediately. > It has to block till produce_free_peb() produced a free PEB. > > Reported-and-Tested-by: Richard Genoud <richard.genoud@gmail.com> > Signed-off-by: Richard Weinberger <richard@nod.at> What is pool size, I wonder?
Am 03.10.2013 17:00, schrieb Artem Bityutskiy: > On Sat, 2013-09-28 at 15:55 +0200, Richard Weinberger wrote: >> If no free PEBs are available refill_wl_user_pool() must not >> return with -ENOSPC immediately. >> It has to block till produce_free_peb() produced a free PEB. >> >> Reported-and-Tested-by: Richard Genoud <richard.genoud@gmail.com> >> Signed-off-by: Richard Weinberger <richard@nod.at> > > What is pool size, I wonder? Currently it's 25 (UBI_FM_WL_POOL_SIZE). If experience shows that 25 is too low/big we can change this constant. Maybe it's also worth making them configurable... Thanks, //richard
On Thu, 2013-10-03 at 17:08 +0200, Richard Weinberger wrote: > Am 03.10.2013 17:00, schrieb Artem Bityutskiy: > > On Sat, 2013-09-28 at 15:55 +0200, Richard Weinberger wrote: > >> If no free PEBs are available refill_wl_user_pool() must not > >> return with -ENOSPC immediately. > >> It has to block till produce_free_peb() produced a free PEB. > >> > >> Reported-and-Tested-by: Richard Genoud <richard.genoud@gmail.com> > >> Signed-off-by: Richard Weinberger <richard@nod.at> > > > > What is pool size, I wonder? > > Currently it's 25 (UBI_FM_WL_POOL_SIZE). > If experience shows that 25 is too low/big we can change this constant. > Maybe it's also worth making them configurable... I if it is a possible scenario that this function will not return until 25 (or even 10) PEBs are erased?
Am 03.10.2013 17:27, schrieb Artem Bityutskiy: > On Thu, 2013-10-03 at 17:08 +0200, Richard Weinberger wrote: >> Am 03.10.2013 17:00, schrieb Artem Bityutskiy: >>> On Sat, 2013-09-28 at 15:55 +0200, Richard Weinberger wrote: >>>> If no free PEBs are available refill_wl_user_pool() must not >>>> return with -ENOSPC immediately. >>>> It has to block till produce_free_peb() produced a free PEB. >>>> >>>> Reported-and-Tested-by: Richard Genoud <richard.genoud@gmail.com> >>>> Signed-off-by: Richard Weinberger <richard@nod.at> >>> >>> What is pool size, I wonder? >> >> Currently it's 25 (UBI_FM_WL_POOL_SIZE). >> If experience shows that 25 is too low/big we can change this constant. >> Maybe it's also worth making them configurable... > > I if it is a possible scenario that this function will not return until > 25 (or even 10) PEBs are erased? > Sure. It will try to gain up to 25 PEBs but return if it gets less. That's why a pool has a current and a max size. Thanks, //richard
On Thu, 2013-10-03 at 17:53 +0200, Richard Weinberger wrote: > Am 03.10.2013 17:27, schrieb Artem Bityutskiy: > > On Thu, 2013-10-03 at 17:08 +0200, Richard Weinberger wrote: > >> Am 03.10.2013 17:00, schrieb Artem Bityutskiy: > >>> On Sat, 2013-09-28 at 15:55 +0200, Richard Weinberger wrote: > >>>> If no free PEBs are available refill_wl_user_pool() must not > >>>> return with -ENOSPC immediately. > >>>> It has to block till produce_free_peb() produced a free PEB. > >>>> > >>>> Reported-and-Tested-by: Richard Genoud <richard.genoud@gmail.com> > >>>> Signed-off-by: Richard Weinberger <richard@nod.at> > >>> > >>> What is pool size, I wonder? > >> > >> Currently it's 25 (UBI_FM_WL_POOL_SIZE). > >> If experience shows that 25 is too low/big we can change this constant. > >> Maybe it's also worth making them configurable... > > > > I if it is a possible scenario that this function will not return until > > 25 (or even 10) PEBs are erased? > > > > Sure. It will try to gain up to 25 PEBs but return if it gets less. > That's why a pool has a current and a max size. So if erasing speed is say, 250ms, then it would take 6.25 seconds?
Am 03.10.2013 18:00, schrieb Artem Bityutskiy: > On Thu, 2013-10-03 at 17:53 +0200, Richard Weinberger wrote: >> Am 03.10.2013 17:27, schrieb Artem Bityutskiy: >>> On Thu, 2013-10-03 at 17:08 +0200, Richard Weinberger wrote: >>>> Am 03.10.2013 17:00, schrieb Artem Bityutskiy: >>>>> On Sat, 2013-09-28 at 15:55 +0200, Richard Weinberger wrote: >>>>>> If no free PEBs are available refill_wl_user_pool() must not >>>>>> return with -ENOSPC immediately. >>>>>> It has to block till produce_free_peb() produced a free PEB. >>>>>> >>>>>> Reported-and-Tested-by: Richard Genoud <richard.genoud@gmail.com> >>>>>> Signed-off-by: Richard Weinberger <richard@nod.at> >>>>> >>>>> What is pool size, I wonder? >>>> >>>> Currently it's 25 (UBI_FM_WL_POOL_SIZE). >>>> If experience shows that 25 is too low/big we can change this constant. >>>> Maybe it's also worth making them configurable... >>> >>> I if it is a possible scenario that this function will not return until >>> 25 (or even 10) PEBs are erased? >>> >> >> Sure. It will try to gain up to 25 PEBs but return if it gets less. >> That's why a pool has a current and a max size. > > So if erasing speed is say, 250ms, then it would take 6.25 seconds? Only in the very worst case if we have to call 25 times produce_free_peb(). Of course we could add a check to return immediately if produce_free_peb() got called a few times in series. But I really would like to wait with such performance tweaks until fastmap is more mature. Thanks, //richard
On Thu, 2013-10-03 at 18:35 +0200, Richard Weinberger wrote: > Am 03.10.2013 18:00, schrieb Artem Bityutskiy: > > On Thu, 2013-10-03 at 17:53 +0200, Richard Weinberger wrote: > >> Am 03.10.2013 17:27, schrieb Artem Bityutskiy: > >>> On Thu, 2013-10-03 at 17:08 +0200, Richard Weinberger wrote: > >>>> Am 03.10.2013 17:00, schrieb Artem Bityutskiy: > >>>>> On Sat, 2013-09-28 at 15:55 +0200, Richard Weinberger wrote: > >>>>>> If no free PEBs are available refill_wl_user_pool() must not > >>>>>> return with -ENOSPC immediately. > >>>>>> It has to block till produce_free_peb() produced a free PEB. > >>>>>> > >>>>>> Reported-and-Tested-by: Richard Genoud <richard.genoud@gmail.com> > >>>>>> Signed-off-by: Richard Weinberger <richard@nod.at> > >>>>> > >>>>> What is pool size, I wonder? > >>>> > >>>> Currently it's 25 (UBI_FM_WL_POOL_SIZE). > >>>> If experience shows that 25 is too low/big we can change this constant. > >>>> Maybe it's also worth making them configurable... > >>> > >>> I if it is a possible scenario that this function will not return until > >>> 25 (or even 10) PEBs are erased? > >>> > >> > >> Sure. It will try to gain up to 25 PEBs but return if it gets less. > >> That's why a pool has a current and a max size. > > > > So if erasing speed is say, 250ms, then it would take 6.25 seconds? > > Only in the very worst case if we have to call 25 times produce_free_peb(). > > Of course we could add a check to return immediately if produce_free_peb() > got called a few times in series. > But I really would like to wait with such performance tweaks until fastmap > is more mature. OK, but how about at least adding a comment talking about this unlikely scenario?
Am 03.10.2013 18:41, schrieb Artem Bityutskiy: > On Thu, 2013-10-03 at 18:35 +0200, Richard Weinberger wrote: >> Am 03.10.2013 18:00, schrieb Artem Bityutskiy: >>> On Thu, 2013-10-03 at 17:53 +0200, Richard Weinberger wrote: >>>> Am 03.10.2013 17:27, schrieb Artem Bityutskiy: >>>>> On Thu, 2013-10-03 at 17:08 +0200, Richard Weinberger wrote: >>>>>> Am 03.10.2013 17:00, schrieb Artem Bityutskiy: >>>>>>> On Sat, 2013-09-28 at 15:55 +0200, Richard Weinberger wrote: >>>>>>>> If no free PEBs are available refill_wl_user_pool() must not >>>>>>>> return with -ENOSPC immediately. >>>>>>>> It has to block till produce_free_peb() produced a free PEB. >>>>>>>> >>>>>>>> Reported-and-Tested-by: Richard Genoud <richard.genoud@gmail.com> >>>>>>>> Signed-off-by: Richard Weinberger <richard@nod.at> >>>>>>> >>>>>>> What is pool size, I wonder? >>>>>> >>>>>> Currently it's 25 (UBI_FM_WL_POOL_SIZE). >>>>>> If experience shows that 25 is too low/big we can change this constant. >>>>>> Maybe it's also worth making them configurable... >>>>> >>>>> I if it is a possible scenario that this function will not return until >>>>> 25 (or even 10) PEBs are erased? >>>>> >>>> >>>> Sure. It will try to gain up to 25 PEBs but return if it gets less. >>>> That's why a pool has a current and a max size. >>> >>> So if erasing speed is say, 250ms, then it would take 6.25 seconds? >> >> Only in the very worst case if we have to call 25 times produce_free_peb(). >> >> Of course we could add a check to return immediately if produce_free_peb() >> got called a few times in series. >> But I really would like to wait with such performance tweaks until fastmap >> is more mature. > > OK, but how about at least adding a comment talking about this unlikely > scenario? I'm fine with this. Will send a patch. :-) Thanks, //richard
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index c95bfb1..02317c1 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c @@ -599,10 +599,6 @@ static void refill_wl_user_pool(struct ubi_device *ubi) return_unused_pool_pebs(ubi, pool); for (pool->size = 0; pool->size < pool->max_size; pool->size++) { - if (!ubi->free.rb_node || - (ubi->free_count - ubi->beb_rsvd_pebs < 1)) - break; - pool->pebs[pool->size] = __wl_get_peb(ubi); if (pool->pebs[pool->size] < 0) break;
If no free PEBs are available refill_wl_user_pool() must not return with -ENOSPC immediately. It has to block till produce_free_peb() produced a free PEB. Reported-and-Tested-by: Richard Genoud <richard.genoud@gmail.com> Signed-off-by: Richard Weinberger <richard@nod.at> --- drivers/mtd/ubi/wl.c | 4 ---- 1 file changed, 4 deletions(-)