Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d2685327-fc5d-b389-88ed-6bad7fda5e36@oracle.com>
Date:   Mon, 25 Apr 2022 12:46:52 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.8.1
Subject: Re: [PATCH RFC v21 3/7] NFSD: move create/destroy of laundry_wq to
 init_nfsd and exit_nfsd
Content-Language: en-US
To:     "J. Bruce Fields" <bfields@fieldses.org>
Cc:     chuck.lever@oracle.com, jlayton@redhat.com,
        viro@zeniv.linux.org.uk, linux-nfs@vger.kernel.org,
        linux-fsdevel@vger.kernel.org
References: <1650739455-26096-1-git-send-email-dai.ngo@oracle.com>
 <1650739455-26096-4-git-send-email-dai.ngo@oracle.com>
 <8640dbe0-cece-4515-fa4f-efa2e0a14303@oracle.com>
 <20220425193545.GG24825@fieldses.org>
From:   dai.ngo@oracle.com
In-Reply-To: <20220425193545.GG24825@fieldses.org>
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?WEVaR21tbVpQOUgwcVY3VU9pUk5sUCtBRmJUeTBvNDhDNG9aMHdhOXpDZlNT?=
 =?utf-8?B?KzRydVNxczY4dkx4UFZzKzZCaCtBVzhHeUpvM1pUWDJZOEJyaURMMllWOTVs?=
 =?utf-8?B?NzRFMU1NMnB4SklrcVZXL2lScklrZEp0OHV1am4wMDhyQkJYR3JmQWp0Tjdt?=
 =?utf-8?B?VzR3blJrdy9zT3o2TU1VNkNOZnRoa290N3NUcnpsNjRrQ0NjWllYbGY4MTNX?=
 =?utf-8?B?b3B0REYwR0dHckpxa1RTMmlIRk5HMnhnWkN6cklZMHJJOExHeEJyRGpqSFdC?=
 =?utf-8?B?SmFCZURwUlBzc1JqTUkrNHpoeWhOYkJKdnRSR0JsdUFxdzY3alozZVJBczR2?=
 =?utf-8?B?WitacHJOdTN1UWFHMGdzUDJGWHhabjV4dW44YnFITmFIRlRoQXRYREtpZzRM?=
 =?utf-8?B?cXNSa1o4czBMMGxvYm9nbHlhWFgxdm5wa0VKVUhWMkRKbUtvbE9DcUVKN0JU?=
 =?utf-8?B?SVdzZm1DeXBuM1UwV1FMSE1CRkNBaXRMUHRjRnp3OS93aGVXMndOc0wrbVFM?=
 =?utf-8?B?N0NKYVF1czB0ajc5c09rMkszTEhya1lsTS9KMURPRTFtSU1pYWNJUE1ZVWkv?=
 =?utf-8?B?RU1UTngxaGtGNFNmZHRpTEo0Qm9adVRrMnd4b0FvM0JrRTVsZWpzc1E3Z2pm?=
 =?utf-8?B?aWw5eGZTTWl1TXhzNDg3aWVVTkJ2MUFKMWNMV1I3QjdwdktzaXUrUXRucFcx?=
 =?utf-8?B?OE1zY2lZbFNaOWJtN2JDaXkrbnJ5UFQyN3JERTVxdTFHNHJiRGxMcERDbEhN?=
 =?utf-8?B?ejhBQUt1ODlMVkFsTzF5YUptQURuZy9HZmR4QVpUR0xnL2lia1lycXBDWnEy?=
 =?utf-8?B?a3hzZUJpdFFnUXc2UzU2ZEZIZ2p5SG1sVWlJNVRsbWRPelZlVGFtVXZrREdl?=
 =?utf-8?B?TmV6ZUx2a0lrZ28yTCtheFp0VHJrYjBORlBOYng3UGc0UzQ4NitMdFgrdjFC?=
 =?utf-8?B?NWtiZ3U5K0d5UkRVTFcrNnBZUTg0dVdPWmsxczNXOTdVaTFORkZNZWpiTHZN?=
 =?utf-8?B?YXQ3TE03QVozUkZkR3doSjUyNGpXOVRvbUZpeDZZbUpaMDlmZS9KaVdRRWtT?=
 =?utf-8?B?R05IRytPRUdOUHZqdkFqUHZGSUJjL01uQ0xrM3FVMDFyUUhSUjRORXlocERE?=
 =?utf-8?B?T1NuS1IxS09JTmJFYVBIWCt6WDg1d2ZrQm9vU1ErUk54OVdXRTBNTjZrS1Jq?=
 =?utf-8?B?ZUNrSng4bWZoUUI0d0NicTI0cHdxRWRFWUZmUEkvNHNYTGRYSUJXU3JrdmFt?=
 =?utf-8?B?NXVwc29rWEJIZXM4NXlPMXN4TEI2aDM5a0M5Tm5EQy9INU45Rm5kM04zWFov?=
 =?utf-8?B?Z3psZlJKRWJSSTZOYUNJWkVqOU9RRXdHQjJMczN5bW80VDVvRENvRVhoNnpF?=
 =?utf-8?B?TmVpRVhBS1B2L1ZTVHVJdCtZeTNiRHJjWTlEM2xxbW44MnR4UjR4Q0NDSUti?=
 =?utf-8?B?eTluRHgzeUNCWWxuOFgwNWQvL0pnd21VZ3VNeDJkcGJFYUdNSHJWMlBxRUVm?=
 =?utf-8?B?bUFiRnpiT0V4Z3JVSmgwOGMrNzVzWXJPVTgydmd3RmZ1bDZqVktRc1JJdVU1?=
 =?utf-8?B?RXlmUGhvVndVV2M1UUV1UHpOQ1kydGxBZGZNa2JLNWtQUG8yT3NjWktHVDdS?=
 =?utf-8?B?b0NVZmQwWnlXZVpFNm5jakpCV0VmKzJyelUvSkxlbENON1JZaDJ3bnZwbjRq?=
 =?utf-8?B?b0VQZjZwZ2pndkJweEsvN0xCVzVYWG9OVGVSRDJPVFhkM2tCNlRxVkxKSC9m?=
 =?utf-8?B?SkZsRGwyQWVBK2FnUHN3YS9YUjNsU0Z2NThXb2plZGkycnNubTJ5Vnd5MGsy?=
 =?utf-8?B?Q0FEcW9McW12ZExUTzJ2Mkdpc3g1MzZ2Y0pPZ0Y1czJXa0EvNVNlb2dsWTBL?=
 =?utf-8?B?VWVQT1ZEY0NDNkdsaE1maFdJVHo0ZDVBZzNCclRrWE4vMzZuV1N5ZTduUm94?=
 =?utf-8?B?WnloQ1ZxTTQ0eUhxTzJ1NURXNDMveWh0WlMrZFVKU0ZaK25MWFo4RVBLTHdO?=
 =?utf-8?B?Qks3NVlRZTdSZk0zWXdtZ0J0TGRiNVFGL0o3Qm1wZVNteWJUQlA3Uzl3YmVE?=
 =?utf-8?B?Z3YzdHEwOU5KNE9FR0hwb0N6VmpmeDZqd0c2NVl2SGtVVlRia1RQUU5leDk4?=
 =?utf-8?B?cHZqK2tPT0pyQVJpQTlXdEVKb25JVzczbWcvaWxXZzVjUDk4VkVMbFVYRlNC?=
 =?utf-8?B?bk15aXc0aUlhck02ZnBNOUlZS1JEV0FETTMzS3NMdFo5cnczeDVhWVd5Y1Fv?=
 =?utf-8?B?QWRZQURTd2dUK0pkdUZqMHZvRlBEb2JnWFIxam53R3M0OTRsQkhVdXBpZUNr?=
 =?utf-8?B?QXBlUXo4bmhCV21ENjVkWmpnd29DTWlSc2Z2eitDVXJlSTRqY1hydz09?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cbb176ce-1912-4181-fdbb-08da26f45a59
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4257.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Apr 2022 19:46:55.7475
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: fDKHh+nEwBq+07181sgMSm2gJ74CktqYVQmrHDbSTJI1Po26bICaRku7GSkWrfSBhj86O7rI6NwGHgBxwnKWxQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3669
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-04-25_08:2022-04-25,2022-04-25 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 suspectscore=0
 malwarescore=0 spamscore=0 phishscore=0 bulkscore=0 mlxlogscore=999
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2204250087
X-Proofpoint-GUID: Rx7_l60wC8wWwqvfv9_c5PZoA7C1S18b
X-Proofpoint-ORIG-GUID: Rx7_l60wC8wWwqvfv9_c5PZoA7C1S18b
X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        SPF_HELO_NONE,SPF_NONE autolearn=ham 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-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


On 4/25/22 12:35 PM, J. Bruce Fields wrote:
> On Mon, Apr 25, 2022 at 08:27:22AM -0700, dai.ngo@oracle.com wrote:
>> This patch has problem to build with this error:
>>
>>>> nfsctl.c:(.exit.text+0x0): undefined reference to `laundry_wq'
>>>> mipsel-linux-ld: nfsctl.c:(.exit.text+0x4): undefined reference to `laundry_wq'
>> This happens when CONFIG_NFSD is defined but CONFIG_NFSD_V4
>> is not. I think to fix this we need to also move the declaration
>> of laundry_wq from nfs4state.c to nfsctl.c. However this seems
>> odd since the laundry_wq is only used for v4, unless you have
>> any other suggestion.
> I'd just leave laundry_wq private to nfs4state.c.  Define
> create_laundromat() and destroy_laundromat() in nfs4state.c too.  And in
> nfsd.h, do the usual trick of defining no-op versions of those functions
> in the non-v4 case.  (See e.g. what we do with nfsd4_init/free_slabs().)

ok, thanks.

-Dai

>
> --b.
>
>> -Dai
>>
>> On 4/23/22 11:44 AM, Dai Ngo wrote:
>>> This patch moves create/destroy of laundry_wq from nfs4_state_start
>>> and nfs4_state_shutdown_net to init_nfsd and exit_nfsd to prevent
>>> the laundromat from being freed while a thread is processing a
>>> conflicting lock.
>>>
>>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>>> ---
>>>   fs/nfsd/nfs4state.c | 15 ++-------------
>>>   fs/nfsd/nfsctl.c    |  6 ++++++
>>>   fs/nfsd/nfsd.h      |  1 +
>>>   3 files changed, 9 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>> index b08c132648b9..b70ba2eb5665 100644
>>> --- a/fs/nfsd/nfs4state.c
>>> +++ b/fs/nfsd/nfs4state.c
>>> @@ -125,7 +125,7 @@ static void free_session(struct nfsd4_session *);
>>>   static const struct nfsd4_callback_ops nfsd4_cb_recall_ops;
>>>   static const struct nfsd4_callback_ops nfsd4_cb_notify_lock_ops;
>>> -static struct workqueue_struct *laundry_wq;
>>> +struct workqueue_struct *laundry_wq;
>>>   static bool is_session_dead(struct nfsd4_session *ses)
>>>   {
>>> @@ -7798,22 +7798,12 @@ nfs4_state_start(void)
>>>   {
>>>   	int ret;
>>> -	laundry_wq = alloc_workqueue("%s", WQ_UNBOUND, 0, "nfsd4");
>>> -	if (laundry_wq == NULL) {
>>> -		ret = -ENOMEM;
>>> -		goto out;
>>> -	}
>>>   	ret = nfsd4_create_callback_queue();
>>>   	if (ret)
>>> -		goto out_free_laundry;
>>> +		return ret;
>>>   	set_max_delegations();
>>>   	return 0;
>>> -
>>> -out_free_laundry:
>>> -	destroy_workqueue(laundry_wq);
>>> -out:
>>> -	return ret;
>>>   }
>>>   void
>>> @@ -7850,7 +7840,6 @@ nfs4_state_shutdown_net(struct net *net)
>>>   void
>>>   nfs4_state_shutdown(void)
>>>   {
>>> -	destroy_workqueue(laundry_wq);
>>>   	nfsd4_destroy_callback_queue();
>>>   }
>>> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
>>> index 16920e4512bd..884e873b46ad 100644
>>> --- a/fs/nfsd/nfsctl.c
>>> +++ b/fs/nfsd/nfsctl.c
>>> @@ -1544,6 +1544,11 @@ static int __init init_nfsd(void)
>>>   	retval = register_cld_notifier();
>>>   	if (retval)
>>>   		goto out_free_all;
>>> +	laundry_wq = alloc_workqueue("%s", WQ_UNBOUND, 0, "nfsd4");
>>> +	if (laundry_wq == NULL) {
>>> +		retval = -ENOMEM;
>>> +		goto out_free_all;
>>> +	}
>>>   	return 0;
>>>   out_free_all:
>>>   	unregister_pernet_subsys(&nfsd_net_ops);
>>> @@ -1566,6 +1571,7 @@ static int __init init_nfsd(void)
>>>   static void __exit exit_nfsd(void)
>>>   {
>>> +	destroy_workqueue(laundry_wq);
>>>   	unregister_cld_notifier();
>>>   	unregister_pernet_subsys(&nfsd_net_ops);
>>>   	nfsd_drc_slab_free();
>>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>>> index 23996c6ca75e..d41dcf1c4312 100644
>>> --- a/fs/nfsd/nfsd.h
>>> +++ b/fs/nfsd/nfsd.h
>>> @@ -72,6 +72,7 @@ extern unsigned long		nfsd_drc_max_mem;
>>>   extern unsigned long		nfsd_drc_mem_used;
>>>   extern const struct seq_operations nfs_exports_op;
>>> +extern struct workqueue_struct *laundry_wq;
>>>   /*
>>>    * Common void argument and result helpers