Received-SPF: pass (google.com: domain of linux-kernel+bounces-112473-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <c8efae98-3cf8-c21c-bfa4-d5998ab92a0e@amd.com>
Date: Sat, 23 Mar 2024 21:21:32 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH] drm/amdgpu: fix deadlock while reading mqd from debugfs
Content-Language: en-US
To: Johannes Weiner <hannes@cmpxchg.org>,
 =?UTF-8?Q?Christian_K=c3=b6nig?= <ckoenig.leichtzumerken@gmail.com>
Cc: Alex Deucher <alexander.deucher@amd.com>,
 =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
 amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
 linux-kernel@vger.kernel.org
References: <20240307221609.7651-1-hannes@cmpxchg.org>
 <c411dce6-faaf-46c3-8bb6-8c4db871e598@gmail.com>
 <20240314170948.GA581298@cmpxchg.org> <20240323145247.GC448621@cmpxchg.org>
From: "Sharma, Shashank" <shashank.sharma@amd.com>
In-Reply-To: <20240323145247.GC448621@cmpxchg.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?RFA0M0xsTCs2WUdSMk1qT1BsZXprMTVua1ZQdXI0OVZ1VmhlYlN3RGRvbkFM?=
 =?utf-8?B?SVBTQk5ncGExMU1qbmduY1ZZMkMxNU82OWw5V2VxQmpMaGRXN1N6dHBsazFZ?=
 =?utf-8?B?Y0JIM296Z3Y1M3BkS3R2cWU4R2xKVW5SRkFnTVJsaDlCZ1R4Zkh1Nkt0bjlw?=
 =?utf-8?B?cHVYbTV0bXJvc0xaaEtNMmpDQ1pLTHcyeDFEVkpjQmlMMHFicFhLTUpZQmJC?=
 =?utf-8?B?L3E3c2RYRDNDYlQ1R1BUSWo2ZlBWRDJSVitFRzZXTzVUTWNsSFZZSFZJcVpX?=
 =?utf-8?B?SjNZQmlSdU5FRW5SY1R4Rk9QTnEzdDZJQnRKeDlOS2orMlhsNHI3MjNkbWFR?=
 =?utf-8?B?eTJwLzR0aGZLTzRmVExuTG16VnErcm1XT09CM0RYODNtVk1zekU5UlJrNW9a?=
 =?utf-8?B?Rys4dnI4Q3JuQjVybml3WkgzdGdCYzM1UUI3TFBJczZTeklQOTZnK2w4aVFv?=
 =?utf-8?B?Y25JeS9QU3ZCOU13eDlVTjhHZWVtRTFWMDZ3eDNBQzgxSEJMc3BaVURFV2x1?=
 =?utf-8?B?MlA0MC9lM0dIQnh0VGYxa0laTmdLblJEZncyZURxcExQaFhRenk1NnJLbzlu?=
 =?utf-8?B?VXhzNlBnTEpKYXo1ekEzZEdQQzNRcE1ZMllERG9hT2FGZjZGZ2M1dVQvemk4?=
 =?utf-8?B?bFFrRHBhK2NhVHJ4V2tIcDdYNlNBV1ZpeXZWeFRlNGlNcjUrOXVJR1publR6?=
 =?utf-8?B?NEoyaUJMc2ZOYkxqVmNPclJKc3F1c011YUpBSlNiamM3OHpvak02dUJMQ1E0?=
 =?utf-8?B?dG80SHJ2eXFTR0ZjMXNPeEhlbWdXVHZjR3RDS1YyT0dmTUJmR3pYYks4Ykpu?=
 =?utf-8?B?dE5yWmdicHhLcVMvMzAwRDYzck5kMDNuSDdMTVJhaEwyeDdZc0tzZXlQWStN?=
 =?utf-8?B?STZhUkF3b3VyYnI0amJyMWdBRnNtYkRCNFlFN2VEOC8yOVpKQm55RFZDMXky?=
 =?utf-8?B?TkdoQVNpYnRyYTk2WW1YcWJKRVhCRGF4UUNYcnZkN2VIbkhxdE1CbHNCZTZL?=
 =?utf-8?B?TVo0SjUzT3BpRUFMaEdUVDBKa2pNQ2lxMmYwVm1sWUFSaXRtNisxV1Z5aGdk?=
 =?utf-8?B?OWVwWkJGNzE1dVVXdXdXdDFNZ1JLMjBjTWhkY3hiN2pob3cvRDk3SkdCZDlH?=
 =?utf-8?B?SkdYRFdpdFRLb05keWdCNEpqNndGUTcvZUVLVnI5TTNZY2RDMU83M0hEb2Zh?=
 =?utf-8?B?SGdRQm82b09RMEtjdUZWMkNZeSt4MzI2S1k3S2RHeFd4dC9iM2YvbFlGd05N?=
 =?utf-8?B?TENwUU15YWhRNXdYTHh0bGljV0VDb0ZGTWx0S2F2TEU1QmNJc3FlUGVaMkFx?=
 =?utf-8?B?dmFTYnovdDltYzhuNjRPYlNyR2hwTG45aFk3aGROcGs1bnJtcnVCZ3FaSkZ3?=
 =?utf-8?B?S2V1RUxrbStRVnZDN0s4VWVqdWh4THZ6SFlxRU1WbUNZbG1XTE9BTnQybkRs?=
 =?utf-8?B?dUYySnJMZ2hadHdxQWg3ZTRwSFJpN21TZ2lJN0dUSDdwMTJVQWx4NjhUU0R1?=
 =?utf-8?B?cENlY242RldSY09hWXM4ZGNTM0s5emtTcENPMWdJSWZISjhRTUxwQnlqQUl6?=
 =?utf-8?B?VkFGLzMvWTZTYzlHK2lRU2g4UjBMeFZnRFFlOFl1N29jRjU3OHhvNUlFd01z?=
 =?utf-8?B?aGtESDFIZlhISVEvK0pMcW44K1phY2MwK2xabmhzdm9lS0dYOWhMRVo3TmRS?=
 =?utf-8?B?S25oUFl4bXVCTGJ3VmZxS1RTVWRCMDNycVhKVnBKZkw2UTRzT29QK1QzdWkz?=
 =?utf-8?B?cXNrK1EvUFFtM2kzcmVKeGVIdXpzUTE5R2xtcXhEZE1kYnpSeHZwTzZhKzlH?=
 =?utf-8?B?ZU0yMjNZSGR4emtxbG5GVmYvekgzUTRLTXJJdFZtRjZZZnJYTENDd2ROSDlV?=
 =?utf-8?B?SWU2eVhNeHVDaGNucXZXTEJBR3hoUUJnTVBsL1lBbTlRdGFZR0pmdjZVd3Fl?=
 =?utf-8?B?bTd0aXJTS0JtR2tyUm1wZDkwRWNxYzAvN09aZ2lMVm44V2krSjUzR2NYbWpy?=
 =?utf-8?B?NjIrQ2pXbnptMDRrSS9qdlI4M3h2ZGhJQlFxa3ZvQlBWQUdENHlYUllKdDZx?=
 =?utf-8?B?dmdiWTJyMEJkSXJuSWtmcGRtQTRYRWRueitLSGFTSlROSm9GeWI1ZVc3WmRR?=
 =?utf-8?Q?+FQxyIWfahlxobg7J6O4uJzGd?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b23381b1-6d14-4025-8259-08dc4b76d74d
X-MS-Exchange-CrossTenant-AuthSource: MW4PR12MB5667.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Mar 2024 20:21:37.2873
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: HeW2gZyuDOMkDdlF9RDOWZGtqDzUK4NDrbE6lKRezTxNazY2Ttrb1wZ4fkeQR3B5UehFXQyseem/GSB0qODLVA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR12MB7604


On 23/03/2024 15:52, Johannes Weiner wrote:
> On Thu, Mar 14, 2024 at 01:09:57PM -0400, Johannes Weiner wrote:
>> Hello,
>>
>> On Fri, Mar 08, 2024 at 12:32:33PM +0100, Christian König wrote:
>>> Am 07.03.24 um 23:07 schrieb Johannes Weiner:
>>>> Lastly I went with an open loop instead of a memcpy() as I wasn't
>>>> sure if that memory is safe to address a byte at at time.
>> Shashank pointed out to me in private that byte access would indeed be
>> safe. However, after actually trying it it won't work because memcpy()
>> doesn't play nice with mqd being volatile:
>>
>> /home/hannes/src/linux/linux/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c: In function 'amdgpu_debugfs_mqd_read':
>> /home/hannes/src/linux/linux/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c:550:22: warning: passing argument 1 of '__builtin_dynamic_object_size' discards 'volatil' qualifier from pointer target type [-Wdiscarded-qualifiers]
>>    550 |         memcpy(kbuf, mqd, ring->mqd_size);
>>
>> So I would propose leaving the patch as-is. Shashank, does that sound
>> good to you?
> Friendly ping :)
>
> Shashank, is your Reviewed-by still good for this patch, given the
> above?

Ah, sorry I missed this due to some parallel work, and just realized the 
memcpy/volatile limitation.

I also feel the need of protecting MQD read under a lock to avoid 
parallel change in MQD while we do byte-by-byte copy, but I will add 
that in my to-do list.

Please feel free to use my R-b.

- Shashank

> Thanks