Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <f6de2b9a-005f-d1f5-9818-cfbee2bdddc5@amd.com>
Date:   Tue, 12 Apr 2022 15:40:52 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH v1] drm/scheduler: Don't kill jobs in interrupt context
Content-Language: en-US
To:     Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        Tomeu Vizoso <tomeu.vizoso@collabora.com>,
        Steven Price <steven.price@arm.com>,
        Rob Herring <robh@kernel.org>,
        Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>,
        Rob Clark <robdclark@gmail.com>
Cc:     dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        Dmitry Osipenko <digetx@gmail.com>
References: <20220411221536.283312-1-dmitry.osipenko@collabora.com>
 <064d8958-a288-64e1-b2a4-c2302a456d5b@amd.com>
 <a04733af-5ff1-a1b4-527d-68b28a037231@collabora.com>
From:   Andrey Grodzovsky <andrey.grodzovsky@amd.com>
In-Reply-To: <a04733af-5ff1-a1b4-527d-68b28a037231@collabora.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 2
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cncxaTdKc21yK3VhR2N5YXVTSEgwSjgzUjY4OGlaZlI5MUs3ck1rT0hZOGVE?=
 =?utf-8?B?UUVMYXA5Z05uazFUZXBwam1FQklSaEh0bXVMZmNwQmhYM1N3SUxsM1hwV2Nu?=
 =?utf-8?B?VDNVbkVwNU5MZ3B3MTVIc1ZJTXFZaVdxd0sxSm1QY01IR21YT25oK3ZpT25D?=
 =?utf-8?B?QWJjeHU3SmZEdzVYbDlGZ1dyWCtjam5YNTRKRE5hVUI0dCs1OFdyWDk1VDdh?=
 =?utf-8?B?dU9jTGlQeWY3Vll2cXBQc1ZsVGowS3QxR0Z3MnNpTVNickl3KzMxbzBHampi?=
 =?utf-8?B?T1lvNktYZUdKRUJQaHBxVjdJVzNoQ2xUVFltVlRQK293OEliSnJJS1NwNmlW?=
 =?utf-8?B?Q2lyVjM4R0ExMXUrV2lMSnNLclBXaWZuR3BtN0RWU1o0aC8xa1B5dUNkOFhE?=
 =?utf-8?B?dXVqSGFmMi8rbm82V0FmZTJPWnQ4RE9tOGtrQzRXenVNZ1A5MGhwS1RRdlJI?=
 =?utf-8?B?aXE2Vmlub1NrK1YydXNudlVKT2lmWjVFMk9aTjBabncrYjZSQ24yZGFDK3hr?=
 =?utf-8?B?Tm0xOFpYYkxDZ2hmNDN3SzI4NE1ram1kVStuMTRPdjZCV2NTSi8wY1c3UjdK?=
 =?utf-8?B?cjBWbEM1QVdVWVRLYXNDcVg1ODU3ZTlCeFNTSWNvNkliejVWMjFNZ3BNa1BM?=
 =?utf-8?B?aC9ESGFjamZHQkVRa1B1T3hERU8rdW5hZTAxOFFGeGtXUys1Y0dybmlxdStu?=
 =?utf-8?B?L0xaOFJ5L0NoR3dreEcwUy9HWFR3WkNoQmNJRUdNS2hpT3FiYlBiNVYxM25i?=
 =?utf-8?B?dmdLcFZWd1lrbzZEWnZXWDFXV21BV2Q4V2p6bElaRFdLZGx4a05Sb2hGUXA3?=
 =?utf-8?B?dkkwYXJRM3hxZGpIVWVEaUlIR09weEtvSU11YzNlcENsbXpmQXFjaVNaZDQw?=
 =?utf-8?B?ZjFqTlFFcVFTdGhFV2xXeGRiNGFicHo5S2lkM2g3U0grNzVkc2h2YW1zQzJh?=
 =?utf-8?B?dkZBNDQrRWdTSGE2Y3dpUjgrZ2dHN3QySUN3djljQ0xQb0UrTWZMdld3UHRz?=
 =?utf-8?B?ZXZNcmZmbHNrQlpoN3pOTFhtQXljUXZUQXl0bEk1NklGcXBUQ1ByMXNuemo1?=
 =?utf-8?B?VFRPUkhKTEM2WmhGTGg5dEM5TGttVmU2T0xQY05SbWpxVjJOWVhwUUgyQVQx?=
 =?utf-8?B?b2gwNkFac0xJMENIUVB1ZlYxcHlEZFZNejhObVNDV3RVSXo4YTVBdTlWTVR0?=
 =?utf-8?B?RDNoZXRKVFNrNjZQMXBXRWwwc0w0UmZCdmRZbzhQMlZJdjhnY2t5K01qSUNw?=
 =?utf-8?B?TnBYTTYxa3p4Y3M4RXlSK3cxbEtQeFc4eUphYUNiQnVocCttTGFLUFc2NG81?=
 =?utf-8?B?TG5taHVQMUt5Q2RxczJVaGo4dWoycTBXNHNHSlRDb2wzRDgzMGczRHlWbWZP?=
 =?utf-8?B?S0tydlFDSUhWUmcvK05vK2VyNXBPTWtWOWp4MGV6L2tydlFLTDYzay82OGRL?=
 =?utf-8?B?ZjgvY2J6SUNPSTFTeWpqNjNPZzhmNlBCZXRnOG9SN0pQK2FqL0FXak9DZElP?=
 =?utf-8?B?WU14d0ZEbm9ubUhrRHo5WHg3RmU1N0pXdUtSOER0SjZaaWVLNExmQW8xRXBa?=
 =?utf-8?B?cldHZXZqTWM0S0JJLzJKWDlHYm1ERDVROVRlRTQzRlMwdmxWbVFRcWpSZHQr?=
 =?utf-8?B?anJnb3RVZDl5STVhRGJkcEdtdlZaMnowZm1IaXlOUi9HRDk0RmdxSEJJalBK?=
 =?utf-8?B?R0NheFdVVkIrd3VQRmRpeitLSC9HbXdlMHN2a2tTTnJtVGxxMTlmV0FZZmtk?=
 =?utf-8?B?aVRLLzhEZ2ZOUG96a2Zwb2ptZ0hONGhKNjFWWk1wVGNLTmNsVmpaeWVBQ3d3?=
 =?utf-8?B?U3JGaFZrT3FWZ1FNbWhKb2NqODVUQ2t1R1loNkN1cDJYZTdhSWxGZjBjUmRi?=
 =?utf-8?B?NFdTZ09ZalUyOW5seGlhclpRVnhLSHRiaWQ4dkFLaERCenk3eEdkS0JpMkZB?=
 =?utf-8?B?YlBkN3dTVVNWdHRxZWJOM2s3a0kwbGw3QkJwQkVLK1pKRnJ6Y2Z1d2pGaXBX?=
 =?utf-8?B?TlVyaHFmNklQODRVYkloN3pCQi9JUnJiUlRLYk1UbXlvdUlvZXdyQ2dHWlFI?=
 =?utf-8?B?aHFJNStRc2RRRlpDZHRUNmtzTTN2cW9SbGlEbXo0TnNTYVlLTkZ0dzdXaDZW?=
 =?utf-8?B?VFIyNnhNNkRzTHZtcDRINXFSY09GRmxjZ3NXQzV3NEdDWHE3ZWZBR200akRp?=
 =?utf-8?B?QjBoUjg0UGpPb3NodUVRbnpHMXNWRkUrZFJSK0NBUnd0WEtkRVFraTY4MWZI?=
 =?utf-8?B?S2NSL1NDalJsNGs0M1loVmpOdk96OHdhdEpabDZYaWRvRCtpTnZQV2YwUndq?=
 =?utf-8?B?WDVSUExwYzhLN3Y0SjlHZkxnSXA2c3BYS2lHcGEzL2l1NXpIRVpMaWxETUQr?=
 =?utf-8?Q?A9Hxg5WLomWUW1PbRiDNBUEe0xr4k9oJJ+tMBWkYq8Y9u?=
X-MS-Exchange-AntiSpam-MessageData-1: 7Fzy4evj1h/GFw==
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d3b3ee33-15ef-4679-a0c2-08da1cbc5bba
X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1947.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Apr 2022 19:40:54.8795
 (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: O4CNoftl/4w6GwmQOyVZsD3wNXsU5af98gst5TFoVttv0rx8KNfcbnb376618wgiNJNT5uR9w3YV4j3exGNRKQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR12MB5285
X-Spam-Status: No, score=-3.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


On 2022-04-12 14:20, Dmitry Osipenko wrote:
> On 4/12/22 19:51, Andrey Grodzovsky wrote:
>> On 2022-04-11 18:15, Dmitry Osipenko wrote:
>>> Interrupt context can't sleep. Drivers like Panfrost and MSM are taking
>>> mutex when job is released, and thus, that code can sleep. This results
>>> into "BUG: scheduling while atomic" if locks are contented while job is
>>> freed. There is no good reason for releasing scheduler's jobs in IRQ
>>> context, hence use normal context to fix the trouble.
>>
>> I am not sure this is the beast Idea to leave job's sw fence signalling
>> to be
>> executed in system_wq context which is prone to delays of executing
>> various work items from around the system. Seems better to me to leave the
>> fence signaling within the IRQ context and offload only the job freeing or,
>> maybe handle rescheduling to thread context within drivers implemention
>> of .free_job cb. Not really sure which is the better.
> We're talking here about killing jobs when driver destroys context,
> which doesn't feel like it needs to be a fast path. I could move the
> signalling into drm_sched_entity_kill_jobs_cb() and use unbound wq, but
> do we really need this for a slow path?


You can't move the signaling back to drm_sched_entity_kill_jobs_cb
since this will bring back the lockdep splat that 'drm/sched: Avoid 
lockdep spalt on killing a processes'
was fixing.

I see your point and i guess we can go this way too. Another way would 
be to add to
panfrost and msm job a  work_item and reschedule to thread context from 
within their
.free_job callbacks but that probably to cumbersome to be justified here.

Andrey


Reviewed-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>