From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Serge E. Hallyn" Subject: Re: [PATCH 09/13] sysfs: Implement sysfs_getattr & sysfs_permission Date: Wed, 4 Nov 2009 08:24:18 -0600 Message-ID: <20091104142418.GB4355@us.ibm.com> References: <1257249429-12384-9-git-send-email-ebiederm@xmission.com> <20091104042038.GE27639@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Greg Kroah-Hartman , Kay Sievers , Greg KH , linux-kernel@vger.kernel.org, Tejun Heo , Cornelia Huck , linux-fsdevel@vger.kernel.org, Eric Dumazet , Benjamin LaHaise , "Eric W. Biederman" To: "Eric W. Biederman" Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org Quoting Eric W. Biederman (ebiederm@xmission.com): > "Serge E. Hallyn" writes: > > > So the inode->i_mutex is not needed? > > Good question. Nothing in sysfs needs it. The VFS does not grab the > inode mutex on this path, but the vfs does grab the inode mutex when > writing to the inode. All callers of fs/attr.c:notify_change() do seem to take the i_mutex, though. And Documentation/filesystem/Locking claims that ->setattr() does need i_mutex. So I assume that setting of inode->i_ctime etc, which is what you're doing here, needs to be protected by the i_mutex. > Since the VFs isn't grabbing the inode_mutex there is probably a race in > here somewhere if someone looks at things just right. > > I am too tired tonight to be that person. The readers take no lock of any sort (i.e. generic_fillattr and its callers) so IIUC they could get inconsistent data... -serge