Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Received-SPF: Pass (protection.outlook.com: domain of seco.com designates
 20.160.56.80 as permitted sender) receiver=protection.outlook.com;
 client-ip=20.160.56.80; helo=inpost-eu.tmcas.trendmicro.com; pr=C
Authentication-Results-Original: dkim=none (message not signed)
 header.d=none;dmarc=none action=none header.from=seco.com;
Message-ID: <d4737c45-2bbf-d364-9768-20baa46f6af4@seco.com>
Date:   Tue, 4 Apr 2023 12:04:57 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH v3 2/2] soc: fsl: qbman: Use raw spinlock for cgr_lock
Content-Language: en-US
To:     Crystal Wood <oss@buserror.net>, Li Yang <leoyang.li@nxp.com>,
        linuxppc-dev@lists.ozlabs.org, linux-arm-kernel@lists.infradead.org
Cc:     Vladimir Oltean <vladimir.oltean@nxp.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Camelia Groza <camelia.groza@nxp.com>,
        linux-kernel@vger.kernel.org, Roy Pledge <roy.pledge@nxp.com>,
        "David S . Miller" <davem@davemloft.net>,
        Madalin Bucur <madalin.bucur@nxp.com>
References: <20230404145557.2356894-1-sean.anderson@seco.com>
 <20230404145557.2356894-2-sean.anderson@seco.com>
 <48dacc58c7c04ba8a005d8edd56744c8455f007e.camel@buserror.net>
From:   Sean Anderson <sean.anderson@seco.com>
In-Reply-To: <48dacc58c7c04ba8a005d8edd56744c8455f007e.camel@buserror.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-Microsoft-Antispam-Message-Info-Original: HNehBWq39FDrO2xoctyLlO/Km4NI+2+e1ghOE4fAbSZMU+pkC9svdxoj8pkDyT7rSdk8skBE5kJtd/a7MMie+P9hVmGymqvVyO9rafyBk0zLTfo8NirjX9teKfX8sdcOALjbBxjR/2y8cP4n+qYiO6q8CQKNMriNEZ1YMW7zKwTTXcmn3crMiTuLvq04jX3fRrNM1keEOriZkx1zbP9xAeggnpmA+5fjxj4txbOD8rbQI1SIdIjZTD1tnvfUarS99eeCtLPz6Fd0Rt/fX7e4bqyE+mqgEl/+aFxZy+J8ZnZNws98ojaLUqpzVdx+nYNTIit39k4Mtz2d2Mz3Y9gtl0S144wwqSDkuMHr/zv2tMexTINrHVat+e7qo6AJ2G3mfdPGyNaBicWmd1y5E4z+vkdtVIkZLNIWsNbXb0Epg/AFZRPAGKuZsX4LaVojIuPaF7D/y5lTXY5nIrd9J/87L8uLl/W6xWCh0KlT/PpTEcSOuph1Fs/SikgoNynNbaabRwC5V6w9AqkBRRanGcBtPOggwfCfJqJR11V1qhJrwT9QBpkGGSVh+ljcGny99EUwpir+PNZty2VZAMK1i+2yvBCVs23XAq67giY+xSIJw/RtpkJ5ZJur69PGr2JQG5nhiG9QgjcAJRmcvJDKvKU67xgoURvl0KN/YH9dTy+Yfzw0VPKPea65dhiMw6XrsNre
X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DB9PR03MB8847.eurprd03.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(39850400004)(396003)(376002)(366004)(136003)(346002)(451199021)(7416002)(5660300002)(44832011)(86362001)(83380400001)(6666004)(186003)(6486002)(31696002)(53546011)(6512007)(26005)(6506007)(38350700002)(38100700002)(2616005)(8936002)(66946007)(66476007)(8676002)(478600001)(66556008)(54906003)(4326008)(52116002)(110136005)(36756003)(41300700001)(316002)(31686004)(2906002)(45980500001)(43740500002);DIR:OUT;SFP:1101;
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS2PR03MB10139
X-EOPAttributedMessage: 0
X-MS-Exchange-Transport-CrossTenantHeadersStripped: VI1EUR05FT060.eop-eur05.prod.protection.outlook.com
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id-Prvs: 52e6f84f-c9a6-4ed5-1223-08db35265915
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: SLBMLKXckImL4KhmbYMrbpgCukIusYz/khktkqS3hPihXySFxYof13YrM6COCl1k7neVPfgh4E2SAMuZAvuTjwfQJFOquNTLR4XZdXyeXdtyTW8M3cdGlXpkrHd/O0J/+uqOUpAHqY1VH88ggkkSZa41Ji+hZ1vv0lZ9yJ7ZzwZSzjEkNG1pXuFKoLjnjPlGqEpXCFEdbfREVCQ8WhEVgqmZnUkcZ7Cn/s0FLj6qv2ZwemBswVoYSAKjiQugaSFBvQL68Z0EUoDY7oaeS5aHthAutNCBX0+Jwy+w+cHTzv/SnntWQeiIy6EXrSMnyUAGharABJMGav+EOSIgUw1jdAvQklj0GJJVpNJOOTnnXTkUuRpkibjnxOelHs59N0SNGujvW0XZC/kMyp7onl8jdLEKBr7VJY4LJVardRl1MbQmkhphOcpMT4TMDDGjU/pu7htsTvwdx3rk461hx+pkxWuY7BlyhptunhNyCyRH/ZBhn8DrqRlbEQYDk0p4DfqHne6YPL/7m8QJmUVIPHkwXez+mZsDQiVYl4V45+hWXVQUIiDHVpyqSeeXCfBmJAAP5H6gfx5JsU2c3xKA8hShkxmEybH9H1/xIF/YsMHpaASmVNxYnIwoPiFhEwja4GO7F8egr1eekmWGfhB86sy5zy6nJOZ+E/ORTN2ztOD0hllL93DyWmQih9s6hRaw+Xqepdg0gxkYz8dAjAWDM9Hbg9TF12mUM8P8LfyGDjtBFtOzxXaDlYMcNK09PBSot4yAJcLB35H02F41nk6tyHCZwA==
X-Forefront-Antispam-Report: CIP:20.160.56.80;CTRY:NL;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:inpost-eu.tmcas.trendmicro.com;PTR:inpost-eu.tmcas.trendmicro.com;CAT:NONE;SFS:(13230028)(136003)(376002)(39850400004)(346002)(396003)(451199021)(36840700001)(46966006)(40470700004)(2616005)(336012)(36860700001)(83380400001)(47076005)(6666004)(34070700002)(6486002)(6512007)(478600001)(316002)(26005)(186003)(54906003)(6506007)(53546011)(2906002)(110136005)(5660300002)(7416002)(44832011)(40460700003)(36756003)(7636003)(41300700001)(4326008)(31696002)(8676002)(356005)(70586007)(70206006)(82310400005)(82740400003)(8936002)(86362001)(7596003)(40480700001)(31686004)(43740500002);DIR:OUT;SFP:1101;
X-OriginatorOrg: seco.com
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2023 16:05:12.7605
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: b582d2db-5fe5-40fc-2ccf-08db35265f6c
X-MS-Exchange-CrossTenant-Id: bebe97c3-6438-442e-ade3-ff17aa50e733
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=bebe97c3-6438-442e-ade3-ff17aa50e733;Ip=[20.160.56.80];Helo=[inpost-eu.tmcas.trendmicro.com]
X-MS-Exchange-CrossTenant-AuthSource: VI1EUR05FT060.eop-eur05.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR03MB8325
X-Spam-Status: No, score=-2.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS 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 4/4/23 11:33, Crystal Wood wrote:
> On Tue, 2023-04-04 at 10:55 -0400, Sean Anderson wrote:
> 
>> @@ -1456,11 +1456,11 @@ static void tqm_congestion_task(struct work_struct
>> *work)
>>         union qm_mc_result *mcr;
>>         struct qman_cgr *cgr;
>>  
>> -       spin_lock_irq(&p->cgr_lock);
>> +       raw_spin_lock_irq(&p->cgr_lock);
>>         qm_mc_start(&p->p);
>>         qm_mc_commit(&p->p, QM_MCC_VERB_QUERYCONGESTION);
>>         if (!qm_mc_result_timeout(&p->p, &mcr)) {
>> -               spin_unlock_irq(&p->cgr_lock);
>> +               raw_spin_unlock_irq(&p->cgr_lock);
> 
> qm_mc_result_timeout() spins with a timeout of 10 ms which is very
> inappropriate for a raw lock.  What is the actual expected upper bound?

Hm, maybe we can move this qm_mc stuff outside cgr_lock? In most other
places they're called without cgr_lock, which implies that its usage
here is meant to synchronize against some other function.

>>                 dev_crit(p->config->dev, "QUERYCONGESTION timeout\n");
>>                 qman_p_irqsource_add(p, QM_PIRQ_CSCI);
>>                 return;
>> @@ -1476,7 +1476,7 @@ static void qm_congestion_task(struct work_struct
>> *work)
>>         list_for_each_entry(cgr, &p->cgr_cbs, node)
>>                 if (cgr->cb && qman_cgrs_get(&c, cgr->cgrid))
>>                         cgr->cb(p, cgr, qman_cgrs_get(&rr, cgr->cgrid));
>> -       spin_unlock_irq(&p->cgr_lock);
>> +       raw_spin_unlock_irq(&p->cgr_lock);
>>         qman_p_irqsource_add(p, QM_PIRQ_CSCI);
>>  }
> 
> The callback loop is also a bit concerning...

The callbacks (in .../dpaa/dpaa_eth.c and .../caam/qi.c) look OK. The
only thing which might take a bit is dpaa_eth_refill_bpools, which
allocates memory (from the atomic pool).

--Sean