| Message ID | 34e2eabbef5916c784dc16856ce25b3967f9b405.1656531090.git.khalid.aziz@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for shared PTEs across processes | expand |
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. > }; > > 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? > + 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 >
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 >>
On Wed, Jun 29, 2022 at 04:53:53PM -0600, Khalid Aziz wrote: > +static void > +mshare_evict_inode(struct inode *inode) > +{ > + clear_inode(inode); > +} Again, what for? And while we are at it, shouldn't you evict the pages when inode gets freed and ->i_data along with it? IOW, aren't you missing truncate_inode_pages_final(&inode->i_data); That, or just leave ->evict_inode NULL...
On 6/30/22 17:01, Al Viro wrote: > On Wed, Jun 29, 2022 at 04:53:53PM -0600, Khalid Aziz wrote: > >> +static void >> +mshare_evict_inode(struct inode *inode) >> +{ >> + clear_inode(inode); >> +} > > Again, what for? And while we are at it, shouldn't you evict the > pages when inode gets freed and ->i_data along with it? > IOW, aren't you missing > truncate_inode_pages_final(&inode->i_data); > That, or just leave ->evict_inode NULL... Good suggestion. I thought I would need to do more in evict_inode() but turned out I didn't. It is time to eject this routine from my code. Thanks, Khalid
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, }; 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); + 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);
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(-)