[PATCH 0/13] sysfs lazification.

Tejun Heo <tj.RemoveThis@kernel.org> writes:

> Hello,
>
> Eric W. Biederman wrote:
>> NFS does exactly the same thing I am doing.
>
> Oh... well, sysfs directories parenting other filesystems are pretty
> rare and well defined. Although it's not very pretty, I don't think
> we'll see any actual problem there. Thanks for the explanation.

To be perfectly clear, if we hit this ugly case. The internal
sysfs_dirent tree is always what it should be. Only the VFS dentry
cache doesn't get updated.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On Fri, 06 Nov 2009, ebiederm.RemoveThis@xmission.com (Eric W. Biederman wrote:
> Tejun Heo <tj.RemoveThis@kernel.org> writes:
>
> > Hello,
> >
> > Eric W. Biederman wrote:
> >> It isn't what I want but it is what the VFS requires. If let the vfs
> >> continue on it's delusional state we will leak the vfs mount and
> >> everything mounted on top of it, with no way to remove the mounts.

"umount -l" on the whole thing will clear any submounts up too.

> >
> > This is caused by not having any way to prevent deletion on
> > directories with submounts, right? How does other distributed
> > filesystems deal with directories with submounts going away underneath
> > it?
>
> NFS does exactly the same thing I am doing.

Yes, this is a problem for NFS too. You cannot tell the NFS server
"this directory is mounted on some client, don't let anything happen
to it!". Basically the remaining choices are:

a) let the old path leading up to the mount still be accessible, even
though it doesn't exist anymore on the server (or has been replaced
with something different)

b) automatically dissolve any submounts if the path disappeard on the
server

I think Al was arguing in favor of b), while Linus said that mounts
must never just disappear, so a) is better. I don't think an
agreement was reached.

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Miklos Szeredi <miklos DeleteThis @szeredi.hu> writes:

> On Fri, 06 Nov 2009, ebiederm DeleteThis @xmission.com (Eric W. Biederman wrote:
>> Tejun Heo <tj DeleteThis @kernel.org> writes:
>>
>> > Hello,
>> >
>> > Eric W. Biederman wrote:
>> >> It isn't what I want but it is what the VFS requires. If let the vfs
>> >> continue on it's delusional state we will leak the vfs mount and
>> >> everything mounted on top of it, with no way to remove the mounts.
>
> "umount -l" on the whole thing will clear any submounts up too.
>
>> >
>> > This is caused by not having any way to prevent deletion on
>> > directories with submounts, right? How does other distributed
>> > filesystems deal with directories with submounts going away underneath
>> > it?
>>
>> NFS does exactly the same thing I am doing.
>
> Yes, this is a problem for NFS too. You cannot tell the NFS server
> "this directory is mounted on some client, don't let anything happen
> to it!". Basically the remaining choices are:
>
> a) let the old path leading up to the mount still be accessible, even
> though it doesn't exist anymore on the server (or has been replaced
> with something different)
>
> b) automatically dissolve any submounts if the path disappeard on the
> server
>
> I think Al was arguing in favor of b), while Linus said that mounts
> must never just disappear, so a) is better. I don't think an
> agreement was reached.

I haven't seen that conversation. I do know it is non-intutive and if
you attempt to delete what is a mount point in another mount namespace
and it won't go away. (What we do for non-distributed filesystems).
So I would favor mount points dissolving if we had the infrastructure.

Regardless the goal for now is to simply catch up with other distributed
filesystems.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo DeleteThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Tejun Heo <tj RemoveThis @kernel.org> writes:

> Eric W. Biederman wrote:
>> diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
>> index 1137418..c5eff49 100644
>> --- a/fs/sysfs/symlink.c
>> +++ b/fs/sysfs/symlink.c
>> @@ -214,6 +214,9 @@ const struct inode_operations sysfs_symlink_inode_operations = {
>> .readlink = generic_readlink,
>> .follow_link = sysfs_follow_link,
>> .put_link = sysfs_put_link,
>> + .setattr = sysfs_setattr,
>
> It would be nice either to separate addition of setattr to symlinks
> into a separate patch or note it in the description.

Good point note added.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Greg KH <greg.TakeThisOut@kroah.com> writes:

> On Tue, Nov 03, 2009 at 03:53:56AM -0800, Eric W. Biederman wrote:
>>
>> The sysfs code updates the vfs caches immediately when the sysfs data
>> structures change causing a lot of unnecessary complications. The
>> following patchset untangles that beast. Allowing for simpler
>> more straight forward code, the removal of a hack from the vfs
>> to support sysfs, and human comprehensible locking on sysfs.
>>
>> Most of these patches have already been reviewed and acked from the
>> last time I had time to work on sysfs.
>>
>> In net the patches look like:
>
> Can you resend these based on the review that you just got, with the new
> acks and changes so that I can apply them?

Coming in just a minute. The fixes are the trailing patches.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Tejun Heo <tj.RemoveThis@kernel.org> writes:

> Eric W. Biederman wrote:
>> In general everything that writes to vfs inodes holds the
>> inode mutex, so hold the inode mutex over sysfs_refresh_inode.
>> The sysfs data structures don't need this but it looks like the
>> vfs might.
>>
>> Signed-off-by: Eric W. Biederman <ebiederm.RemoveThis@aristanetworks.com>
>
> Acked-by: Tejun Heo <tj.RemoveThis@kernel.org>
>
> Sidenote: Hmmm... Originally, sysfs completely depended on vfs locking
> but with sysfs_dirent separation, the tree structure itself and some
> attributes went under the protection of sysfs_mutex while leaving more
> vfs oriented fields under vfs locking. This patchset makes sysfs
> lazier so it can't depend on any vfs layer locking. I think you've
> converted all necessary places while removing dependency on
> dentry/inode from update operations but it might be a good idea to do
> a audit pass over how fields are being protected now.

You raised a good point. I took a quick second pass through.
I did not see anything I have missed, and I did not change anything
else on the vfs path.

So at the very least I don't expect there are any locking related
regressions.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

The sysfs code updates the vfs caches immediately when the sysfs data
structures change causing a lot of unnecessary complications. The
following patchset untangles that beast. Allowing for simpler
more straight forward code, the removal of a hack from the vfs
to support sysfs, and human comprehensible locking on sysfs.

Most of these patches have already been reviewed and acked from the
last time I had time to work on sysfs.

This acks have been folded in and the two small bugs found in the
previous review have been fixed in the trailing patches (they are
minor enough nits that even a bisect that happens to land in the
middle should not see sysfs problems).

In net the patches look like:

fs/namei.c | 22 ---
fs/sysfs/dir.c | 388 ++++++++++++++++---------------------------------
fs/sysfs/file.c | 41 +----
fs/sysfs/inode.c | 178 ++++++++++++++---------
fs/sysfs/symlink.c | 11 +-
fs/sysfs/sysfs.h | 9 +-
include/linux/namei.h | 1 -
7 files changed, 256 insertions(+), 394 deletions(-)

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

From: Eric W. Biederman <ebiederm.RemoveThis@maxwell.aristanetworks.com>

The sysfs_mutex is required to ensure updates are and will remain
atomic with respect to other inode iattr updates, that do not happen
through the filesystem.

Acked-by: Serge Hallyn <serue.RemoveThis@us.ibm.com>
Acked-by: Tejun Heo <tj.RemoveThis@kernel.org>
Signed-off-by: Eric W. Biederman <ebiederm.RemoveThis@aristanetworks.com>
---
fs/sysfs/inode.c | 41 +++++++++++++++++++++++++++++------------
1 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index e28cecf..8a08250 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -122,23 +122,39 @@ int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
return error;
}

+static int sysfs_sd_setsecdata(struct sysfs_dirent *sd, void **secdata, u32 *secdata_len)
+{
+ struct sysfs_inode_attrs *iattrs;
+ void *old_secdata;
+ size_t old_secdata_len;
+
+ iattrs = sd->s_iattr;
+ if (!iattrs)
+ iattrs = sysfs_init_inode_attrs(sd);
+ if (!iattrs)
+ return -ENOMEM;
+
+ old_secdata = iattrs->ia_secdata;
+ old_secdata_len = iattrs->ia_secdata_len;
+
+ iattrs->ia_secdata = *secdata;
+ iattrs->ia_secdata_len = *secdata_len;
+
+ *secdata = old_secdata;
+ *secdata_len = old_secdata_len;
+ return 0;
+}
+
int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value,
size_t size, int flags)
{
struct sysfs_dirent *sd = dentry->d_fsdata;
- struct sysfs_inode_attrs *iattrs;
void *secdata;
int error;
u32 secdata_len = 0;

if (!sd)
return -EINVAL;
- if (!sd->s_iattr)
- sd->s_iattr = sysfs_init_inode_attrs(sd);
- if (!sd->s_iattr)
- return -ENOMEM;
-
- iattrs = sd->s_iattr;

if (!strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN)) {
const char *suffix = name + XATTR_SECURITY_PREFIX_LEN;
@@ -150,12 +166,13 @@ int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value,
&secdata, &secdata_len);
if (error)
goto out;
- if (iattrs->ia_secdata)
- security_release_secctx(iattrs->ia_secdata,
- iattrs->ia_secdata_len);
- iattrs->ia_secdata = secdata;
- iattrs->ia_secdata_len = secdata_len;

+ mutex_lock(&sysfs_mutex);
+ error = sysfs_sd_setsecdata(sd, &secdata, &secdata_len);
+ mutex_unlock(&sysfs_mutex);
+
+ if (secdata)
+ security_release_secctx(secdata, secdata_len);
} else
return -EINVAL;
out:
--
1.6.5.2.143.g8cc62

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

From: Eric W. Biederman <ebiederm.DeleteThis@xmission.com>

Currently sysfs updates the timestamps on the vfs directory
inode when we create or remove a directory entry but doesn't
update the cached copy on the sysfs_dirent, fix that oversight.

Acked-by: Tejun Heo <tj.DeleteThis@kernel.org>
Acked-by: Serge Hallyn <serue.DeleteThis@us.ibm.com>
Signed-off-by: Eric W. Biederman <ebiederm.DeleteThis@aristanetworks.com>
---
fs/sysfs/dir.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index b5e8499..fa37126 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -464,6 +464,8 @@ void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt,
*/
int __sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
{
+ struct sysfs_inode_attrs *ps_iattr;
+
if (sysfs_find_dirent(acxt->parent_sd, sd->s_name))
return -EEXIST;

@@ -476,6 +478,13 @@ int __sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)

sysfs_link_sibling(sd);

+ /* Update timestamps on the parent */
+ ps_iattr = acxt->parent_sd->s_iattr;
+ if (ps_iattr) {
+ struct iattr *ps_iattrs = &ps_iattr->ia_iattr;
+ ps_iattrs->ia_ctime = ps_iattrs->ia_mtime = CURRENT_TIME;
+ }
+
return 0;
}

@@ -554,10 +563,19 @@ int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
*/
void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
{
+ struct sysfs_inode_attrs *ps_iattr;
+
BUG_ON(sd->s_flags & SYSFS_FLAG_REMOVED);

sysfs_unlink_sibling(sd);

+ /* Update timestamps on the parent */
+ ps_iattr = acxt->parent_sd->s_iattr;
+ if (ps_iattr) {
+ struct iattr *ps_iattrs = &ps_iattr->ia_iattr;
+ ps_iattrs->ia_ctime = ps_iattrs->ia_mtime = CURRENT_TIME;
+ }
+
sd->s_flags |= SYSFS_FLAG_REMOVED;
sd->s_sibling = acxt->removed;
acxt->removed = sd;
--
1.6.5.2.143.g8cc62

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

From: Eric W. Biederman <ebiederm.TakeThisOut@xmission.com>

Calling d_drop unconditionally when a sysfs_dirent is deleted has
the potential to leak mounts, so instead implement dentry delete
and revalidate operations that cause sysfs dentries to be removed
at the appropriate time.

Acked-by: Tejun Heo <tj.TakeThisOut@kernel.org>
Acked-by: Serge Hallyn <serue.TakeThisOut@us.ibm.com>
Signed-off-by: Eric W. Biederman <ebiederm.TakeThisOut@aristanetworks.com>
---
fs/sysfs/dir.c | 73 +++++++++++++++++++++++++++++++++++--------------------
1 files changed, 46 insertions(+), 27 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 130dfc3..b5e8499 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -298,6 +298,46 @@ void release_sysfs_dirent(struct sysfs_dirent * sd)
goto repeat;
}

+static int sysfs_dentry_delete(struct dentry *dentry)
+{
+ struct sysfs_dirent *sd = dentry->d_fsdata;
+ return !!(sd->s_flags & SYSFS_FLAG_REMOVED);
+}
+
+static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd)
+{
+ struct sysfs_dirent *sd = dentry->d_fsdata;
+ int is_dir;
+
+ mutex_lock(&sysfs_mutex);
+
+ /* The sysfs dirent has been deleted */
+ if (sd->s_flags & SYSFS_FLAG_REMOVED)
+ goto out_bad;
+
+ mutex_unlock(&sysfs_mutex);
+out_valid:
+ return 1;
+out_bad:
+ /* Remove the dentry from the dcache hashes.
+ * If this is a deleted dentry we use d_drop instead of d_delete
+ * so sysfs doesn't need to cope with negative dentries.
+ */
+ is_dir = (sysfs_type(sd) == SYSFS_DIR);
+ mutex_unlock(&sysfs_mutex);
+ if (is_dir) {
+ /* If we have submounts we must allow the vfs caches
+ * to lie about the state of the filesystem to prevent
+ * leaks and other nasty things.
+ */
+ if (have_submounts(dentry))
+ goto out_valid;
+ shrink_dcache_parent(dentry);
+ }
+ d_drop(dentry);
+ return 0;
+}
+
static void sysfs_dentry_iput(struct dentry * dentry, struct inode * inode)
{
struct sysfs_dirent * sd = dentry->d_fsdata;
@@ -307,6 +347,8 @@ static void sysfs_dentry_iput(struct dentry * dentry, struct inode * inode)
}

static const struct dentry_operations sysfs_dentry_ops = {
+ .d_revalidate = sysfs_dentry_revalidate,
+ .d_delete = sysfs_dentry_delete,
.d_iput = sysfs_dentry_iput,
};

@@ -527,44 +569,21 @@ void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
}

/**
- * sysfs_drop_dentry - drop dentry for the specified sysfs_dirent
+ * sysfs_dec_nlink - Decrement link count for the specified sysfs_dirent
* @sd: target sysfs_dirent
*
- * Drop dentry for @sd. @sd must have been unlinked from its
+ * Decrement nlink for @sd. @sd must have been unlinked from its
* parent on entry to this function such that it can't be looked
* up anymore.
*/
-static void sysfs_drop_dentry(struct sysfs_dirent *sd)
+static void sysfs_dec_nlink(struct sysfs_dirent *sd)
{
struct inode *inode;
- struct dentry *dentry;

inode = ilookup(sysfs_sb, sd->s_ino);
if (!inode)
return;

- /* Drop any existing dentries associated with sd.
- *
- * For the dentry to be properly freed we need to grab a
- * reference to the dentry under the dcache lock, unhash it,
- * and then put it. The playing with the dentry count allows
- * dput to immediately free the dentry if it is not in use.
- */
-repeat:
- spin_lock(&dcache_lock);
- list_for_each_entry(dentry, &inode->i_dentry, d_alias) {
- if (d_unhashed(dentry))
- continue;
- dget_locked(dentry);
- spin_lock(&dentry->d_lock);
- __d_drop(dentry);
- spin_unlock(&dentry->d_lock);
- spin_unlock(&dcache_lock);
- dput(dentry);
- goto repeat;
- }
- spin_unlock(&dcache_lock);
-
/* adjust nlink and update timestamp */
mutex_lock(&inode->i_mutex);

@@ -611,7 +630,7 @@ void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt)
acxt->removed = sd->s_sibling;
sd->s_sibling = NULL;

- sysfs_drop_dentry(sd);
+ sysfs_dec_nlink(sd);
sysfs_deactivate(sd);
unmap_bin_file(sd);
sysfs_put(sd);
--
1.6.5.2.143.g8cc62

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

From: Eric W. Biederman <ebiederm.TakeThisOut@xmission.com>

Now that sysfs_getattr and sysfs_permission refresh the vfs
inode there is no need to immediatly push the mode change
into the vfs cache. Reducing the amount of work needed and
simplifying the locking.

Acked-by: Tejun Heo <tj.TakeThisOut@kernel.org>
Acked-by: Serge Hallyn <serue.TakeThisOut@us.ibm.com>
Signed-off-by: Eric W. Biederman <ebiederm.TakeThisOut@aristanetworks.com>
---
fs/sysfs/file.c | 31 ++++++++-----------------------
1 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index faa1a80..dc30d9e 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -579,38 +579,23 @@ EXPORT_SYMBOL_GPL(sysfs_add_file_to_group);
*/
int sysfs_chmod_file(struct kobject *kobj, struct attribute *attr, mode_t mode)
{
- struct sysfs_dirent *victim_sd = NULL;
- struct dentry *victim = NULL;
- struct inode * inode;
+ struct sysfs_dirent *sd;
struct iattr newattrs;
int rc;

- rc = -ENOENT;
- victim_sd = sysfs_get_dirent(kobj->sd, attr->name);
- if (!victim_sd)
- goto out;
+ mutex_lock(&sysfs_mutex);

- mutex_lock(&sysfs_rename_mutex);
- victim = sysfs_get_dentry(victim_sd);
- mutex_unlock(&sysfs_rename_mutex);
- if (IS_ERR(victim)) {
- rc = PTR_ERR(victim);
- victim = NULL;
+ rc = -ENOENT;
+ sd = sysfs_find_dirent(kobj->sd, attr->name);
+ if (!sd)
goto out;
- }
-
- inode = victim->d_inode;

- mutex_lock(&inode->i_mutex);
-
- newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
+ newattrs.ia_mode = (mode & S_IALLUGO) | (sd->s_mode & ~S_IALLUGO);
newattrs.ia_valid = ATTR_MODE;
- rc = sysfs_setattr(victim, &newattrs);
+ rc = sysfs_sd_setattr(sd, &newattrs);

- mutex_unlock(&inode->i_mutex);
out:
- dput(victim);
- sysfs_put(victim_sd);
+ mutex_unlock(&sysfs_mutex);
return rc;
}
EXPORT_SYMBOL_GPL(sysfs_chmod_file);
--
1.6.5.2.143.g8cc62

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.TakeThisOut@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

From: Eric W. Biederman <ebiederm.DeleteThis@xmission.com>

The granularity of sysfs time when we keep it is 1 ns. Which
when passed to timestamp_trunc results in a nop. So remove
the unnecessary function call making sysfs_setattr slightly
easier to read.

Acked-by: Tejun Heo <tj.DeleteThis@kernel.org>
Acked-by: Serge Hallyn <serue.DeleteThis@us.ibm.com>
Signed-off-by: Eric W. Biederman <ebiederm.DeleteThis@aristanetworks.com>
---
fs/sysfs/inode.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 8a08250..fed7a74 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -103,14 +103,11 @@ int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
if (ia_valid & ATTR_GID)
iattrs->ia_gid = iattr->ia_gid;
if (ia_valid & ATTR_ATIME)
- iattrs->ia_atime = timespec_trunc(iattr->ia_atime,
- inode->i_sb->s_time_gran);
+ iattrs->ia_atime = iattr->ia_atime;
if (ia_valid & ATTR_MTIME)
- iattrs->ia_mtime = timespec_trunc(iattr->ia_mtime,
- inode->i_sb->s_time_gran);
+ iattrs->ia_mtime = iattr->ia_mtime;
if (ia_valid & ATTR_CTIME)
- iattrs->ia_ctime = timespec_trunc(iattr->ia_ctime,
- inode->i_sb->s_time_gran);
+ iattrs->ia_ctime = iattr->ia_ctime;
if (ia_valid & ATTR_MODE) {
umode_t mode = iattr->ia_mode;

--
1.6.5.2.143.g8cc62

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

From: Eric W. Biederman <ebiederm RemoveThis @xmission.com>

Cleanly separate the work that is specific to setting the
attributes of a sysfs_dirent from what is needed to update
the attributes of a vfs inode.

Additionally grab the sysfs_mutex to keep any nasties from
surprising us when updating the sysfs_dirent.

Acked-by: Tejun Heo <tj RemoveThis @kernel.org>
Signed-off-by: Eric W. Biederman <ebiederm RemoveThis @aristanetworks.com>
---
fs/sysfs/inode.c | 52 ++++++++++++++++++++++++++++++++--------------------
fs/sysfs/sysfs.h | 1 +
2 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index fed7a74..fccfb55 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -64,30 +64,15 @@ struct sysfs_inode_attrs *sysfs_init_inode_attrs(struct sysfs_dirent *sd)

return attrs;
}
-int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
+
+int sysfs_sd_setattr(struct sysfs_dirent *sd, struct iattr * iattr)
{
- struct inode * inode = dentry->d_inode;
- struct sysfs_dirent * sd = dentry->d_fsdata;
struct sysfs_inode_attrs *sd_attrs;
struct iattr *iattrs;
unsigned int ia_valid = iattr->ia_valid;
- int error;
-
- if (!sd)
- return -EINVAL;

sd_attrs = sd->s_iattr;

- error = inode_change_ok(inode, iattr);
- if (error)
- return error;
-
- iattr->ia_valid &= ~ATTR_SIZE; /* ignore size changes */
-
- error = inode_setattr(inode, iattr);
- if (error)
- return error;
-
if (!sd_attrs) {
/* setting attributes for the first time, allocate now */
sd_attrs = sysfs_init_inode_attrs(sd);
@@ -110,12 +95,39 @@ int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
iattrs->ia_ctime = iattr->ia_ctime;
if (ia_valid & ATTR_MODE) {
umode_t mode = iattr->ia_mode;
-
- if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
- mode &= ~S_ISGID;
iattrs->ia_mode = sd->s_mode = mode;
}
}
+ return 0;
+}
+
+int sysfs_setattr(struct dentry * dentry, struct iattr * iattr)
+{
+ struct inode * inode = dentry->d_inode;
+ struct sysfs_dirent * sd = dentry->d_fsdata;
+ int error;
+
+ if (!sd)
+ return -EINVAL;
+
+ error = inode_change_ok(inode, iattr);
+ if (error)
+ return error;
+
+ iattr->ia_valid &= ~ATTR_SIZE; /* ignore size changes */
+ if (iattr->ia_valid & ATTR_MODE) {
+ if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
+ iattr->ia_mode &= ~S_ISGID;
+ }
+
+ error = inode_setattr(inode, iattr);
+ if (error)
+ return error;
+
+ mutex_lock(&sysfs_mutex);
+ error = sysfs_sd_setattr(sd, iattr);
+ mutex_unlock(&sysfs_mutex);
+
return error;
}

diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index af4c4e7..a96d967 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -155,6 +155,7 @@ static inline void __sysfs_put(struct sysfs_dirent *sd)
*/
struct inode *sysfs_get_inode(struct sysfs_dirent *sd);
void sysfs_delete_inode(struct inode *inode);
+int sysfs_sd_setattr(struct sysfs_dirent *sd, struct iattr *iattr);
int sysfs_setattr(struct dentry *dentry, struct iattr *iattr);
int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value,
size_t size, int flags);
--
1.6.5.2.143.g8cc62

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo RemoveThis @vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/