All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
@ 2014-04-01  8:01 ` Tanya Brokhman
  0 siblings, 0 replies; 15+ messages in thread
From: Tanya Brokhman @ 2014-04-01  8:01 UTC (permalink / raw)
  To: dedekind1; +Cc: linux-mtd, Tanya Brokhman, open list

At first mount it's possible that there are not enough free PEBs since
there are PEB's pending to be erased. In such scenario, fm_pool (which is
the pool from which user required PEBs are allocated) will be empty.
Try fixing the above described situation by synchronously performing
pending erase work, thus produce another free PEB.

Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>

diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 457ead3..9a36f78 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -595,10 +595,29 @@ static void refill_wl_pool(struct ubi_device *ubi)
 static void refill_wl_user_pool(struct ubi_device *ubi)
 {
 	struct ubi_fm_pool *pool = &ubi->fm_pool;
+	int err;
 
 	return_unused_pool_pebs(ubi, pool);
 
 	for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
+retry:
+		if (!ubi->free.rb_node ||
+		   (ubi->free_count - ubi->beb_rsvd_pebs < 1)) {
+			/*
+			 * There are no available PEBs. Try to free
+			 * PEB by means of synchronous execution of
+			 * pending works.
+			 */
+			if (ubi->works_count == 0)
+				break;
+			spin_unlock(&ubi->wl_lock);
+			err = do_work(ubi);
+			spin_lock(&ubi->wl_lock);
+			if (err < 0)
+				break;
+			goto retry;
+		}
+
 		pool->pebs[pool->size] = __wl_get_peb(ubi);
 		if (pool->pebs[pool->size] < 0)
 			break;
-- 
1.7.6
-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
@ 2014-04-01  8:01 ` Tanya Brokhman
  0 siblings, 0 replies; 15+ messages in thread
From: Tanya Brokhman @ 2014-04-01  8:01 UTC (permalink / raw)
  To: dedekind1; +Cc: linux-mtd, open list, Tanya Brokhman

At first mount it's possible that there are not enough free PEBs since
there are PEB's pending to be erased. In such scenario, fm_pool (which is
the pool from which user required PEBs are allocated) will be empty.
Try fixing the above described situation by synchronously performing
pending erase work, thus produce another free PEB.

Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>

diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 457ead3..9a36f78 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -595,10 +595,29 @@ static void refill_wl_pool(struct ubi_device *ubi)
 static void refill_wl_user_pool(struct ubi_device *ubi)
 {
 	struct ubi_fm_pool *pool = &ubi->fm_pool;
+	int err;
 
 	return_unused_pool_pebs(ubi, pool);
 
 	for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
+retry:
+		if (!ubi->free.rb_node ||
+		   (ubi->free_count - ubi->beb_rsvd_pebs < 1)) {
+			/*
+			 * There are no available PEBs. Try to free
+			 * PEB by means of synchronous execution of
+			 * pending works.
+			 */
+			if (ubi->works_count == 0)
+				break;
+			spin_unlock(&ubi->wl_lock);
+			err = do_work(ubi);
+			spin_lock(&ubi->wl_lock);
+			if (err < 0)
+				break;
+			goto retry;
+		}
+
 		pool->pebs[pool->size] = __wl_get_peb(ubi);
 		if (pool->pebs[pool->size] < 0)
 			break;
-- 
1.7.6
-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* RE: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
  2014-04-01  8:01 ` Tanya Brokhman
@ 2014-04-07  5:21   ` Dolev Raviv
  -1 siblings, 0 replies; 15+ messages in thread
From: Dolev Raviv @ 2014-04-07  5:21 UTC (permalink / raw)
  To: 'Tanya Brokhman', dedekind1; +Cc: linux-mtd, 'open list'

Reviewed-by: Dolev Raviv <draviv@codeaurora.org>

Thanks,
Dolev
-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


-----Original Message-----
From: linux-mtd [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of
Tanya Brokhman
Sent: Tuesday, April 01, 2014 11:02 AM
To: dedekind1@gmail.com
Cc: linux-mtd@lists.infradead.org; open list; Tanya Brokhman
Subject: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap

At first mount it's possible that there are not enough free PEBs since there
are PEB's pending to be erased. In such scenario, fm_pool (which is the pool
from which user required PEBs are allocated) will be empty.
Try fixing the above described situation by synchronously performing pending
erase work, thus produce another free PEB.

Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>

diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index
457ead3..9a36f78 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -595,10 +595,29 @@ static void refill_wl_pool(struct ubi_device *ubi)
static void refill_wl_user_pool(struct ubi_device *ubi)  {
 	struct ubi_fm_pool *pool = &ubi->fm_pool;
+	int err;
 
 	return_unused_pool_pebs(ubi, pool);
 
 	for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
+retry:
+		if (!ubi->free.rb_node ||
+		   (ubi->free_count - ubi->beb_rsvd_pebs < 1)) {
+			/*
+			 * There are no available PEBs. Try to free
+			 * PEB by means of synchronous execution of
+			 * pending works.
+			 */
+			if (ubi->works_count == 0)
+				break;
+			spin_unlock(&ubi->wl_lock);
+			err = do_work(ubi);
+			spin_lock(&ubi->wl_lock);
+			if (err < 0)
+				break;
+			goto retry;
+		}
+
 		pool->pebs[pool->size] = __wl_get_peb(ubi);
 		if (pool->pebs[pool->size] < 0)
 			break;
--
1.7.6
--
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
@ 2014-04-07  5:21   ` Dolev Raviv
  0 siblings, 0 replies; 15+ messages in thread
From: Dolev Raviv @ 2014-04-07  5:21 UTC (permalink / raw)
  To: 'Tanya Brokhman', dedekind1; +Cc: linux-mtd, 'open list'

Reviewed-by: Dolev Raviv <draviv@codeaurora.org>

Thanks,
Dolev
-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


-----Original Message-----
From: linux-mtd [mailto:linux-mtd-bounces@lists.infradead.org] On Behalf Of
Tanya Brokhman
Sent: Tuesday, April 01, 2014 11:02 AM
To: dedekind1@gmail.com
Cc: linux-mtd@lists.infradead.org; open list; Tanya Brokhman
Subject: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap

At first mount it's possible that there are not enough free PEBs since there
are PEB's pending to be erased. In such scenario, fm_pool (which is the pool
from which user required PEBs are allocated) will be empty.
Try fixing the above described situation by synchronously performing pending
erase work, thus produce another free PEB.

Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>

diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index
457ead3..9a36f78 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -595,10 +595,29 @@ static void refill_wl_pool(struct ubi_device *ubi)
static void refill_wl_user_pool(struct ubi_device *ubi)  {
 	struct ubi_fm_pool *pool = &ubi->fm_pool;
+	int err;
 
 	return_unused_pool_pebs(ubi, pool);
 
 	for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
+retry:
+		if (!ubi->free.rb_node ||
+		   (ubi->free_count - ubi->beb_rsvd_pebs < 1)) {
+			/*
+			 * There are no available PEBs. Try to free
+			 * PEB by means of synchronous execution of
+			 * pending works.
+			 */
+			if (ubi->works_count == 0)
+				break;
+			spin_unlock(&ubi->wl_lock);
+			err = do_work(ubi);
+			spin_lock(&ubi->wl_lock);
+			if (err < 0)
+				break;
+			goto retry;
+		}
+
 		pool->pebs[pool->size] = __wl_get_peb(ubi);
 		if (pool->pebs[pool->size] < 0)
 			break;
--
1.7.6
--
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
  2014-04-01  8:01 ` Tanya Brokhman
@ 2014-04-07 13:02   ` Richard Weinberger
  -1 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2014-04-07 13:02 UTC (permalink / raw)
  To: Tanya Brokhman; +Cc: Artem Bityutskiy, linux-mtd, open list

On Tue, Apr 1, 2014 at 10:01 AM, Tanya Brokhman <tlinder@codeaurora.org> wrote:
> At first mount it's possible that there are not enough free PEBs since
> there are PEB's pending to be erased. In such scenario, fm_pool (which is
> the pool from which user required PEBs are allocated) will be empty.
> Try fixing the above described situation by synchronously performing
> pending erase work, thus produce another free PEB.
>
> Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
>
> diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
> index 457ead3..9a36f78 100644
> --- a/drivers/mtd/ubi/wl.c
> +++ b/drivers/mtd/ubi/wl.c
> @@ -595,10 +595,29 @@ static void refill_wl_pool(struct ubi_device *ubi)
>  static void refill_wl_user_pool(struct ubi_device *ubi)
>  {
>         struct ubi_fm_pool *pool = &ubi->fm_pool;
> +       int err;
>
>         return_unused_pool_pebs(ubi, pool);
>
>         for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
> +retry:
> +               if (!ubi->free.rb_node ||
> +                  (ubi->free_count - ubi->beb_rsvd_pebs < 1)) {
> +                       /*
> +                        * There are no available PEBs. Try to free
> +                        * PEB by means of synchronous execution of
> +                        * pending works.
> +                        */
> +                       if (ubi->works_count == 0)
> +                               break;
> +                       spin_unlock(&ubi->wl_lock);
> +                       err = do_work(ubi);
> +                       spin_lock(&ubi->wl_lock);

This is basically what produce_free_peb() does.

> +                       if (err < 0)
> +                               break;
> +                       goto retry;
> +               }
> +
>                 pool->pebs[pool->size] = __wl_get_peb(ubi);

__wl_get_peb() already calls produce_free_peb() when we run out of free PEBs.

Does your patch really fix a problem you encounter or did you find the
issue by reviewing
the code?

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
@ 2014-04-07 13:02   ` Richard Weinberger
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2014-04-07 13:02 UTC (permalink / raw)
  To: Tanya Brokhman; +Cc: linux-mtd, open list, Artem Bityutskiy

On Tue, Apr 1, 2014 at 10:01 AM, Tanya Brokhman <tlinder@codeaurora.org> wrote:
> At first mount it's possible that there are not enough free PEBs since
> there are PEB's pending to be erased. In such scenario, fm_pool (which is
> the pool from which user required PEBs are allocated) will be empty.
> Try fixing the above described situation by synchronously performing
> pending erase work, thus produce another free PEB.
>
> Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
>
> diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
> index 457ead3..9a36f78 100644
> --- a/drivers/mtd/ubi/wl.c
> +++ b/drivers/mtd/ubi/wl.c
> @@ -595,10 +595,29 @@ static void refill_wl_pool(struct ubi_device *ubi)
>  static void refill_wl_user_pool(struct ubi_device *ubi)
>  {
>         struct ubi_fm_pool *pool = &ubi->fm_pool;
> +       int err;
>
>         return_unused_pool_pebs(ubi, pool);
>
>         for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
> +retry:
> +               if (!ubi->free.rb_node ||
> +                  (ubi->free_count - ubi->beb_rsvd_pebs < 1)) {
> +                       /*
> +                        * There are no available PEBs. Try to free
> +                        * PEB by means of synchronous execution of
> +                        * pending works.
> +                        */
> +                       if (ubi->works_count == 0)
> +                               break;
> +                       spin_unlock(&ubi->wl_lock);
> +                       err = do_work(ubi);
> +                       spin_lock(&ubi->wl_lock);

This is basically what produce_free_peb() does.

> +                       if (err < 0)
> +                               break;
> +                       goto retry;
> +               }
> +
>                 pool->pebs[pool->size] = __wl_get_peb(ubi);

__wl_get_peb() already calls produce_free_peb() when we run out of free PEBs.

Does your patch really fix a problem you encounter or did you find the
issue by reviewing
the code?

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
  2014-04-07 13:02   ` Richard Weinberger
@ 2014-04-07 16:05     ` Tanya Brokhman
  -1 siblings, 0 replies; 15+ messages in thread
From: Tanya Brokhman @ 2014-04-07 16:05 UTC (permalink / raw)
  To: Richard Weinberger; +Cc: Artem Bityutskiy, linux-mtd, open list

On 4/7/2014 4:02 PM, Richard Weinberger wrote:
> On Tue, Apr 1, 2014 at 10:01 AM, Tanya Brokhman <tlinder@codeaurora.org> wrote:
>> At first mount it's possible that there are not enough free PEBs since
>> there are PEB's pending to be erased. In such scenario, fm_pool (which is
>> the pool from which user required PEBs are allocated) will be empty.
>> Try fixing the above described situation by synchronously performing
>> pending erase work, thus produce another free PEB.
>>
>> Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
>>
>> diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
>> index 457ead3..9a36f78 100644
>> --- a/drivers/mtd/ubi/wl.c
>> +++ b/drivers/mtd/ubi/wl.c
>> @@ -595,10 +595,29 @@ static void refill_wl_pool(struct ubi_device *ubi)
>>   static void refill_wl_user_pool(struct ubi_device *ubi)
>>   {
>>          struct ubi_fm_pool *pool = &ubi->fm_pool;
>> +       int err;
>>
>>          return_unused_pool_pebs(ubi, pool);
>>
>>          for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
>> +retry:
>> +               if (!ubi->free.rb_node ||
>> +                  (ubi->free_count - ubi->beb_rsvd_pebs < 1)) {
>> +                       /*
>> +                        * There are no available PEBs. Try to free
>> +                        * PEB by means of synchronous execution of
>> +                        * pending works.
>> +                        */
>> +                       if (ubi->works_count == 0)
>> +                               break;
>> +                       spin_unlock(&ubi->wl_lock);
>> +                       err = do_work(ubi);
>> +                       spin_lock(&ubi->wl_lock);
>
> This is basically what produce_free_peb() does.

Right. I didn't use t just because of the termination condition. 
produce_free_peb stops if there is 1 free peb. I need more then 1

>
>> +                       if (err < 0)
>> +                               break;
>> +                       goto retry;
>> +               }
>> +
>>                  pool->pebs[pool->size] = __wl_get_peb(ubi);
>
> __wl_get_peb() already calls produce_free_peb() when we run out of free PEBs.
>
> Does your patch really fix a problem you encounter or did you find the
> issue by reviewing
> the code?
>

Yes. We encountered this issue, as described in the commit message. This 
is the fix. Verified and working for us.

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
@ 2014-04-07 16:05     ` Tanya Brokhman
  0 siblings, 0 replies; 15+ messages in thread
From: Tanya Brokhman @ 2014-04-07 16:05 UTC (permalink / raw)
  To: Richard Weinberger; +Cc: linux-mtd, open list, Artem Bityutskiy

On 4/7/2014 4:02 PM, Richard Weinberger wrote:
> On Tue, Apr 1, 2014 at 10:01 AM, Tanya Brokhman <tlinder@codeaurora.org> wrote:
>> At first mount it's possible that there are not enough free PEBs since
>> there are PEB's pending to be erased. In such scenario, fm_pool (which is
>> the pool from which user required PEBs are allocated) will be empty.
>> Try fixing the above described situation by synchronously performing
>> pending erase work, thus produce another free PEB.
>>
>> Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
>>
>> diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
>> index 457ead3..9a36f78 100644
>> --- a/drivers/mtd/ubi/wl.c
>> +++ b/drivers/mtd/ubi/wl.c
>> @@ -595,10 +595,29 @@ static void refill_wl_pool(struct ubi_device *ubi)
>>   static void refill_wl_user_pool(struct ubi_device *ubi)
>>   {
>>          struct ubi_fm_pool *pool = &ubi->fm_pool;
>> +       int err;
>>
>>          return_unused_pool_pebs(ubi, pool);
>>
>>          for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
>> +retry:
>> +               if (!ubi->free.rb_node ||
>> +                  (ubi->free_count - ubi->beb_rsvd_pebs < 1)) {
>> +                       /*
>> +                        * There are no available PEBs. Try to free
>> +                        * PEB by means of synchronous execution of
>> +                        * pending works.
>> +                        */
>> +                       if (ubi->works_count == 0)
>> +                               break;
>> +                       spin_unlock(&ubi->wl_lock);
>> +                       err = do_work(ubi);
>> +                       spin_lock(&ubi->wl_lock);
>
> This is basically what produce_free_peb() does.

Right. I didn't use t just because of the termination condition. 
produce_free_peb stops if there is 1 free peb. I need more then 1

>
>> +                       if (err < 0)
>> +                               break;
>> +                       goto retry;
>> +               }
>> +
>>                  pool->pebs[pool->size] = __wl_get_peb(ubi);
>
> __wl_get_peb() already calls produce_free_peb() when we run out of free PEBs.
>
> Does your patch really fix a problem you encounter or did you find the
> issue by reviewing
> the code?
>

Yes. We encountered this issue, as described in the commit message. This 
is the fix. Verified and working for us.

-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
  2014-04-07 16:05     ` Tanya Brokhman
@ 2014-04-07 16:42       ` Richard Weinberger
  -1 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2014-04-07 16:42 UTC (permalink / raw)
  To: Tanya Brokhman; +Cc: Artem Bityutskiy, linux-mtd, open list

Am 07.04.2014 18:05, schrieb Tanya Brokhman:
> On 4/7/2014 4:02 PM, Richard Weinberger wrote:
>> On Tue, Apr 1, 2014 at 10:01 AM, Tanya Brokhman <tlinder@codeaurora.org> wrote:
>>> At first mount it's possible that there are not enough free PEBs since
>>> there are PEB's pending to be erased. In such scenario, fm_pool (which is
>>> the pool from which user required PEBs are allocated) will be empty.
>>> Try fixing the above described situation by synchronously performing
>>> pending erase work, thus produce another free PEB.
>>>
>>> Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
>>>
>>> diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
>>> index 457ead3..9a36f78 100644
>>> --- a/drivers/mtd/ubi/wl.c
>>> +++ b/drivers/mtd/ubi/wl.c
>>> @@ -595,10 +595,29 @@ static void refill_wl_pool(struct ubi_device *ubi)
>>>   static void refill_wl_user_pool(struct ubi_device *ubi)
>>>   {
>>>          struct ubi_fm_pool *pool = &ubi->fm_pool;
>>> +       int err;
>>>
>>>          return_unused_pool_pebs(ubi, pool);
>>>
>>>          for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
>>> +retry:
>>> +               if (!ubi->free.rb_node ||
>>> +                  (ubi->free_count - ubi->beb_rsvd_pebs < 1)) {
>>> +                       /*
>>> +                        * There are no available PEBs. Try to free
>>> +                        * PEB by means of synchronous execution of
>>> +                        * pending works.
>>> +                        */
>>> +                       if (ubi->works_count == 0)
>>> +                               break;
>>> +                       spin_unlock(&ubi->wl_lock);
>>> +                       err = do_work(ubi);
>>> +                       spin_lock(&ubi->wl_lock);
>>
>> This is basically what produce_free_peb() does.
> 
> Right. I didn't use t just because of the termination condition. produce_free_peb stops if there is 1 free peb. I need more then 1
> 
>>
>>> +                       if (err < 0)
>>> +                               break;
>>> +                       goto retry;
>>> +               }
>>> +
>>>                  pool->pebs[pool->size] = __wl_get_peb(ubi);
>>
>> __wl_get_peb() already calls produce_free_peb() when we run out of free PEBs.
>>
>> Does your patch really fix a problem you encounter or did you find the
>> issue by reviewing
>> the code?
>>
> 
> Yes. We encountered this issue, as described in the commit message. This is the fix. Verified and working for us.

Wouldn't it be better to fix produce_free_pep() instead of duplicating it?
I.e. Such that you can tell it how many PEBs you need.

Thanks,
//richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
@ 2014-04-07 16:42       ` Richard Weinberger
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2014-04-07 16:42 UTC (permalink / raw)
  To: Tanya Brokhman; +Cc: linux-mtd, open list, Artem Bityutskiy

Am 07.04.2014 18:05, schrieb Tanya Brokhman:
> On 4/7/2014 4:02 PM, Richard Weinberger wrote:
>> On Tue, Apr 1, 2014 at 10:01 AM, Tanya Brokhman <tlinder@codeaurora.org> wrote:
>>> At first mount it's possible that there are not enough free PEBs since
>>> there are PEB's pending to be erased. In such scenario, fm_pool (which is
>>> the pool from which user required PEBs are allocated) will be empty.
>>> Try fixing the above described situation by synchronously performing
>>> pending erase work, thus produce another free PEB.
>>>
>>> Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
>>>
>>> diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
>>> index 457ead3..9a36f78 100644
>>> --- a/drivers/mtd/ubi/wl.c
>>> +++ b/drivers/mtd/ubi/wl.c
>>> @@ -595,10 +595,29 @@ static void refill_wl_pool(struct ubi_device *ubi)
>>>   static void refill_wl_user_pool(struct ubi_device *ubi)
>>>   {
>>>          struct ubi_fm_pool *pool = &ubi->fm_pool;
>>> +       int err;
>>>
>>>          return_unused_pool_pebs(ubi, pool);
>>>
>>>          for (pool->size = 0; pool->size < pool->max_size; pool->size++) {
>>> +retry:
>>> +               if (!ubi->free.rb_node ||
>>> +                  (ubi->free_count - ubi->beb_rsvd_pebs < 1)) {
>>> +                       /*
>>> +                        * There are no available PEBs. Try to free
>>> +                        * PEB by means of synchronous execution of
>>> +                        * pending works.
>>> +                        */
>>> +                       if (ubi->works_count == 0)
>>> +                               break;
>>> +                       spin_unlock(&ubi->wl_lock);
>>> +                       err = do_work(ubi);
>>> +                       spin_lock(&ubi->wl_lock);
>>
>> This is basically what produce_free_peb() does.
> 
> Right. I didn't use t just because of the termination condition. produce_free_peb stops if there is 1 free peb. I need more then 1
> 
>>
>>> +                       if (err < 0)
>>> +                               break;
>>> +                       goto retry;
>>> +               }
>>> +
>>>                  pool->pebs[pool->size] = __wl_get_peb(ubi);
>>
>> __wl_get_peb() already calls produce_free_peb() when we run out of free PEBs.
>>
>> Does your patch really fix a problem you encounter or did you find the
>> issue by reviewing
>> the code?
>>
> 
> Yes. We encountered this issue, as described in the commit message. This is the fix. Verified and working for us.

Wouldn't it be better to fix produce_free_pep() instead of duplicating it?
I.e. Such that you can tell it how many PEBs you need.

Thanks,
//richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
  2014-04-01  8:01 ` Tanya Brokhman
                   ` (2 preceding siblings ...)
  (?)
@ 2014-04-08 13:42 ` Artem Bityutskiy
  2014-04-08 13:44     ` Bityutskiy, Artem
  -1 siblings, 1 reply; 15+ messages in thread
From: Artem Bityutskiy @ 2014-04-08 13:42 UTC (permalink / raw)
  To: Tanya Brokhman; +Cc: linux-mtd, open list

On Tue, 2014-04-01 at 11:01 +0300, Tanya Brokhman wrote:
> At first mount it's possible that there are not enough free PEBs since
> there are PEB's pending to be erased. In such scenario, fm_pool (which is
> the pool from which user required PEBs are allocated) will be empty.
> Try fixing the above described situation by synchronously performing
> pending erase work, thus produce another free PEB.
> 
> Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>

Pushed to linux-ubifs.git / master, thanks!

-- 
Best Regards,
Artem Bityutskiy


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
  2014-04-08 13:42 ` Artem Bityutskiy
@ 2014-04-08 13:44     ` Bityutskiy, Artem
  0 siblings, 0 replies; 15+ messages in thread
From: Bityutskiy, Artem @ 2014-04-08 13:44 UTC (permalink / raw)
  To: tlinder; +Cc: linux-kernel, linux-mtd

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1381 bytes --]

On Tue, 2014-04-08 at 16:42 +0300, Artem Bityutskiy wrote:
> On Tue, 2014-04-01 at 11:01 +0300, Tanya Brokhman wrote:
> > At first mount it's possible that there are not enough free PEBs since
> > there are PEB's pending to be erased. In such scenario, fm_pool (which is
> > the pool from which user required PEBs are allocated) will be empty.
> > Try fixing the above described situation by synchronously performing
> > pending erase work, thus produce another free PEB.
> > 
> > Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
> 
> Pushed to linux-ubifs.git / master, thanks!

Oh, sorry, this one I actually _dropped_. Would you please rather
re-structure the code to avoid duplication. E.g., do what Richard
suggested.

-- 
Best Regards,
Artem Bityutskiy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
@ 2014-04-08 13:44     ` Bityutskiy, Artem
  0 siblings, 0 replies; 15+ messages in thread
From: Bityutskiy, Artem @ 2014-04-08 13:44 UTC (permalink / raw)
  To: tlinder; +Cc: linux-mtd, linux-kernel

On Tue, 2014-04-08 at 16:42 +0300, Artem Bityutskiy wrote:
> On Tue, 2014-04-01 at 11:01 +0300, Tanya Brokhman wrote:
> > At first mount it's possible that there are not enough free PEBs since
> > there are PEB's pending to be erased. In such scenario, fm_pool (which is
> > the pool from which user required PEBs are allocated) will be empty.
> > Try fixing the above described situation by synchronously performing
> > pending erase work, thus produce another free PEB.
> > 
> > Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
> 
> Pushed to linux-ubifs.git / master, thanks!

Oh, sorry, this one I actually _dropped_. Would you please rather
re-structure the code to avoid duplication. E.g., do what Richard
suggested.

-- 
Best Regards,
Artem Bityutskiy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
  2014-04-08 13:44     ` Bityutskiy, Artem
@ 2014-04-08 22:17       ` Richard Weinberger
  -1 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2014-04-08 22:17 UTC (permalink / raw)
  To: Bityutskiy, Artem; +Cc: tlinder, linux-kernel, linux-mtd

On Tue, Apr 8, 2014 at 3:44 PM, Bityutskiy, Artem
<artem.bityutskiy@intel.com> wrote:
> On Tue, 2014-04-08 at 16:42 +0300, Artem Bityutskiy wrote:
>> On Tue, 2014-04-01 at 11:01 +0300, Tanya Brokhman wrote:
>> > At first mount it's possible that there are not enough free PEBs since
>> > there are PEB's pending to be erased. In such scenario, fm_pool (which is
>> > the pool from which user required PEBs are allocated) will be empty.
>> > Try fixing the above described situation by synchronously performing
>> > pending erase work, thus produce another free PEB.
>> >
>> > Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
>>
>> Pushed to linux-ubifs.git / master, thanks!
>
> Oh, sorry, this one I actually _dropped_. Would you please rather
> re-structure the code to avoid duplication. E.g., do what Richard
> suggested.

Tatyana, can you also please find out how many PEBs you need?
Strictly speaking we need only one (which should be produced by __wl_get_peb().
I want to make sure that we're not just papering over an issue. :-)

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap
@ 2014-04-08 22:17       ` Richard Weinberger
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2014-04-08 22:17 UTC (permalink / raw)
  To: Bityutskiy, Artem; +Cc: linux-mtd, linux-kernel, tlinder

On Tue, Apr 8, 2014 at 3:44 PM, Bityutskiy, Artem
<artem.bityutskiy@intel.com> wrote:
> On Tue, 2014-04-08 at 16:42 +0300, Artem Bityutskiy wrote:
>> On Tue, 2014-04-01 at 11:01 +0300, Tanya Brokhman wrote:
>> > At first mount it's possible that there are not enough free PEBs since
>> > there are PEB's pending to be erased. In such scenario, fm_pool (which is
>> > the pool from which user required PEBs are allocated) will be empty.
>> > Try fixing the above described situation by synchronously performing
>> > pending erase work, thus produce another free PEB.
>> >
>> > Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
>>
>> Pushed to linux-ubifs.git / master, thanks!
>
> Oh, sorry, this one I actually _dropped_. Would you please rather
> re-structure the code to avoid duplication. E.g., do what Richard
> suggested.

Tatyana, can you also please find out how many PEBs you need?
Strictly speaking we need only one (which should be produced by __wl_get_peb().
I want to make sure that we're not just papering over an issue. :-)

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-04-08 22:18 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-01  8:01 [RFC/PATCH] mtd: ubi: Free peb's synchronously for fastmap Tanya Brokhman
2014-04-01  8:01 ` Tanya Brokhman
2014-04-07  5:21 ` Dolev Raviv
2014-04-07  5:21   ` Dolev Raviv
2014-04-07 13:02 ` Richard Weinberger
2014-04-07 13:02   ` Richard Weinberger
2014-04-07 16:05   ` Tanya Brokhman
2014-04-07 16:05     ` Tanya Brokhman
2014-04-07 16:42     ` Richard Weinberger
2014-04-07 16:42       ` Richard Weinberger
2014-04-08 13:42 ` Artem Bityutskiy
2014-04-08 13:44   ` Bityutskiy, Artem
2014-04-08 13:44     ` Bityutskiy, Artem
2014-04-08 22:17     ` Richard Weinberger
2014-04-08 22:17       ` Richard Weinberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.