Re: [PATCH 4/5] reservation: cross-device reservation support

[Maarten Lankhorst]

Op 28-09-12 17:29, Thomas HellstrÃm schreef:
> On 9/28/12 2:43 PM, Maarten Lankhorst wrote:
>> This adds support for a generic reservations framework that can be
>> hooked up to ttm and dma-buf and allows easy sharing of reservations
>> across devices.
>>
>> The idea is that a dma-buf and ttm object both will get a pointer
>> to a struct reservation_object, which has to be reserved before
>> anything is done with the buffer.
> "Anything is done with the buffer" should probably be rephrased, as different members of the buffer struct
> may be protected by different locks. It may not be practical or even possible to
> protect all buffer members with reservation.
Agreed.
>> Some followup patches are needed in ttm so the lru_lock is no longer
>> taken during the reservation step. This makes the lockdep annotation
>> patch a lot more useful, and the assumption that the lru lock protects
>> atomic removal off the lru list will fail soon, anyway.
> As previously discussed, I'm unfortunately not prepared to accept removal of the reserve-lru atomicity
>  into the TTM code at this point.
> The current code is based on this assumption and removing it will end up with
> efficiencies, breaking the delayed delete code and probably a locking nightmare when trying to write
> new TTM code.
The lru lock removal patch fixed the delayed delete code, it really is not different from the current
situation. In fact it is more clear without the guarantee what various parts are trying to protect.

Nothing prevents you from holding the lru_lock while trylocking,
leaving that guarantee intact for that part. Can you really just review
the patch and tell me where it breaks and/or makes the code unreadable?

See my preemptive reply to patch 1/5 for details. I would prefer you
followup there. :-)

~Maarten

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/