Received-SPF: softfail (google.com: domain of transitioning linux-nfs-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <a06066fe-4fbf-0fcc-3ae8-107fd1c45183@oracle.com>
Date:   Wed, 27 Apr 2022 09:41:16 -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 v22 1/7] NFSD: add courteous server support for thread
 with only delegation
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: <1651049573-29552-1-git-send-email-dai.ngo@oracle.com>
 <1651049573-29552-2-git-send-email-dai.ngo@oracle.com>
 <20220427161221.GD13471@fieldses.org>
From:   dai.ngo@oracle.com
In-Reply-To: <20220427161221.GD13471@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?dUtHV0o0ZXgxZGJoa0lIcFE5Rm12UW9ObGJVQ09uU05wUEJOT3p4RXdOMWtk?=
 =?utf-8?B?TzArRnNMdjl5MW5ITzFLdE00RHZselI0TnRvaHhDZU5HL2hrWTZ1aEJBVHA5?=
 =?utf-8?B?OVIxYWR3b1ZNRlhBOUgvblRQYk5mK2k1dWFKR3lmNmtCZXhzWGpOME5RRnVJ?=
 =?utf-8?B?WGtvenRFV2lvRUh5dFdrRVZVV2FBSzlncVU2QXFhbmR6YUdHWUVXNUx4ZEU3?=
 =?utf-8?B?K0g0QitSOUdtRkVObjk0OUhDWW1ZS1lKZlZJSzdqRkkyd0VueDFGVVBSaU9r?=
 =?utf-8?B?SlJYUmZ5ZG1MdTZNWkpKT3p5bnFOSzRqYWk4YVZLWVdDc2ZJS01oQThQTVhM?=
 =?utf-8?B?Q3c1cENYMGdVSW5aM203MkJRV1lrYmxMeUZpUXp6dTdzNldScmM5UGxHbXNM?=
 =?utf-8?B?WEpLSmQyMkxpZ3UwdjJ6TXdkUWhpaGxGZi82Ykx0NlFmMUlWc3lDWXA3cTZS?=
 =?utf-8?B?ODgyR1Z6WityMmU3ck1FNno3MnBBTCt0eE5RZTNuRXBVR1NScWQ5RWJ2MWR5?=
 =?utf-8?B?c0pDcUFTV09KVXArSCtlTnBzQnhTcExTYXYrSGRUQ0VydElKZ1JLSDlFTkZ6?=
 =?utf-8?B?VFVDVUJyTzQ5bXlBK21vMkRTWEE1Z2RCM2RXa1RyV0x5ZHE4OHh6YTBMaFlJ?=
 =?utf-8?B?aDJHSEpldGtDc3dJbUpYYTQyMjFUQ3Q0bU4xTHJQUUNpTDk4djV6SDBDTXE4?=
 =?utf-8?B?WGY2Uy9QcTBkak8wS1NMa2hlWThhUTBkNG4rTVk2b0pXbXBSQTlCVVBVWk1k?=
 =?utf-8?B?amdJMXk1SGt3bFB5R2RwN3RpU3B2Ky9ZbjlCQ24yeUZUaGVIRXE4dzNpaG5i?=
 =?utf-8?B?dlVWZGNrcG4zSzFZNEF2MndrTTR6YjJtQzhQNm5vaWFjaTI1S2hwR1VsQ1ds?=
 =?utf-8?B?ZU1QM0F2U3RneldhYmQvdlFQelI3c3dsd0RnazZDa1UvczhPcjRGamE1N0xZ?=
 =?utf-8?B?cXZmZjdTRlltSWNCK2RaUFVmSmlLeEdSUDNTemk1dys5R2hvUkJ6RjdGNTdl?=
 =?utf-8?B?YjVma0dQZEhDU2JTUXZLeUhCb2ZnQXVqaStJbWtOZUk1aUhyWFZRM1I0dklC?=
 =?utf-8?B?VDJMdzYvWEU3b09kdWJESHRzM3Z1NEt4MVM2bXRIY1p6YXNRRHk2b2tqck40?=
 =?utf-8?B?RElvOU44RXVXcHZmb0JDNVRDTnNMRFB0QWFUWnFBeE1OaWxScVdseFJiQ0lz?=
 =?utf-8?B?clhydERCb09RUzB2MTBxMHpURCs1UVl3RnMyRWRpZGIrWm1vVUlxT3U2V3NG?=
 =?utf-8?B?OFFJTnJZU1MzTG5JLzMvVkhweXBWeVIrZnhsSlh2MTMvVkhMMWsxdjZ5RTAx?=
 =?utf-8?B?ZzFXR3ZwOEFmUGdVcEFBNFFveDNwUzNSc29hWjJnbkhCQldnTVBmY0J2NUU1?=
 =?utf-8?B?MWZhQ3FhT3lGcVZrRzAxUnhvaXJQemFyTm0vNDZnL0pHQlZyQ1Y1enVCQVRo?=
 =?utf-8?B?NENuV091L1pnMURXOHF6TXowVFR1RE1leGlYMThrdUk1OTZKc0JncmZhZTF5?=
 =?utf-8?B?aHRUb1JVekhVV2pYSDF3NHR5QkY5RnMzLzdiVndzekZRRVBiRXh2cDdwRHJY?=
 =?utf-8?B?TUlkUWxTRStnTCtad1BXWnJIR0J3Wkg0MHIvbjk2dFFxM05GK2ZucmcxZkJ5?=
 =?utf-8?B?U0NEQ0Q5Q1UxUThmWHptVUozVDZ5OE8xK0FiTjhOOUxzeTJLM1hFaGNrZWlI?=
 =?utf-8?B?RE1COVVOZURRSUpCbVhPaEt3U3I4QnhHU3A2ZFBjWDZmVWZrZCtFVjkyZUFy?=
 =?utf-8?B?Ny9kaG5wNHV4Mi9UQzVtNUpYY0tLdmU0UTdKZEZ2Mm4vT1l2Mm1aak45YVJl?=
 =?utf-8?B?aVllNGNmRVhTT3h3MkppNHlCeUROQ25pSDJCWWRGbGpzZ1NlN2xSM1VnMjd2?=
 =?utf-8?B?UkpmMjhlbkZtY2JRRXJZRXZOUnA4NDlRS1lVeXpKcGJGR3d2RFRSdmZ5ZXlY?=
 =?utf-8?B?cUlxVDdoY0tWbTZnWVQrZFRScXQyYmR3TkpOZVF2MU50VHErcWlDaW8vcjZk?=
 =?utf-8?B?eUxRaG9Ic1Q0cFRWWTc0SnIyVHlORUtSZ2hscURMY2hPV29zWlRVa011MW5Q?=
 =?utf-8?B?Snc3Ull6R0l1UThQQUhLYkJhMUtzUmpDc3o4Y0JHcy85ZnpvSkFYV2YyQXZo?=
 =?utf-8?B?UEFCRXgyRDdIZHhhc0ZuSWlXdVVsK2lnd2pWZHI1U1NTS282Y3FLSWsvV0RP?=
 =?utf-8?B?Y0h0MmVvUHNmeWs4UXZGTGNFSmg1cjhUbU1SREhQeEh6Ukh1dGd3Wk5JSVc1?=
 =?utf-8?B?QzZrSU9LUTF3ZE0yUEY0SktCTFoyRWhHZk9WTUVUZExGdHZRUGY0eGpyaDRV?=
 =?utf-8?B?OUVWaHNZd2dOT1YwbDB6U0o3THl1R0FsY0pGWWlZY2NNU3FpS2Vkdz09?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 74faf6ff-13af-4cf8-675e-08da286cc197
X-MS-Exchange-CrossTenant-AuthSource: MN2PR10MB4270.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Apr 2022 16:41:20.0068
 (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: 5rTAqoi4Awq34osKqOvcktoQgAuZ2KUaIaz0KwoFbIrIDqOU8KUysduSDSVhVDzvCAQwxiayTYX2Htz4U0p3Ng==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR10MB4673
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-04-27_04:2022-04-27,2022-04-27 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 spamscore=0 adultscore=0
 mlxscore=0 bulkscore=0 suspectscore=0 malwarescore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2204270104
X-Proofpoint-ORIG-GUID: BrAYWgKRfkk_m164NbmwwQQxgOuH2J70
X-Proofpoint-GUID: BrAYWgKRfkk_m164NbmwwQQxgOuH2J70
X-Spam-Status: No, score=-3.8 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
	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-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


On 4/27/22 9:12 AM, J. Bruce Fields wrote:
> On Wed, Apr 27, 2022 at 01:52:47AM -0700, Dai Ngo wrote:
>> +	if (!try_to_expire_client(clp)) {
>> +		nn = net_generic(clp->net, nfsd_net_id);
>> +		mod_delayed_work(laundry_wq, &nn->laundromat_work, 0);
>> +	}
> ...
>> +static inline bool try_to_expire_client(struct nfs4_client *clp)
>> +{
>> +	bool ret;
>> +
>> +	ret = NFSD4_ACTIVE ==
>> +		cmpxchg(&clp->cl_state, NFSD4_COURTESY, NFSD4_EXPIRABLE);
>> +	return ret;
>> +}
> Hm, this feels a little backwards to me.

yes, I should have renamed the function to something like 'is_client_active()'.

>    I'd expect
> "try_to_expire_client" to return true on success.  Maybe call it
> "client_is_expirable()" to make that clear?  Or stick with
> "try_to_expire_client and make it return 0/-EAGAIN.
>
> If "NFSD4_ACTIVE != cmpxchg(...)" is too confusing, I don't think we
> really even need the atomic return of the previous value, I think it
> would be OK just to do:
>
> 	cmpschg(&clp->cl_state, NFSD4_COURTESY, NFSD4_EXPIRABLE);
> 	return clp->cl_state == NFSD4_EXPIRABLE;

I'll change to this:

static inline bool try_to_expire_client(struct nfs4_client *clp)
{
         bool ret;
	
	cmpxchg(&clp->cl_state, NFSD4_COURTESY, NFSD4_EXPIRABLE);
	return clp->cl_state == NFSD4_EXPIRABLE;

}

and its caller:

	if (try_to_expire_client(clp)) {
		nn = net_generic(clp->net, nfsd_net_id);
		mod_delayed_work(laundry_wq, &nn->laundromat_work, 0);
	}

-Dai

>
> --b.