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

Khalid Aziz
2022-06-30T22:55:20Z

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
>>