Re: [PATCH] staging: tidspbridge: protect dmm_map properly

[Felipe Contreras]

Hi,

On Mon, Dec 20, 2010 at 8:30 PM, Kanigeri, Hari <h-kanigeri2@xxxxxx> wrote:
> On Mon, Dec 20, 2010 at 11:12 AM, Felipe Contreras
> <felipe.contreras@xxxxxxxxx> wrote:
>> We need to protect not only the dmm_map list, but the individual
>> map_obj's, otherwise, we might be building the scatter-gather list with
>> garbage. So, use the existing proc_lock for that.
>>
>> I observed race conditions which caused kernel panics while running
>> stress tests. This patch fixes those.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
>> ---
>> Âdrivers/staging/tidspbridge/rmgr/proc.c | Â 18 ++++++++++++++----
>> Â1 files changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/tidspbridge/rmgr/proc.c b/drivers/staging/tidspbridge/rmgr/proc.c
>> index b47d7aa..21052e3 100644
>> --- a/drivers/staging/tidspbridge/rmgr/proc.c
>> +++ b/drivers/staging/tidspbridge/rmgr/proc.c
>> @@ -781,12 +781,14 @@ int proc_begin_dma(void *hprocessor, void *pmpu_addr, u32 ul_size,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â(u32)pmpu_addr,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âul_size, dir);
>>
>> + Â Â Â mutex_lock(&proc_lock);
>
> May be you should use mutex_lock_interruptable instead of Âmutex_lock.

Right, but I think that should be a separate patch since
mutex_lock(&proc_lock) is already being used.

>> @@ -819,12 +823,14 @@ int proc_end_dma(void *hprocessor, void *pmpu_addr, u32 ul_size,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â(u32)pmpu_addr,
>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âul_size, dir);
>>
>> + Â Â Â mutex_lock(&proc_lock);
>> +
>> Â Â Â Â/* find requested memory are in cached mapping information */
>> Â Â Â Âmap_obj = find_containing_mapping(pr_ctxt, (u32) pmpu_addr, ul_size);
>> Â Â Â Âif (!map_obj) {
>> Â Â Â Â Â Â Â Âpr_err("%s: find_containing_mapping failed\n", __func__);
>> Â Â Â Â Â Â Â Âstatus = -EFAULT;
>> - Â Â Â Â Â Â Â goto err_out;
>> + Â Â Â Â Â Â Â goto no_map;
>> Â Â Â Â}
>>
>> Â Â Â Âif (memory_regain_ownership(map_obj, (u32) pmpu_addr, ul_size, dir)) {
>> @@ -834,6 +840,8 @@ int proc_end_dma(void *hprocessor, void *pmpu_addr, u32 ul_size,
>> Â Â Â Â Â Â Â Âgoto err_out;
>
> Mutex is not released in this case as it is released only at no_map.

Oops! I didn't test proc_end_dma() and quickly added those locks after
I noticed it. I'll resend with the fix.

-- 
Felipe Contreras
--
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/