Am 06.01.17 um 18:30 schrieb Valdis.Kletnieks@vt.edu:

On Fri, 06 Jan 2017 18:12:41 +0100, Johannes Thoma said:

...

Am 06.01.17 um 17:54 schrieb Valdis.Kletnieks@vt.edu:

...

On Fri, 06 Jan 2017 17:16:11 +0100, johannes@johannesthoma.com said:

...

...

...

+ if (start != 0 && retries--) {

Do we have any guarantee, or even good reason to believe, that this will eventually make forward progress, or can the goto hang everything? I'm not thrilled by a potentially unbounded loop inside a mutex_lock(), especially when you holding the mutex may be preventing something else from changing things so forward progress can be made....

Good point. That is what the retries variable is good for. To make this more obvious, I should write:

No, you missed my point.

We're inside a mutex_lock() region. We hit the goto, and drive bitmap_find_next_zero_are_off() again. How do we know that we're not just going to spin our wheels 10 times and fall out? What will change the bitmap_find_next_zero_area_offset() result?

Correct me if I am wrong but setting start from a high value where no more memory is free back to zero should change the result. The scenario I had is: start is 0 -> bitmap_find_next_zero_off succeeds alloc_contig_range fails with -EBUSY start is incremented by the size of the requested block start is near the end of the area, hence bitmap_find_next_zero_off fails (new) now we set start again back to 0, so bitmap_find_next_zero_off succeeds again (it might also fail if someone else just allocated memory in which case we have to (and do) fail). alloc_contig_range succeeds.

Remember - there's a mutex_lock() around that call for a *reason*. And I suspect the reason is specifically so nobody else *can* monkey with the cma area.

How is this better than just dropping the mutex, doing an mdelay(), and then re-taking it?

That would be probably the best way to do it. I suspect you mean something like: if (start != 0) { retries--; if (retries > 0) { mutex_unlock(&cma->lock); mdelay(1); start = 0; mutex_lock(&cma->lock); goto scan_again; } } Hmm..it seems that that would require more work. Thank you for your input, - Johannes