Re: [usb-storage] Re: cma: deadlock using usb-storage and fs

[Mike Kravetz <mike.kravetz@oracle.com>]

On 1/7/19 10:13 AM, Gaël PORTAY wrote:
> On Tue, Dec 18, 2018 at 01:14:42PM -0800, Laura Abbott wrote:
>> On 12/18/18 11:42 AM, Mike Kravetz wrote:
>>> On 12/17/18 1:57 PM, Laura Abbott wrote:
>>> I am wondering if we still need to hold the cma_mutex while calling
>>> alloc_contig_range().  Looking back at the history, it appears that
>>> the reason for holding the mutex was to prevent two threads from operating
>>> on the same pageblock.
>>>
>>> Commit 2c7452a075d4 ("mm/page_isolation.c: make start_isolate_page_range()
>>> fail if already isolated") will cause alloc_contig_range to return EBUSY
>>> if two callers are attempting to operate on the same pageblock.  This was
>>> added because memory hotplug as well as gigantac page allocation call
>>> alloc_contig_range and could conflict with each other or cma.   cma_alloc
>>> has logic to retry if EBUSY is returned.  Although, IIUC it assumes the
>>> EBUSY is the result of specific pages being busy as opposed to someone
>>> else operating on the pageblock.  Therefore, the retry logic to 'try a
>>> different set of pages' is not what one  would/should attempt in the case
>>> someone else is operating on the pageblock.
>>>
>>> Would it be possible or make sense to remove the mutex and retry when
>>> EBUSY?  Or, am I missing some other reason for holding the mutex.
>>>
>>
>> I had forgotten that start_isolate_page_range had been updated to
>> return -EBUSY. It looks like we would need to update
>> the callback for migrate_pages in __alloc_contig_migrate_range
>> since alloc_migrate_target by default will use __GFP_IO.
>> So I _think_ if we update that to honor GFP_NOIO we could
>> remove the mutex assuming the rest of migrate_pages honors
>> it properly.
>>
> 
> I have also removed the mutex (start_isolate_page_range retunrs -EBUSY),
> and it worked (in my case).
> 
> But I did not do the proper magic because I am not sure of what should
> be done and how: -EBUSY is not handled and __GFP_NOIO is not honored. 

If we remove the mutex, I am pretty sure we would want to distinguish
between the (at least) two types of _EBUSY that can be returned by
alloc_contig_range().  Seems that the retry logic should be different if
a page block is busy as opposed to pages within the range.

I'm busy with other things, but could get to this later this week or early
next week unless someone else has the time.
-- 
Mike Kravetz