Re: [PATCH 4/4] locks: map correct ino/dev pairs when exporting to userspace
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
> > ---
> > 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, , );
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] locks: map correct ino/dev pairs when exporting to userspace
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, , );
> > /* 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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] locks: map correct ino/dev pairs when exporting to userspace
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
> ---
> 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, , );
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 :0 ", fl_pid);
> }
--
Jeff Layton
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] locks: map correct ino/dev pairs when exporting to userspace
On Wed, Aug 1, 2018 at 12:10 AM, 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
> ---
> 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, , );
> /* 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] locks: map correct ino/dev pairs when exporting to userspace
/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
---
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, , );
/* 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 :0 ", fl_pid);
}
--
2.15.1
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
