On Tue, Apr 03, 2007 at 05:05:20PM +0200, Miklos Szeredi wrote:
> On Tue, 2007-04-03 at 18:21 +0400, Alexey Dobriyan wrote:
> > On Mon, Mar 26, 2007 at 11:35:42PM -0800, akpm@xxxxxxxxxxxxxxxxxxxx wrote:
> > > The patch titled
> > > add file position info to proc
> > > has been added to the -mm tree. Its filename is
> > > add-file-position-info-to-proc.patch
> >
> > I tried to stress-test it with the following program and script and
> > lockdep barfs on me reasonably quickly:
>
> Ugh. As I see it, this is independent from the above patch, as the same
> can happen with /proc/PID/fd. Or did I miss something?
Yeah, it's independent from /proc/*/fdinfo stuff.
> And it seems quite benign, one of the mutexes is for the proc inode, the
> other is for the pipe inode, and hopefully they haven't got anything
> else to do with each other.
>
> > It seems that lockdep is unhappy about ->i_mutex taken in
> > ->release/pipe_read_release()/pipe_read_fasync()
> > which is triggered from put_files_struct() in proc_readfd_common()
> >
> > Now checking if giving pipe's i_mutex its own lockdep class with fix
> > things.
>
> Should help. Although I'm wondering if it's worth bothering with, as it
> doesn't seem to happen in real life for lockdep users.
The following patch makes lockdep happy but I wonder if it's correct
way.
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -908,6 +908,8 @@ static struct dentry_operations pipefs_d
.d_dname = pipefs_dname,
};
+static struct lock_class_key pipe_inode_imutex_key;
+
static struct inode * get_pipe_inode(void)
{
struct inode *inode = new_inode(pipe_mnt->mnt_sb);
@@ -936,6 +938,8 @@ static struct inode * get_pipe_inode(voi
inode->i_gid = current->fsgid;
inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
+ lockdep_set_class(&inode->i_mutex, &pipe_inode_imutex_key);
+
return inode;
fail_iput:
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
