lkml.org 
[lkml]   [2010]   [Feb]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Fix __d_path for lazy unmounts
Andrew Morton wrote:
> On Sat, 20 Feb 2010 04:27:38 -0800 john.johansen@canonical.com wrote:
>
>> From: John Johansen <john.johansen@canonical.com>
>>
>> When __d_path() hits a lazily unmounted mount point, it tries to prepend
>> the name of the lazily unmounted dentry to the path name. It gets this wrong,
>> and also overwrites the slash that separates the name from the following
>> pathname component. This patch fixes that; if a process was in directory
>> /foo/bar and /foo got lazily unmounted, the old result was ``foobar'' (note the
>> missing slash), while the new result with this patch is ``/foo/bar''.
>>
>> Signed-off-by: John Johansen <john.johansen@canonical.com>
>> ---
>> fs/dcache.c | 27 +++++++++++++++++++++++----
>> 1 files changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/dcache.c b/fs/dcache.c
>> index 953173a..df49666 100644
>> --- a/fs/dcache.c
>> +++ b/fs/dcache.c
>> @@ -1922,11 +1922,9 @@ char *__d_path(const struct path *path, struct path *root,
>> retval = end-1;
>> *retval = '/';
>>
>> - for (;;) {
>> + while(dentry != root->dentry || vfsmnt != root->mnt) {
>
thanks, forgot to refresh after checkpatch

> Please put a space between the `while' and the `('.
>
>> struct dentry * parent;
>>
>> - if (dentry == root->dentry && vfsmnt == root->mnt)
>> - break;
>> if (dentry == vfsmnt->mnt_root || IS_ROOT(dentry)) {
>> /* Global root? */
>> if (vfsmnt->mnt_parent == vfsmnt) {
>> @@ -1950,9 +1948,30 @@ out:
>> return retval;
>>
>> global_root:
>> - retval += 1; /* hit the slash */
>> + /*
>> + * We went past the (vfsmount, dentry) we were looking for and have
>> + * either hit a root dentry, a lazily unmounted dentry, an
>> + * unconnected dentry, or the file is on a pseudo filesystem.
>> + */
>> + if ((dentry->d_sb->s_flags & MS_NOUSER) ||
>> + (dentry->d_name.len = 1 && *dentry->d_name.name == '/')) {
>
> Did you really mean to assign 1 to dentry->d_name.len here? Was `=='
> intended? I hope so, because modifying the dentry in d_path() would be odd.
>
> If this was a mistake then why did the patch pass testing?
>
Nope, definite bug, missed that case in testing. In this case every test that
had a leading '/' had a d_name.len == 1 as well.

I haven't seen the case of where a root dentry has a leading / and doesn't have
a d_name.len == 1 and if that case never happens the test wouldn't be needed.

I will respin the patch, and include testing this time

thanks
john


\
 
 \ /
  Last update: 2010-02-23 02:59    [W:0.109 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site