Hello,

On 6/8/26 16:04, Maxime Ripard wrote:
> On Mon, Jun 08, 2026 at 03:13:56PM +0200, Maarten Lankhorst wrote:
>> Hey,
>>
>> On 6/8/26 14:42, Maxime Ripard wrote:
>>> On Sat, Jun 06, 2026 at 11:44:10PM +0200, Maarten Lankhorst wrote:
>>>> Hey,
>>>>
>>>> On 6/6/26 18:31, Natalie Vock wrote:
>>>>> On 6/6/26 00:44, Eric Chanudet wrote:
>>>>>> Accept only one "region value" pair entry for the dmem.max, dmem.min,
>>>>>> dmem.low files.>
>>>>>> This changes the UAPI that otherwise accepted multiple lines for setting
>>>>>> multiple entries in one write. No existing user is known to rely on
>>>>>> writing multiple regions in a single write.
>>>>>
>>>>> Ugh, shoot.
>>>>>
>>>>> For dmem.low specifically, there already are some userspace thingies 
>>>>> floating around that may write more than one region/value pairs.
>>>>>
>>>>> These thingies all depend on that one patchset for dmemcg protection that 
>>>>> I should really get around to merging[1]. Since the userspace utilities 
>>>>> depend on not-yet-merged patches, they sort of have to expect stuff 
>>>>> changing under their belts, so I wouldn't really consider those users a 
>>>>> blocker by necessity.
>>>>>
>>>>> As I see it, we could go down one of two paths:
>>>>> 1. We go ahead with the patch as proposed, and I make sure that the users 
>>>>> I know of adapt. Could be a bit icky wrt. "do not break userspace" rules, 
>>>>> but since the already use non-merged UAPIs in one place, you can argue 
>>>>> that these users kind of have to expect breakage.
>>>>> 2. We use the old handling allowing multiple lines for dmem.min and 
>>>>> dmem.low only. This preserves compatibility but uglifies the code by 
>>>>> quite a bit.
>>>>>
>>>>> All things considered, I think I personally would prefer going with 1. 
>>>>> and taking the patch as proposed and just having one codepath handling 
>>>>> every limit file. Just highlighting this so we don't do it on accident.
>>>>>
>>>>> [1] https://patchwork.freedesktop.org/series/163183/
>>>>>
>>>>
>>>> I prefer option 1 as well, but would like an ack from one of the core 
>>>> cgroup maintainers too,
>>>> and what Maxime's opinion on this as well.
>>>
>>> Option 1 works for me too if doable
>>>
>>> Maxime
>>
>>
>> I see this as an acked-by?
>>
>> I'll commit this patch to drm-misc-next if so.
>>
>> Fortunately it may not even break those scripts in the typical case
>> where only 1 region is registered, eg the most common laptop/desktop
>> case.
> 
> Natalie had a bunch of comments afaik, so I was expecting a v2, but if
> you intend to merge it as is, you can add it if you want.

Yeah forgot about that for a second, will wait for v2!

Kind regards,
~Maarten Lankhorst

Reply via email to