| Message ID | 20180731211045.5671-5-mfasheh@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfs: map unique ino/dev pairs for user space | expand |
On Wed, Aug 1, 2018 at 12:10 AM, Mark Fasheh <mfasheh@suse.de> wrote: > /proc/locks does not always print the correct inode number/device pair. > Update lock_get_status() to use vfs_map_unique_ino_dev() to get the real, > unique values for userspace. > > Signed-off-by: Mark Fasheh <mfasheh@suse.de> > --- > fs/locks.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/fs/locks.c b/fs/locks.c > index db7b6917d9c5..3a012df87fd8 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -2621,6 +2621,7 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, > loff_t id, char *pfx) > { > struct inode *inode = NULL; > + struct dentry *dentry; > unsigned int fl_pid; > struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info; > > @@ -2633,8 +2634,10 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, > if (fl_pid == 0) > return; > > - if (fl->fl_file != NULL) > + if (fl->fl_file != NULL) { > inode = locks_inode(fl->fl_file); > + dentry = file_dentry(fl->fl_file); > + } > > seq_printf(f, "%lld:%s ", id, pfx); > if (IS_POSIX(fl)) { > @@ -2681,10 +2684,13 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, > : (fl->fl_type == F_WRLCK) ? "WRITE" : "READ "); > } > if (inode) { > + __u64 ino; > + dev_t dev; > + > + vfs_map_unique_ino_dev(dentry, &ino, &dev); > /* userspace relies on this representation of dev_t */ > seq_printf(f, "%d %02x:%02x:%ld ", fl_pid, > - MAJOR(inode->i_sb->s_dev), > - MINOR(inode->i_sb->s_dev), inode->i_ino); > + MAJOR(dev), MINOR(dev), inode->i_ino); Don't you mean ,ino); ? Thanks, Amir.
On Tue, 2018-07-31 at 14:10 -0700, Mark Fasheh wrote: > /proc/locks does not always print the correct inode number/device pair. > Update lock_get_status() to use vfs_map_unique_ino_dev() to get the real, > unique values for userspace. > > Signed-off-by: Mark Fasheh <mfasheh@suse.de> > --- > fs/locks.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/fs/locks.c b/fs/locks.c > index db7b6917d9c5..3a012df87fd8 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -2621,6 +2621,7 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, > loff_t id, char *pfx) > { > struct inode *inode = NULL; > + struct dentry *dentry; > unsigned int fl_pid; > struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info; > > @@ -2633,8 +2634,10 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, > if (fl_pid == 0) > return; > > - if (fl->fl_file != NULL) > + if (fl->fl_file != NULL) { > inode = locks_inode(fl->fl_file); > + dentry = file_dentry(fl->fl_file); > + } > > seq_printf(f, "%lld:%s ", id, pfx); > if (IS_POSIX(fl)) { > @@ -2681,10 +2684,13 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, > : (fl->fl_type == F_WRLCK) ? "WRITE" : "READ "); > } > if (inode) { > + __u64 ino; > + dev_t dev; > + > + vfs_map_unique_ino_dev(dentry, &ino, &dev); This code is under a spinlock (blocked_locks_lock or ctx->flc_lock). I don't think it'll be ok to call ->getattr while holding a spinlock. > /* userspace relies on this representation of dev_t */ > seq_printf(f, "%d %02x:%02x:%ld ", fl_pid, > - MAJOR(inode->i_sb->s_dev), > - MINOR(inode->i_sb->s_dev), inode->i_ino); > + MAJOR(dev), MINOR(dev), inode->i_ino); > } else { > seq_printf(f, "%d <none>:0 ", fl_pid); > }
On Wed, Aug 01, 2018 at 08:37:31AM +0300, Amir Goldstein wrote: > > if (inode) { > > + __u64 ino; > > + dev_t dev; > > + > > + vfs_map_unique_ino_dev(dentry, &ino, &dev); > > /* userspace relies on this representation of dev_t */ > > seq_printf(f, "%d %02x:%02x:%ld ", fl_pid, > > - MAJOR(inode->i_sb->s_dev), > > - MINOR(inode->i_sb->s_dev), inode->i_ino); > > + MAJOR(dev), MINOR(dev), inode->i_ino); > > Don't you mean ,ino); ? Indeed I do, thanks for catching that one Amir. --Mark
Hi Jeff, On Wed, Aug 01, 2018 at 07:03:54AM -0400, Jeff Layton wrote: > On Tue, 2018-07-31 at 14:10 -0700, Mark Fasheh wrote: > > /proc/locks does not always print the correct inode number/device pair. > > Update lock_get_status() to use vfs_map_unique_ino_dev() to get the real, > > unique values for userspace. > > > > Signed-off-by: Mark Fasheh <mfasheh@suse.de> > > --- > > fs/locks.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/fs/locks.c b/fs/locks.c > > index db7b6917d9c5..3a012df87fd8 100644 > > --- a/fs/locks.c > > +++ b/fs/locks.c > > @@ -2621,6 +2621,7 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, > > loff_t id, char *pfx) > > { > > struct inode *inode = NULL; > > + struct dentry *dentry; > > unsigned int fl_pid; > > struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info; > > > > @@ -2633,8 +2634,10 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, > > if (fl_pid == 0) > > return; > > > > - if (fl->fl_file != NULL) > > + if (fl->fl_file != NULL) { > > inode = locks_inode(fl->fl_file); > > + dentry = file_dentry(fl->fl_file); > > + } > > > > seq_printf(f, "%lld:%s ", id, pfx); > > if (IS_POSIX(fl)) { > > @@ -2681,10 +2684,13 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, > > : (fl->fl_type == F_WRLCK) ? "WRITE" : "READ "); > > } > > if (inode) { > > + __u64 ino; > > + dev_t dev; > > + > > + vfs_map_unique_ino_dev(dentry, &ino, &dev); > > This code is under a spinlock (blocked_locks_lock or ctx->flc_lock). I > don't think it'll be ok to call ->getattr while holding a spinlock. Hmm, indeed it does call under spinlock. I'll hve to rethink how to approach this in fs/locks.c Thanks, --Mark
diff --git a/fs/locks.c b/fs/locks.c index db7b6917d9c5..3a012df87fd8 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -2621,6 +2621,7 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, loff_t id, char *pfx) { struct inode *inode = NULL; + struct dentry *dentry; unsigned int fl_pid; struct pid_namespace *proc_pidns = file_inode(f->file)->i_sb->s_fs_info; @@ -2633,8 +2634,10 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, if (fl_pid == 0) return; - if (fl->fl_file != NULL) + if (fl->fl_file != NULL) { inode = locks_inode(fl->fl_file); + dentry = file_dentry(fl->fl_file); + } seq_printf(f, "%lld:%s ", id, pfx); if (IS_POSIX(fl)) { @@ -2681,10 +2684,13 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, : (fl->fl_type == F_WRLCK) ? "WRITE" : "READ "); } if (inode) { + __u64 ino; + dev_t dev; + + vfs_map_unique_ino_dev(dentry, &ino, &dev); /* userspace relies on this representation of dev_t */ seq_printf(f, "%d %02x:%02x:%ld ", fl_pid, - MAJOR(inode->i_sb->s_dev), - MINOR(inode->i_sb->s_dev), inode->i_ino); + MAJOR(dev), MINOR(dev), inode->i_ino); } else { seq_printf(f, "%d <none>:0 ", fl_pid); }
/proc/locks does not always print the correct inode number/device pair. Update lock_get_status() to use vfs_map_unique_ino_dev() to get the real, unique values for userspace. Signed-off-by: Mark Fasheh <mfasheh@suse.de> --- fs/locks.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)