Am 06.01.17 um 18:30 schrieb Valdis.Kletnieks@xxxxxx:
> On Fri, 06 Jan 2017 18:12:41 +0100, Johannes Thoma said:
>
>> Am 06.01.17 um 17:54 schrieb Valdis.Kletnieks@xxxxxx:
>>> On Fri, 06 Jan 2017 17:16:11 +0100, johannes@xxxxxxxxxxxxxxxxx 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
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@xxxxxxxxxxxxxxxxx
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
