From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: [patch 01/27] fs: cleanup files_lock Date: Sun, 26 Apr 2009 08:12:27 +0200 Message-ID: <20090426061227.GA28555@wotan.suse.de> References: <20090425012020.457460929@suse.de> <20090425012209.061706790@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Al Viro , Alan Cox To: "Eric W. Biederman" Return-path: Received: from cantor.suse.de ([195.135.220.2]:42594 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750845AbZDZGM3 (ORCPT ); Sun, 26 Apr 2009 02:12:29 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Apr 24, 2009 at 10:35:10PM -0700, Eric W. Biederman wrote: > npiggin@suse.de writes: > > > Lock tty_files with tty_mutex, provide helpers to manipulate the per-sb > > files list, and unexport the files_lock spinlock. > > This conflicts a bit with some of my ongoing work, which is generalizing > the file list to make it more useful and makes the tty case much less > of a special case. OK. My first patch should be fine, though. > Do you know if the performance improvement would be anywhere near as good if > file_list and file_list_lock becoming per inode? Interesting (I didn't look closely at your patches yet). Probably that would be quite reasonable. > Do you have any idea what the performance improvement with changing the file_list_lock > is? Several of these locks hit in the same workloads so they mask each other. I only just got the patchset to the stage where I can really benchmark it. I could try your alternative as well. > > Index: linux-2.6/fs/open.c > > =================================================================== > > --- linux-2.6.orig/fs/open.c > > +++ linux-2.6/fs/open.c > > @@ -828,7 +828,7 @@ static struct file *__dentry_open(struct > > f->f_path.mnt = mnt; > > f->f_pos = 0; > > f->f_op = fops_get(inode->i_fop); > > - file_move(f, &inode->i_sb->s_files); > > + file_sb_list_add(f, inode->i_sb); > > You can make this just: > if (!special_file(inode->i_mode)) > file_add(f, &inode->i_files); > > And save yourself a lot of complexity. Probably right, but I'll leave that for someone else to do.