Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <7f895a99-adfa-bcbd-c130-a924c668b8af@amd.com>
Date:   Wed, 18 May 2022 14:03:28 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [PATCH v2] dma-buf: Move sysfs work out of DMA-BUF export path
Content-Language: en-US
To:     "T.J. Mercier" <tjmercier@google.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Suren Baghdasaryan <surenb@google.com>,
        Kalesh Singh <kaleshsingh@google.com>,
        Minchan Kim <minchan@google.com>,
        Greg Kroah-Hartman <gregkh@google.com>,
        John Stultz <jstultz@google.com>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        Daniel Vetter <daniel.vetter@ffwll.ch>,
        Hridya Valsaraju <hridya@google.com>, kernel-team@android.com,
        linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org
References: <20220516171315.2400578-1-tjmercier@google.com>
 <175c5af3-9224-9c8e-0784-349dad9a2954@amd.com>
 <CABdmKX2GcgCs1xANYPBp8OEtk9qqH7AvCzpdppj9rHXvMqWSAw@mail.gmail.com>
 <0875fa95-3a25-a354-1433-201fca81ed3e@amd.com>
 <CABdmKX1+VYfdzyVYOS5MCsr4ptGTygmuUP9ikyh-vW6DgKk2kg@mail.gmail.com>
 <YoM9BAwybcjG7K/H@kroah.com> <d820893c-fa2e-3bac-88be-f39c06d89c01@amd.com>
 <CABdmKX2m1b1kdACKM19S+u9uR5RTy1UGMRgd+3QA_oAyCpeggg@mail.gmail.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <CABdmKX2m1b1kdACKM19S+u9uR5RTy1UGMRgd+3QA_oAyCpeggg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dFR2MU11Rks1dzUvSHJ0MEZOczh2NnVlN2k1dFRLSS84a2FWYjFPbGw5Qjls?=
 =?utf-8?B?WFJHaDcxVy9RTWRFYzM2ZW41N2ZaTCt2anRoazhoTTNXQzVFYVFyV0tSRU8y?=
 =?utf-8?B?Qjhoa1J2cnUzMWNvd3hqMVdZblZnNXdodSswei9sM0M4K004TzdyeHM0aUVu?=
 =?utf-8?B?YjhYMjQ0S0VmcEJKRFdJL0lOUjRKaDAxSCt0YStnNGlvUkVPOGtrQ1hTd3lO?=
 =?utf-8?B?REdncDVaS0gwS0hyMzVNT1B0SC92eVJWVGlMTzJ3STVZMXd0cWVlL0dleU9w?=
 =?utf-8?B?VzhmWmZSeEcxYUxJM3VrWXBhS01BVW14Njk2VFNPSFdXRnlyN2V1aGI1dTNm?=
 =?utf-8?B?R0dlMDlESVg5czk3dlVCNkdLZXcxdzQzUVNDVWsvcXNPL2VZWUI2OFZDb2FY?=
 =?utf-8?B?VUZ0Z0dFYWFSUEZWbDduRWg0KytxRGtGL0VMY3VFZjZEYXBISStQQmJRSWow?=
 =?utf-8?B?aTlQemFoQkNEcHk5eUVqU3NvdWdlYXFpV3Noako0Ui8vQW5WM3M1MWdXUzZ3?=
 =?utf-8?B?TkI5cEVVVm92RGU5ZER4ZHdCQjJRVk5jakQ2dkJoUFRKU3pYb3h2MHMydFNT?=
 =?utf-8?B?WnhlTHpwMDlpaUZhLzYvbG9zWlY0TFdoK3llZm52N3JHWXdaOEU2dmRNQnZB?=
 =?utf-8?B?OE5mVmkwL3Z6cW1sOVZVYVA0M3kvT0g5b1B6QUpQRlJ0QytkVFZ4UjJHWDVq?=
 =?utf-8?B?RTliL1Y3WlVwOGsraDUxejMwRytodGZpWDRpQy8veEUwM2YzS1ZYNE5Taklt?=
 =?utf-8?B?Vzc0MVA5VTh3ZStvUFRFQ2xzaFpTdEpRRDhWVG5ZaDdCbTZXYk9NWHdGbU1C?=
 =?utf-8?B?aEh5VGhyYVVIcUtFR1JiZGdvd1dIcStQWWZRekRobGVGQmppb0dmRkJsa2tG?=
 =?utf-8?B?bHI2cEp6bVVMQkpqbWFSTDZ2anAyVzJZM0txaUdLNDcyTHN6NUQweXlYeHpU?=
 =?utf-8?B?bjNmSE0rUzJ6N2w3eWlVYzBqRTl0UFpFMTVkWmtqcXF0RFAvUGE2emtGS1Zm?=
 =?utf-8?B?cTRNKzN4dEhKL04yeHcyZENvdmtzYy93elFZbzRuZ3hHcndsRC84YnFUUVdV?=
 =?utf-8?B?WHJVdmpDZ2xwNXRlQzZ2NnpSeFJoUDJlLzh1aG5uWDQybm1MalpGZS9xaFZj?=
 =?utf-8?B?VUthWHRMT2Z0cGthUGxWaUF4MkwyY1o5UFAwOFJGOVl2UlJZVzNTMWJSVTZk?=
 =?utf-8?B?bEc1aDcxdkh1OVYvRVpqM0pJRVJRTnI4M2YvYm5RTlhwOUFPS1BkcTZuTDRX?=
 =?utf-8?B?L0NjbEdCemlYY3dQL1JrQmpiVE5zdGg5b2xqVHN3dUVXTENRSFFoa1I1U0xQ?=
 =?utf-8?B?YUNheFRDcmpGa2JzT1RneEdscFpZWE50V0ZvRnVWekdiOU94Y3J1b3ZTWk9U?=
 =?utf-8?B?bXZsTUY3UGkzTkxHRHRZLzBqZlcvTmRNaUFsa3IrWCt3RmVaeURoWFBrQWxo?=
 =?utf-8?B?eTZrcHU2N0NrK0p0YXAyYmVCUkMvVWtHY29NdHYxRXZhVW15c2tVZXdhYkM0?=
 =?utf-8?B?UHFpUWhuZ1ovWnJUamRXN1VGdUlKdjhNNkM1UU9LSWRRS3dHSVRRVFFlNEd2?=
 =?utf-8?B?bWUra1hBc3FiQW1jY3VXYTI1c08yWDkzR3diZTArNGtRaHViS0hGLzVPSlRP?=
 =?utf-8?B?eWRVWlV4Zkh1NENIVFpiVUlscTFvSDdkR3hCdXZlY2Fka1lZOWdMRTFoU0Ev?=
 =?utf-8?B?RjNXdXRBcFJvUGdOZ0MyWEtEOTdSa1FDOGtaRTlUYnZqeHEyUmxMQ3BERGlW?=
 =?utf-8?B?cXJWSCtvQzdSenpFaGQ1WWUycHRGNXNkODJVWFpCczFIdVNEM01xRklEUnE2?=
 =?utf-8?B?WkxGRFBCMkMzakxBbkxNd2F4SjRpdXdCSVkwbmVoQ205aHlwcVZHWHgxOVNa?=
 =?utf-8?B?aW5UNFBOT2FRWnNqcWhmQzRCalludyttWUhseDY1ZjRlbzh2Q0ZnL2tDV1VK?=
 =?utf-8?B?ZjBXOEY4SjVnd2tBNkJlVzIxa0tzaXFoZmc5d2lQb01DeXB3ZTlLbkpiaXBq?=
 =?utf-8?B?N1Z5OXJzcXF3UThURDRDQlJ2UFZhQUN2bDVPNjh2QzNoUnBaK3U2eHhDUlo2?=
 =?utf-8?B?UjJZeEhNNjZtRnZjbmJHSXNUS1huUzJ6YmJieUoyRjE4cnFlMWVWTGp3RkNu?=
 =?utf-8?B?QVJuZUJQMnduYzlrOWliZk5hWVFGM2IzSTg0dzNhZDJmVWN0WWl6ZXBBWVNE?=
 =?utf-8?B?ZWdZMXladUkzcTlUQW42MWZGc1U1VWI5dWJEYXBGVzBGdjZ3d2hKa0lLUjdO?=
 =?utf-8?B?N25WTlZJd1A3UlFzOVI3cjVmWU5oY0lQZURDWHNZSVNYa05hSHRzSWpGUDdX?=
 =?utf-8?B?TGxpbE1sRThFa1g4YkkycUpjNHI3bWF2bGw1QVNXN080aVhkeTQ1dz09?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c68ef7e1-dc21-4410-5dbe-08da38c66fc6
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 May 2022 12:03:35.8953
 (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: SSnm2Hpr1LSa8mVd/prsxfMW7hBXsDhHianLRl00qSbLxCk0JESqmkKQfzCt2Otj
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR12MB6201
X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 18.05.22 um 01:09 schrieb T.J. Mercier:
> [SNIP]
>>> Perhaps we should go just one step further and make a misc device node
>>> for dmabug debugging information to be in and just have userspace
>>> poll/read on the device node and we spit the info that used to be in
>>> debugfs out through that?  That way this only affects systems when they
>>> want to read the information and not normal code paths?  Yeah that's a
>>> hack, but this whole thing feels overly complex now.
>> Yeah, totally agree on the complexity note. I'm just absolutely not keen
>> to add hack over hack over hack to make something work which from my
>> point of view has some serious issues with it's design.
>>
> Why is this patch a hack? We found a problem with the initial design
> which nobody saw when it was originally created, and now we're trying
> to address it within the constraints that exist.

Well the issue is that you don't try to tackle the underlying problem, 
but rather just mitigate the unforeseen consequences. That is pretty 
clearly a hack to me.

> Is there some other
> solution to the problem of exports getting blocked that you would
> suggest here?

Well pretty much the same as Greg outlined as well. Go back to your 
drawing board and come back with a solution which does not need such 
workarounds.

Alternatively you can give me a full overview of the design and explain 
why exactly that interface here is necessary in exactly that form.

>> For example trying to do accounting based on DMA-bufs is extremely
>> questionable to begin with. See a modern game for example can have
>> between 10k and 100k of different buffers, reserving one file descriptor
>> for each of those objects is absolutely not going to work.
>>
>> So my request is to please describe your full use case and not just why
>> you think this patch is justified.
>>
> The use case was described in the commit message when the feature was
> initially added (after discussion about it on the list) including
> links to code that uses the feature:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20210603214758.2955251-1-hridya%40google.com%2F&amp;data=05%7C01%7Cchristian.koenig%40amd.com%7C3f6e3e98fc6f45ead80d08da385a59e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637884257979664387%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=i%2BSfpB%2B6iBolBHu6KrKH3njq0zo1SBbrKDHZOjpsIks%3D&amp;reserved=0

Yeah and to be honest I have the strong feeling now that this was 
absolutely not well thought through. This description as well as the in 
kernel documentation are not even remotely sufficient to explain what 
you guys are doing with this.

So please come up with a complete design description for both userspace 
and kernel why this interface here is necessary inside the upstream kernel.

If you can't convince me that this is a good idea I will just bluntly 
mark this DMA-buf sysfs interface as deprecated.

Regards,
Christian.

>
>
>> Regards,
>> Christian.
>>
>>> thanks,
>>>
>>> greg k-h