2006-09-16 07:51:08

by Mathieu Desnoyers

[permalink] [raw]
Subject: [PATCH 1/11] LTTng-core 0.5.111 : Relay+DebugFS (DebugFS fix)

1 - DebugFS stalled dentry patch
DebugFS seems to keep a stalled dentry when a process is in a directory that is
being removed. Force a differed deletion.
patch-2.6.17-lttng-core-0.5.111-debugfs.diff


OpenPGP public key: http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68


Attachments:
(No filename) (355.00 B)
patch-2.6.17-lttng-core-0.5.111-debugfs.diff (644.00 B)
Download all attachments

2006-09-18 01:00:27

by Karim Yaghmour

[permalink] [raw]
Subject: Re: [PATCH 1/11] LTTng-core 0.5.111 : Relay+DebugFS (DebugFS fix)


Mathieu Desnoyers wrote:
> 1 - DebugFS stalled dentry patch

I think it would be best if patches posted were in the body of
the email instead of as attachments. It makes it easier to
review and comment.

Thanks,

Karim
--
President / Opersys Inc.
Embedded Linux Training and Expertise
http://www.opersys.com / 1.866.677.4546

2006-09-18 12:09:32

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 1/11] LTTng-core 0.5.111 : Relay+DebugFS (DebugFS fix)

On Sat, Sep 16, 2006 at 03:51:03AM -0400, Mathieu Desnoyers wrote:
> 1 - DebugFS stalled dentry patch
> DebugFS seems to keep a stalled dentry when a process is in a directory that is
> being removed. Force a differed deletion.
> patch-2.6.17-lttng-core-0.5.111-debugfs.diff
>
>
> OpenPGP public key: http://krystal.dyndns.org:8080/key/compudj.gpg
> Key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

> --- a/fs/debugfs/inode.c
> +++ b/fs/debugfs/inode.c
> @@ -266,6 +266,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_dir);
> void debugfs_remove(struct dentry *dentry)
> {
> struct dentry *parent;
> + int ret = 0;
>
> if (!dentry)
> return;
> @@ -278,9 +279,10 @@ void debugfs_remove(struct dentry *dentr
> if (debugfs_positive(dentry)) {
> if (dentry->d_inode) {
> if (S_ISDIR(dentry->d_inode->i_mode))
> - simple_rmdir(parent->d_inode, dentry);
> + ret = simple_rmdir(parent->d_inode, dentry);
> else
> - simple_unlink(parent->d_inode, dentry);
> + ret = simple_unlink(parent->d_inode, dentry);
> + if(ret) d_delete(dentry);

Are you saying that perhaps all other users of simple_unlink() are also
broken like this? If so, why not just fix simple_unlink()?

Also, wrong coding style used :(

thanks,

greg k-h

2006-09-19 13:04:07

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/11] LTTng-core 0.5.111 : Relay+DebugFS (DebugFS fix)

On Sun, Sep 17, 2006 at 09:21:15PM -0400, Karim Yaghmour wrote:
>
> Mathieu Desnoyers wrote:
> > 1 - DebugFS stalled dentry patch
>
> I think it would be best if patches posted were in the body of
> the email instead of as attachments. It makes it easier to
> review and comment.

Also the subject line is utterly useless. Please take a look at
Documentation/SubmittingPatches for the general format of a patch.
>
> Thanks,
>
> Karim
> --
> President / Opersys Inc.
> Embedded Linux Training and Expertise
> http://www.opersys.com / 1.866.677.4546
---end quoted text---

2006-09-20 12:21:33

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [PATCH 1/11] LTTng-core 0.5.111 : Relay+DebugFS (DebugFS fix)

* Greg KH ([email protected]) wrote:
> On Sat, Sep 16, 2006 at 03:51:03AM -0400, Mathieu Desnoyers wrote:
> > 1 - DebugFS stalled dentry patch
> > DebugFS seems to keep a stalled dentry when a process is in a directory that is
> > being removed. Force a differed deletion.
> > patch-2.6.17-lttng-core-0.5.111-debugfs.diff
> >
> >
> > OpenPGP public key: http://krystal.dyndns.org:8080/key/compudj.gpg
> > Key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
>
> > --- a/fs/debugfs/inode.c
> > +++ b/fs/debugfs/inode.c
> > @@ -266,6 +266,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_dir);
> > void debugfs_remove(struct dentry *dentry)
> > {
> > struct dentry *parent;
> > + int ret = 0;
> >
> > if (!dentry)
> > return;
> > @@ -278,9 +279,10 @@ void debugfs_remove(struct dentry *dentr
> > if (debugfs_positive(dentry)) {
> > if (dentry->d_inode) {
> > if (S_ISDIR(dentry->d_inode->i_mode))
> > - simple_rmdir(parent->d_inode, dentry);
> > + ret = simple_rmdir(parent->d_inode, dentry);
> > else
> > - simple_unlink(parent->d_inode, dentry);
> > + ret = simple_unlink(parent->d_inode, dentry);
> > + if(ret) d_delete(dentry);
>
> Are you saying that perhaps all other users of simple_unlink() are also
> broken like this? If so, why not just fix simple_unlink()?
>

I don't think that libfs is fundamentally broken, as simple_unlink always
returns 0 but simple_rmdir may fail if !simple_empty(dentry). I think that the
decision of what to do in such situation is "simply" left to the caller.

But you probably know more than I do on that matter.

Mathieu



OpenPGP public key: http://krystal.dyndns.org:8080/key/compudj.gpg
Key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68