Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <6919f0a9-b3e0-b5c3-0669-4b84beed84a6@oracle.com>
Date:   Mon, 21 Mar 2022 11:10:09 +1100
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.7.0
Subject: Re: [RESEND PATCH v7 1/8] kernfs: Introduce interface to access
 global kernfs_open_file_mutex.
Content-Language: en-US
To:     "Eric W. Biederman" <ebiederm@xmission.com>
Cc:     tj@kernel.org, viro@zeniv.linux.org.uk, gregkh@linuxfoundation.org,
        akpm@linux-foundation.org, linux-kernel@vger.kernel.org
References: <20220317072612.163143-1-imran.f.khan@oracle.com>
 <20220317072612.163143-2-imran.f.khan@oracle.com>
 <87r16z191l.fsf@email.froward.int.ebiederm.org>
From:   Imran Khan <imran.f.khan@oracle.com>
In-Reply-To: <87r16z191l.fsf@email.froward.int.ebiederm.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bG9wYWRzWGd2YXVHSFEzcXRISVlCMTg4ZmpQTFVpZnY1dHBHRHhQWWV2dTE3?=
 =?utf-8?B?aFptcDJEYSsxZC9mTUt3MXN4c0R6U29tK1JYMkc2cUlhSGQ4akk4c082Wkl6?=
 =?utf-8?B?ZXJLdzh1c0xmbmVXRTVHTXkrNG5QVnllWGZlRm5IUWdRR1lTcUl1QmNPK2xZ?=
 =?utf-8?B?Kzd6ckQrZGpodlJXM29WZUZuOThxaFRmT1NBVnZBaDhYODFmOVdlMkdKOEUw?=
 =?utf-8?B?eXVWYTJkV20wbGZYdHExSEJ4dE5mcmFjV2JOZERqanFHVURpVm5jeStlOCtP?=
 =?utf-8?B?ZkFhdDJNUjFuZEVmZXBLb2QyWHlKUmJsdEhHdTRWVVlLSnVDb2ZoRHpIQUY0?=
 =?utf-8?B?MXM5ZDMvNytnS0Jzak9Fam94SWU3dXZKdUZIY2dyYUVURE9keGxnVEdrM0x5?=
 =?utf-8?B?eVBWRW5weTl6VFVpdGpkRGIzMHcvZWtzMlZuTmZ6ZmxYWUFGRlV6Q0wvQTBF?=
 =?utf-8?B?cldYWUJUQlBvOEFiclJHZHdjbmdnQTdIMFNvQm03RzNUbFlIY2lQTWdJMGJR?=
 =?utf-8?B?S1R0T0RJNkQwUXNoeGxSTlhRSGd5Q0xDczRRS3IweUw0U0JHTG5hSDdUbnhZ?=
 =?utf-8?B?ZjZNcVhza3BTYnJhSzIyNmorNmhOUWFXRTlWclZsVUQ0UWFEUWw0c2xueFBY?=
 =?utf-8?B?QTErL0JoV1BpUURjUzNuYkwycGlTb1Buckhxc3dheWljQUFsUlFnZG1ObXlB?=
 =?utf-8?B?MU5GRXRobU0zeGNtWk5tWDNhempYZDg4amx2VGp6aCtTSlpWcXo2NldwUHJQ?=
 =?utf-8?B?YjhTbjVaVFhESkNXdk5LcDZVY2U5WFQwMG45dnpDWGl5ZnlXQkE3SXI2Ulhy?=
 =?utf-8?B?d0pEWDFBN0swZzVidC9yMDFZYitWTWorTkJxOTVjSUhldVg4ZjA4clMyZmcr?=
 =?utf-8?B?WWlCUTdzbWVWa3daS2lCQ2NNWVdCcWJBTFRwZkx2VzBhVjJKZXJyTGJVRENl?=
 =?utf-8?B?OFNnV2g1cjRvT2hBdE4yeWhTNTUzQnZiUDJMb0ZjMzZGdWkrR2FSNU8zbkRC?=
 =?utf-8?B?cFNVNVp0ZTdwTFhlRjNIVFYxMGVDUmlaRGdLb0ZYQ2VVYjNtWHJLaW8rK05Q?=
 =?utf-8?B?SGlCRm44bTJVZTNZbEJMMitUVDE5NWVCbnJ0SmhET25kRU1KNXRaaUc3cG5x?=
 =?utf-8?B?cEl5b2JNbk0rVHJHa09WenBWYVdnb3FMTGZ3ZXdUQzJYT1hCMklSdFhybjl0?=
 =?utf-8?B?RlNSZXJkSG1tbmFGUHlEbXFVdnRpYU8rVWJGckVtZ2xxcUFtNjR3cmtMUkx1?=
 =?utf-8?B?OHVDMHZpTFdZMFVsVTdnbGdqSG5OcEZzVEtpeDBpaW5KS2daZkMwR2RORzRy?=
 =?utf-8?B?TVprZkkxb3Q1QWpnU0I0VnI1eXE2QXJQaE4yME5uNEdnVE1jbnVKV0xBYTRN?=
 =?utf-8?B?Uk45eERSZW1LQ0VHWkVxVXBNVTRYYVVtVitvV05pbGJ6TngxVG5teVFodkZv?=
 =?utf-8?B?emVxbEVBVFdJUDJVZ0dpUGtnUGhONkRoSHRTaWUrbEo5VnpCTHFES1ZXOGhy?=
 =?utf-8?B?dWdVZWVFa0lNTlNsWEZ2MlRLK3VzWnVMcHBpM05hSTBiQkp0cFF5WEFJMDQ3?=
 =?utf-8?B?L2FVUHVveVQ0ZS9YcGtPaVNjaWNSN1VoSmltcndwdnBMc284OXdzWFJLVU02?=
 =?utf-8?B?SmsybDZ4bWFkb0ZjK2NMeVVqUG1LWUJ4U01Na2p6K1gwdnhNTHExcGQ2bHEz?=
 =?utf-8?B?cDNEdWtMSlIvSEpMd0hYNU1lSlhvREZtRi9EOVRCRitPZ3c1RWFCbjUxTENv?=
 =?utf-8?B?ZkZPbFdtbTh6M2FaZWtnVHJlMWREMWdTY0pvTkVOanBUZnl0YmZPSVFjaUc1?=
 =?utf-8?B?VVRkS0picG1HVWd4dW9rbGRKYldwL2ZqZmhwQTl3a0xvd0hmLzVhdEwxYW5M?=
 =?utf-8?B?VjhsRmZIQXdFTnZwSFJGYzR5OWQ4WHhzVElwb3dYayswMmFkbTcwbk1ISUdZ?=
 =?utf-8?B?TkFoa2dvRWExMmVUZWlVenV5NEpkR2RUSmUzWFRaZ2w2ejN5YUpTZ2l1UU1C?=
 =?utf-8?B?d21vS0lCNE1BPT0=?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ed53af9d-3dae-4d31-15fb-08da0acf2df3
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4468.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Mar 2022 00:10:17.3634
 (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: Yjyd7e+LlcbCXU4dd6X/cH5jJ6APJZXOAdmrVxnvB2q+fDHynbk9R6iKtInGcGd3vRCe2Ru5gkzn/cff56UA4Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB4379
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10292 signatures=694221
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 phishscore=0
 mlxlogscore=999 adultscore=0 suspectscore=0 malwarescore=0 spamscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2203200170
X-Proofpoint-GUID: IQ_pAcb8x5BdUaWthu072JAzkOaVJrpr
X-Proofpoint-ORIG-GUID: IQ_pAcb8x5BdUaWthu072JAzkOaVJrpr
X-Spam-Status: No, score=-2.0 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=no 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

Hello Eric,

Thanks for reviewing this.

On 19/3/22 4:10 am, Eric W. Biederman wrote:
> Imran Khan <imran.f.khan@oracle.com> writes:
> 
[...]
>>  static DEFINE_SPINLOCK(kernfs_open_node_lock);
>> -static DEFINE_MUTEX(kernfs_open_file_mutex);
>> +DEFINE_MUTEX(kernfs_open_file_mutex);
> ^^^^^????
> 
> Why when you want things more localized are you making a lock more
> global?

The idea was to keep the interface in kernfs-internal.h. But since this
mutex is used only in kernfs/file.c, I can keep the mutex static and
bring interfaces that access this mutex in kernfs/file.c. This will
cover your other query as well.

>> diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
>> index eeaa779b929c..df00a5f5a367 100644
>> --- a/fs/kernfs/kernfs-internal.h
>> +++ b/fs/kernfs/kernfs-internal.h
>> @@ -164,4 +164,22 @@ void kernfs_drain_open_files(struct kernfs_node *kn);
>>   */
>>  extern const struct inode_operations kernfs_symlink_iops;
>>  
>> +extern struct mutex kernfs_open_file_mutex;
>> +
>> +static inline struct mutex *kernfs_open_file_mutex_ptr(struct kernfs_node *kn)
>> +{
>> +	return &kernfs_open_file_mutex;
>> +}
>> +
>> +static inline struct mutex *kernfs_open_file_mutex_lock(struct kernfs_node *kn)
>> +{
>> +	struct mutex *lock;
>> +
>> +	lock = kernfs_open_file_mutex_ptr(kn);
>> +
>> +	mutex_lock(lock);
>> +
>> +	return lock;
>> +}
> 
> Why don't these functions live in fs/kern/fs/file.c
> 

I can put these in kernfs/file.c. Please see earlier comment.

Thanks,
 -- Imran