Re: [PATCH v2 2/9] mm/mshare: pre-populate msharefs with information file

[Khalid Aziz <khalid.aziz@oracle.com>]

On 6/30/22 15:37, Darrick J. Wong wrote:
> On Wed, Jun 29, 2022 at 04:53:53PM -0600, Khalid Aziz wrote:
>> Users of mshare feature to share page tables need to know the size
>> and alignment requirement for shared regions. Pre-populate msharefs
>> with a file, mshare_info, that provides this information.
>>
>> Signed-off-by: Khalid Aziz <khalid.aziz@oracle.com>
>> ---
>>   mm/mshare.c | 62 +++++++++++++++++++++++++++++++++++++++++------------
>>   1 file changed, 48 insertions(+), 14 deletions(-)
>>
>> diff --git a/mm/mshare.c b/mm/mshare.c
>> index c8fab3869bab..3e448e11c742 100644
>> --- a/mm/mshare.c
>> +++ b/mm/mshare.c
>> @@ -25,8 +25,8 @@
>>   static struct super_block *msharefs_sb;
>>   
>>   static const struct file_operations msharefs_file_operations = {
>> -	.open	= simple_open,
>> -	.llseek	= no_llseek,
>> +	.open		= simple_open,
>> +	.llseek		= no_llseek,
> 
> I feel like there's a lot of churn between the previous patch and this
> one that could have been in the previous patch.

I will look at what changes can be consolidated between two patches, including this change. Thanks for catching this.

> 
>>   };
>>   
>>   static int
>> @@ -42,23 +42,52 @@ msharefs_d_hash(const struct dentry *dentry, struct qstr *qstr)
>>   	return 0;
>>   }
>>   
>> +static void
>> +mshare_evict_inode(struct inode *inode)
>> +{
>> +	clear_inode(inode);
>> +}
>> +
>>   static const struct dentry_operations msharefs_d_ops = {
>>   	.d_hash = msharefs_d_hash,
>>   };
>>   
>> +static ssize_t
>> +mshare_info_read(struct file *file, char __user *buf, size_t nbytes,
>> +		loff_t *ppos)
>> +{
>> +	char s[80];
>> +
>> +	sprintf(s, "%ld", PGDIR_SIZE);
> 
> SO what is this "mshare_info" file supposed to reveal?  Hugepage size?
> I wonder why this isn't exported in struct mshare_info?

This file gives the alignment and size requirement for mshare regions which is the same for every mshare region. struct 
mshare_info is a per-mshare region structure that provides starting address and size for just that region.

--
Khalid

> 
>> +	return simple_read_from_buffer(buf, nbytes, ppos, s, strlen(s));
>> +}
>> +
>> +static const struct file_operations mshare_info_ops = {
>> +	.read   = mshare_info_read,
>> +	.llseek	= noop_llseek,
>> +};
>> +
>> +static const struct super_operations mshare_s_ops = {
>> +	.statfs	     = simple_statfs,
>> +	.evict_inode = mshare_evict_inode,
>> +};
>> +
>>   static int
>>   msharefs_fill_super(struct super_block *sb, struct fs_context *fc)
>>   {
>> -	static const struct tree_descr empty_descr = {""};
>> +	static const struct tree_descr mshare_files[] = {
>> +		[2] = { "mshare_info", &mshare_info_ops, 0444},
>> +		{""},
>> +	};
>>   	int err;
>>   
>> -	sb->s_d_op = &msharefs_d_ops;
>> -	err = simple_fill_super(sb, MSHARE_MAGIC, &empty_descr);
>> -	if (err)
>> -		return err;
>> -
>> -	msharefs_sb = sb;
>> -	return 0;
>> +	err = simple_fill_super(sb, MSHARE_MAGIC, mshare_files);
>> +	if (!err) {
>> +		msharefs_sb = sb;
>> +		sb->s_d_op = &msharefs_d_ops;
>> +		sb->s_op = &mshare_s_ops;
>> +	}
>> +	return err;
>>   }
>>   
>>   static int
>> @@ -84,20 +113,25 @@ static struct file_system_type mshare_fs = {
>>   	.kill_sb		= kill_litter_super,
>>   };
>>   
>> -static int
>> +static int __init
>>   mshare_init(void)
>>   {
>>   	int ret = 0;
>>   
>>   	ret = sysfs_create_mount_point(fs_kobj, "mshare");
>>   	if (ret)
>> -		return ret;
>> +		goto out;
>>   
>>   	ret = register_filesystem(&mshare_fs);
>> -	if (ret)
>> +	if (ret) {
>>   		sysfs_remove_mount_point(fs_kobj, "mshare");
>> +		goto out;
>> +	}
>> +
>> +	return 0;
>>   
>> +out:
>>   	return ret;
>>   }
>>   
>> -fs_initcall(mshare_init);
>> +core_initcall(mshare_init);
>> -- 
>> 2.32.0
>>