Prev: 2.6.31-rc9 kernel BUG and mvsas
Next: [PATCH 3/3] function-graph: Allow writing the same val to set_graph_function
From: Serge E. Hallyn on 3 Nov 2009 23:00
Quoting Eric W. Biederman (ebiederm(a)xmission.com):
> From: Eric W. Biederman <ebiederm(a)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.

confused... why not do this in sysfs_addrm_finish()?

I guess you'd have to do at it at top before dropping sysfs_mutex
so it wouldn't be as pretty as I was thinking, but at least you
could just do it once.

>
> Acked-by: Tejun Heo <tj(a)kernel.org>
> Signed-off-by: Eric W. Biederman <ebiederm(a)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-fsdevel" in
> the body of a message to majordomo(a)vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)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 on 3 Nov 2009 23:20
"Serge E. Hallyn" <serue(a)us.ibm.com> writes:

> Quoting Eric W. Biederman (ebiederm(a)xmission.com):
>> From: Eric W. Biederman <ebiederm(a)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.
>
> confused... why not do this in sysfs_addrm_finish()?
>
> I guess you'd have to do at it at top before dropping sysfs_mutex
> so it wouldn't be as pretty as I was thinking, but at least you
> could just do it once.

Well sysfs_addrm_finish doesn't really know if you did anything.

Beyond that my ultimate goal is to kill sysfs_addrm_start and
sysfs_addrm_finish. Of course that requires fixing all of the
sysfs users that depend on the impossible to get right recursive
directory removal in sysfs, so it is not the subject of this patchset.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)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: Serge E. Hallyn on 4 Nov 2009 00:00
Quoting Eric W. Biederman (ebiederm(a)xmission.com):
> "Serge E. Hallyn" <serue(a)us.ibm.com> writes:
>
> > Quoting Eric W. Biederman (ebiederm(a)xmission.com):
> >> From: Eric W. Biederman <ebiederm(a)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.
> >
> > confused... why not do this in sysfs_addrm_finish()?
> >
> > I guess you'd have to do at it at top before dropping sysfs_mutex
> > so it wouldn't be as pretty as I was thinking, but at least you
> > could just do it once.
>
> Well sysfs_addrm_finish doesn't really know if you did anything.

Oh right - well it used to through cnt right? but not after your
last patch.

> Beyond that my ultimate goal is to kill sysfs_addrm_start and
> sysfs_addrm_finish. Of course that requires fixing all of the
> sysfs users that depend on the impossible to get right recursive
> directory removal in sysfs, so it is not the subject of this patchset.

I didn't see the patch nixing inode->i_mtime (and cnt) changing from
sysfs_addrm_finish() until after responding. Got it now.

Acked-by: Serge Hallyn <serue(a)us.ibm.com>

-serge
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: 2.6.31-rc9 kernel BUG and mvsas
Next: [PATCH 3/3] function-graph: Allow writing the same val to set_graph_function