Message-ID: <182e3214-0f60-8b65-5981-23d2a2ea54b9@nvidia.com>
Date:   Tue, 31 Jan 2023 10:17:21 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH 1/5] genirq: Use hlist for managing resend handlers
Content-Language: en-US
To:     Thomas Gleixner <tglx@linutronix.de>,
        Marc Zyngier <maz@kernel.org>, Michael Walle <michael@walle.cc>
Cc:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        Hans de Goede <hdegoede@redhat.com>,
        Wolfram Sang <wsa+renesas@sang-engineering.com>,
        linux-kernel@vger.kernel.org
References: <20230130005725.3517597-1-sdonthineni@nvidia.com>
 <20230130005725.3517597-2-sdonthineni@nvidia.com> <87pmav13ly.ffs@tglx>
From:   Shanker Donthineni <sdonthineni@nvidia.com>
In-Reply-To: <87pmav13ly.ffs@tglx>
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?a0plMFFjcWZjS255V0V1VTliQXJnVXI3TzdZQ1J0N1orckhoZ280S1lGdHZW?=
 =?utf-8?B?aDlFbjErK3lDVHBBcWlyM25oZncxdE5UNzMybUFPZnROc2ZuUUs1OEFoS2cr?=
 =?utf-8?B?WHRwNXo5SWc1ay9TbGs4b1JCQWpibmx5bHA0TjFGQm1acjlqeTFTVW42R2tr?=
 =?utf-8?B?RFFHSVU1SWN5dUdGNXVNdEdCdzRmU0ZVUlY4RHVKak5STEdRU1VIRWtUQXU1?=
 =?utf-8?B?a0t5QTRxVUdQbU5BU2c5U3ZGbTNhZ1hhMk16QVk5dmVteng0VUlwMzB2YjZW?=
 =?utf-8?B?b0k5K0l3S2UyYjdnOVV6UmhFZTZNNmlBZTRkVEZlaU1CbFFqVjhTRlloVGdk?=
 =?utf-8?B?SzZqbnhkdG1CdEJlcitqMyt4TkdyendJWXZtWndhZlQvbWRqNCtxUE5wZ1BK?=
 =?utf-8?B?RUx2cm5sYVlqMWNPbWVIRGN2N3FrcXNkeWpqMkJobnhvaTN3cWVqU0hHVEg1?=
 =?utf-8?B?dk50Umc2RUZReE1wd05hR3BObG5LeHB6L0NGM3hCYk9aMUptT0dWTGM4TGlS?=
 =?utf-8?B?NzJIR081S2xEQ09ZT1l0TTFIYllFNHRoWVNYMm5hNDlqODhjM1g2ekZpS3Mv?=
 =?utf-8?B?TU9nZFZ2MXVMTWZRT29oNUxHakNEcXYwVVlCY2RmZHRtTGRTVlNTSDRJSmNp?=
 =?utf-8?B?OFB3ZFpjUjc1elMwcmwyRWZiYnVKTkt6TlpSMnNkVzRKWVU2YjV0TldMekRG?=
 =?utf-8?B?S09ubGF3czM4WkwrQzVqelFVaVhES2FiSDNKMGZOYjgrNXZzZHlzb1FjeHZm?=
 =?utf-8?B?Z3ZuMXRlLzB0NzBKK1htTHd4SVA3TDEvRXkrWnZCeko4ekV5UFBmYUhZYnow?=
 =?utf-8?B?T1N3WUhBVjk0dmN6OWJnVzVkdWY1VTFXOFVSbWlSQlVQRmNndzhpWmRUSDAx?=
 =?utf-8?B?dndGa0w3OWFON2U5WjhHeENtVnhlQnlJRFV2aFlpdHRqZm5uSkdmanUzOEFs?=
 =?utf-8?B?ODZyWE8ycWg5eERoN2xWdkZaTStzakVaZDdjRmc5NkZNS3psZE9FaTJqUG9m?=
 =?utf-8?B?eVZOeFlDQnV1eXlkNUI1b3VwNVgxMUdLVWtETVpsUHlMTUYyYVdubFFHbVcw?=
 =?utf-8?B?cHQyZ3BVdVgrWTYrQmlUQXJWY1R1emV5QnZKNzh4Lzhtc042OTNxQ2o3c3k0?=
 =?utf-8?B?MzVEM3luemxtVkhWV0xaOWRTMVpKdzI0QlVjSVdFcDhjOEZiQmtDdVI5clB0?=
 =?utf-8?B?cVlkZEN5SEZxS29GWi9pRjZSS1pHQ05wSEtmdUMwNkd4emtZOWZ1MDJqWDh6?=
 =?utf-8?B?M3VTdDZRSkJwb0RaQXp0UkRCSUNuQjBLTXVtbXFEREdpanp6dnlhdVhkRytM?=
 =?utf-8?B?TDBSdEVkTTNtUzlRQVppV29wNS9YbjFOcUoxMkcxWE0zOUFKRHhrd3FiYlAy?=
 =?utf-8?B?Q2QyZ0gvYlgyU2txbTFUdXg2K0M4QXFFY0lLa0YyTjUwY2tqbnExQUZlTmQv?=
 =?utf-8?B?amlTWXlhZnhUTnJleXVSZW93WlY1MmNZV25KMkhJKzg1RkhCeEJLN0FoUndq?=
 =?utf-8?B?NWVrRUFqeWtSWVhhR29Tbi9hNzVFTitaWS9YUUFlSWRFcDgwSklCcUp4TU8v?=
 =?utf-8?B?b0tIVDhqaHhzckltK2NwNmxiSGl6akNQVWtOYy9PME9yclZWTjlwTnFDUzZN?=
 =?utf-8?B?TlM3bWJTb0xUbUZzbFlmQ3J5UEhYaENSTW5KZjNMRkR1NnhEcFZ4dGkxU3NH?=
 =?utf-8?B?Yk52UldNK0p2OTNielBWQVBtbUZSQTRpRnZDWmhTaWY2TEZCRXFkR1EzbnNJ?=
 =?utf-8?B?OUorUDZxU1IzL0IzbkhFUnRxU2RjcW9Tc2JYZThaSFNYalBaTjNNK3YwS25Y?=
 =?utf-8?B?NWo5cnZlTy9MQ2lIV1VoMGVHVFdZM3JoSkZqVnFUZm9rQ0ZscnRxSlFtaThz?=
 =?utf-8?B?S3hKa1Q1REFVTWtRemJ4eGZGREFub2FGL3VGQ244SGRhT09FVE9ST2g2N3h1?=
 =?utf-8?B?SDZmYVBBQ1A0bC9pTjlHUGQ1b0MrcWZGbkhIV3prN3hjMkN4bzZMODB4SFA2?=
 =?utf-8?B?K1JpdzB0NDcxcTE0QXQrSXB6K2xYT21Dd1BoU3RkSG5vU3QydHJGai9ySUxo?=
 =?utf-8?B?Lzc0TGdscm5aaUxwbkt5YXE4TmFsbXF5V3lZYWp2Uy91MyszMmdySXk1WUx2?=
 =?utf-8?B?YVJtNVZuNjhzVUhJYUNVNE8ySXBIUmVURXlpSWx4UEVnSkh1eEFpeWdMbHdy?=
 =?utf-8?B?OEE9PQ==?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c6b7b32d-da65-4a36-a7ad-08db03a6a304
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB5762.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2023 16:17:23.9689
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Fap+Z3xBrvB/jvCK1wS7Mx6Gi31bPp1IAcfEAngrKGX1U86BTTTBIoZPm3LUvsLFQP7QA32bXJ2odvJwjKpnoQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4223
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 1/31/23 02:59, Thomas Gleixner wrote:
> On Sun, Jan 29 2023 at 18:57, Shanker Donthineni wrote:
>> +/* hlist_head to handle software resend of interrupts: */
>> +static HLIST_HEAD(irq_resend_list);
>> +static DEFINE_RAW_SPINLOCK(irq_resend_lock);
>>
>>   /*
>>    * Run software resends of IRQ's
>> @@ -30,18 +31,16 @@ static DECLARE_BITMAP(irqs_resend, IRQ_BITMAP_BITS);
>>   static void resend_irqs(struct tasklet_struct *unused)
>>   {
>>        struct irq_desc *desc;
>> -     int irq;
>> -
>> -     while (!bitmap_empty(irqs_resend, nr_irqs)) {
>> -             irq = find_first_bit(irqs_resend, nr_irqs);
>> -             clear_bit(irq, irqs_resend);
>> -             desc = irq_to_desc(irq);
>> -             if (!desc)
>> -                     continue;
>> +     struct hlist_node *n;
>> +
>> +     raw_spin_lock_irq(&irq_resend_lock);
>> +     hlist_for_each_entry_safe(desc, n, &irq_resend_list, resend_node) {
>> +             hlist_del_init(&desc->resend_node);
>>                local_irq_disable();
>>                desc->handle_irq(desc);
>>                local_irq_enable();
>>        }
>> +     raw_spin_unlock_irq(&irq_resend_lock);
> The lock ordering is broken here. irq_sw_resend() is invoked with
> desc->lock held and takes irq_resend_lock.
> 
> Lockdep clearly would have told you...
> 
>          raw_spin_lock_irq(&irq_resend_lock);
>          while (!hlist_empty(...)) {
>                  desc = hlist_entry(irq_resend_list.first, ...);
>                  hlist_del_init(&desc->resend_node);
>                  raw_spin_unlock(&...);
>                  desc->handle_irq();
>                  raw_spin_lock(&...);
>          }

Thanks for catching mistakes, I'll change it to this, please correct me if
I'm doing another mistake.

static void resend_irqs(struct tasklet_struct *unused)
{
         struct irq_desc *desc;

         raw_spin_lock_irq(&irq_resend_lock);
         while (hlist_empty(&irq_resend_list)) {
                 desc = hlist_entry(irq_resend_list.first, struct irq_desc,
                                    resend_node);
                 hlist_del_init(&desc->resend_node);
                 raw_spin_unlock(&irq_resend_lock);
                 desc->handle_irq(desc);
                 raw_spin_lock(&irq_resend_lock);
         }
         raw_spin_unlock_irq(&irq_resend_lock);
}

Thanks,
Shanker