linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 5/5] f2fs: Fix for parent inode information during server cache eviction
@ 2012-12-08  5:55 Namjae Jeon
  2012-12-10  0:43 ` Jaegeuk Kim
  0 siblings, 1 reply; 8+ messages in thread
From: Namjae Jeon @ 2012-12-08  5:55 UTC (permalink / raw)
  To: jaegeuk.kim
  Cc: linux-fsdevel, linux-kernel, linux-f2fs-devel, Namjae Jeon,
	Namjae Jeon, Amit Sahrawat

From: Namjae Jeon <namjae.jeon@samsung.com>

Test Case:
[NFS Client]
ls -lR .

[NFS Server]
while [ 1 ]
do
echo 3 > /proc/sys/vm/drop_caches
done

Error: "No such file or directory"

When cache is dropped at the server, it results in lookup failure at the
NFS client. Even though the file exists. Looking at the code to rebuild 
the inode in case of cache eviction. It tries to initiate a lookup operation
for ".." to get the parent information using the on-disk inode number.

But, in case of f2fs we do not need to perform a lookup based upon name.
As for f2fs layout - the f2fs inode already has reference to the parent 
inode number. So, like a normal inode build-up, parent inode can also be 
regenerated by reading parent inode number.

f2fs_inode_info represents the in-memory inode, while f2fs_inode represents
the on-disk. So, we introduce the parent inode number in the f2fs_inode_info also.
Whild doing do_read_inode() reading on-disk information, we populate the parent 
inode number also.This way whenever we reference f2fs_inode using F2FS_I over 
VFS inode. We are sure to get the parent inode in the inode.

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
---
 fs/f2fs/dir.c   |   16 ----------------
 fs/f2fs/f2fs.h  |    1 +
 fs/f2fs/inode.c |    1 +
 fs/f2fs/namei.c |    5 +++--
 4 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index d900c08..a4c9c9d 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -226,22 +226,6 @@ struct f2fs_dir_entry *f2fs_parent_dir(struct inode *dir, struct page **p)
 	return de;
 }
 
-ino_t f2fs_inode_by_name(struct inode *dir, struct qstr *qstr)
-{
-	ino_t res = 0;
-	struct f2fs_dir_entry *de;
-	struct page *page;
-
-	de = f2fs_find_entry(dir, qstr, &page);
-	if (de) {
-		res = le32_to_cpu(de->ino);
-		kunmap(page);
-		f2fs_put_page(page, 0);
-	}
-
-	return res;
-}
-
 void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry *de,
 		struct page *page, struct inode *inode)
 {
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 8c3f1ef..0b56cbb 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -146,6 +146,7 @@ struct f2fs_inode_info {
 	unsigned int clevel;		/* maximum level of given file name */
 	nid_t i_xattr_nid;		/* node id that contains xattrs */
 	struct extent_info ext;		/* in-memory extent cache entry */
+	unsigned long parent_ino;
 };
 
 static inline void get_extent_info(struct extent_info *ext,
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index aa4ef4f..1880d8a 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -107,6 +107,7 @@ static int do_read_inode(struct inode *inode)
 	fi->flags = 0;
 	fi->data_version = le64_to_cpu(F2FS_CKPT(sbi)->checkpoint_ver) - 1;
 	fi->i_advise = ri->i_advise;
+	fi->parent_ino = le32_to_cpu(ri->i_pino);
 	get_extent_info(&fi->ext, ri->i_ext);
 	f2fs_put_page(node_page, 1);
 	return 0;
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 89b7675..ab021cc 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -183,8 +183,9 @@ out:
 
 struct dentry *f2fs_get_parent(struct dentry *child)
 {
-	struct qstr dotdot = QSTR_INIT("..", 2);
-	unsigned long ino = f2fs_inode_by_name(child->d_inode, &dotdot);
+	unsigned long ino;
+	struct f2fs_inode_info *fi = F2FS_I(child->d_inode);
+	ino = fi->parent_ino;
 	if (!ino)
 		return ERR_PTR(-ENOENT);
 	return d_obtain_alias(f2fs_iget(child->d_inode->i_sb, ino));
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 5/5] f2fs: Fix for parent inode information during server cache eviction
  2012-12-08  5:55 [PATCH 5/5] f2fs: Fix for parent inode information during server cache eviction Namjae Jeon
@ 2012-12-10  0:43 ` Jaegeuk Kim
  2012-12-10  3:40   ` Namjae Jeon
  0 siblings, 1 reply; 8+ messages in thread
From: Jaegeuk Kim @ 2012-12-10  0:43 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: linux-fsdevel, linux-f2fs-devel, Namjae Jeon, Amit Sahrawat

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

2012-12-08 (토), 14:55 +0900, Namjae Jeon:
> From: Namjae Jeon <namjae.jeon@samsung.com>
> 
> Test Case:
> [NFS Client]
> ls -lR .
> 
> [NFS Server]
> while [ 1 ]
> do
> echo 3 > /proc/sys/vm/drop_caches
> done
> 
> Error: "No such file or directory"
> 
> When cache is dropped at the server, it results in lookup failure at the
> NFS client. Even though the file exists. Looking at the code to rebuild 
> the inode in case of cache eviction. It tries to initiate a lookup operation
> for ".." to get the parent information using the on-disk inode number.
> 

Could you describe why this patch resolves that bug?
Before applying this, we need to figure out why that bug is occurred.
IMO, from the viewpoint of functionality, ".." resolution should work
too.
Thanks,

-- 
Jaegeuk Kim
Samsung

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 5/5] f2fs: Fix for parent inode information during server cache eviction
  2012-12-10  0:43 ` Jaegeuk Kim
@ 2012-12-10  3:40   ` Namjae Jeon
  2012-12-10  4:37     ` Jaegeuk Kim
  0 siblings, 1 reply; 8+ messages in thread
From: Namjae Jeon @ 2012-12-10  3:40 UTC (permalink / raw)
  To: jaegeuk.kim; +Cc: linux-fsdevel, linux-f2fs-devel, Namjae Jeon, Amit Sahrawat

2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> 2012-12-08 (토), 14:55 +0900, Namjae Jeon:
>> From: Namjae Jeon <namjae.jeon@samsung.com>
>>
>> Test Case:
>> [NFS Client]
>> ls -lR .
>>
>> [NFS Server]
>> while [ 1 ]
>> do
>> echo 3 > /proc/sys/vm/drop_caches
>> done
>>
>> Error: "No such file or directory"
>>
>> When cache is dropped at the server, it results in lookup failure at the
>> NFS client. Even though the file exists. Looking at the code to rebuild
>> the inode in case of cache eviction. It tries to initiate a lookup
>> operation
>> for ".." to get the parent information using the on-disk inode number.
>>
>
> Could you describe why this patch resolves that bug?
> Before applying this, we need to figure out why that bug is occurred.
> IMO, from the viewpoint of functionality, ".." resolution should work
> too.
dotdot entry of f2fs is stored when creating only directory not
regular file. Am I correct ?
So when the parent of file was evicted, I thought we could not get
parent inode number of file thoughout dotdot entry.
And f2fs inode is having parent inode number unlike other fs. so I
think we can use this special thing by storing f2fs_inode_info.

Thanks.
> Thanks,
>
> --
> Jaegeuk Kim
> Samsung
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 5/5] f2fs: Fix for parent inode information during server cache eviction
  2012-12-10  3:40   ` Namjae Jeon
@ 2012-12-10  4:37     ` Jaegeuk Kim
  2012-12-10  5:25       ` Namjae Jeon
  0 siblings, 1 reply; 8+ messages in thread
From: Jaegeuk Kim @ 2012-12-10  4:37 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: linux-fsdevel, linux-f2fs-devel, Namjae Jeon, Amit Sahrawat

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

2012-12-10 (월), 12:40 +0900, Namjae Jeon:
> 2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> > 2012-12-08 (토), 14:55 +0900, Namjae Jeon:
> >> From: Namjae Jeon <namjae.jeon@samsung.com>
> >>
> >> Test Case:
> >> [NFS Client]
> >> ls -lR .
> >>
> >> [NFS Server]
> >> while [ 1 ]
> >> do
> >> echo 3 > /proc/sys/vm/drop_caches
> >> done
> >>
> >> Error: "No such file or directory"
> >>
> >> When cache is dropped at the server, it results in lookup failure at the
> >> NFS client. Even though the file exists. Looking at the code to rebuild
> >> the inode in case of cache eviction. It tries to initiate a lookup
> >> operation
> >> for ".." to get the parent information using the on-disk inode number.
> >>
> >
> > Could you describe why this patch resolves that bug?
> > Before applying this, we need to figure out why that bug is occurred.
> > IMO, from the viewpoint of functionality, ".." resolution should work
> > too.
> dotdot entry of f2fs is stored when creating only directory not
> regular file. Am I correct ?

Yep.

> So when the parent of file was evicted, I thought we could not get
> parent inode number of file thoughout dotdot entry.

What do you mean the parent of file? Isn't it a directory?

> And f2fs inode is having parent inode number unlike other fs. so I
> think we can use this special thing by storing f2fs_inode_info.

f2fs stores a dotdot dentry *likewise* other fs.
The pino in f2fs_inode is specially added for POR intentionally.

Still I cannot imagine the bug scenario.

> 
> Thanks.
> > Thanks,
> >
> > --
> > Jaegeuk Kim
> > Samsung
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Jaegeuk Kim
Samsung

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 5/5] f2fs: Fix for parent inode information during server cache eviction
  2012-12-10  4:37     ` Jaegeuk Kim
@ 2012-12-10  5:25       ` Namjae Jeon
  2012-12-10  7:00         ` Jaegeuk Kim
  0 siblings, 1 reply; 8+ messages in thread
From: Namjae Jeon @ 2012-12-10  5:25 UTC (permalink / raw)
  To: jaegeuk.kim; +Cc: linux-fsdevel, linux-f2fs-devel, Namjae Jeon, Amit Sahrawat

2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> 2012-12-10 (월), 12:40 +0900, Namjae Jeon:
>> 2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
>> > 2012-12-08 (토), 14:55 +0900, Namjae Jeon:
>> >> From: Namjae Jeon <namjae.jeon@samsung.com>
>> >>
>> >> Test Case:
>> >> [NFS Client]
>> >> ls -lR .
>> >>
>> >> [NFS Server]
>> >> while [ 1 ]
>> >> do
>> >> echo 3 > /proc/sys/vm/drop_caches
>> >> done
>> >>
>> >> Error: "No such file or directory"
>> >>
>> >> When cache is dropped at the server, it results in lookup failure at
>> >> the
>> >> NFS client. Even though the file exists. Looking at the code to
>> >> rebuild
>> >> the inode in case of cache eviction. It tries to initiate a lookup
>> >> operation
>> >> for ".." to get the parent information using the on-disk inode number.
>> >>
>> >
>> > Could you describe why this patch resolves that bug?
>> > Before applying this, we need to figure out why that bug is occurred.
>> > IMO, from the viewpoint of functionality, ".." resolution should work
>> > too.
>> dotdot entry of f2fs is stored when creating only directory not
>> regular file. Am I correct ?
>
> Yep.
>
>> So when the parent of file was evicted, I thought we could not get
>> parent inode number of file thoughout dotdot entry.
>
> What do you mean the parent of file? Isn't it a directory?
>
>> And f2fs inode is having parent inode number unlike other fs. so I
>> think we can use this special thing by storing f2fs_inode_info.
>
> f2fs stores a dotdot dentry *likewise* other fs.
> The pino in f2fs_inode is specially added for POR intentionally.
>
> Still I cannot imagine the bug scenario.
When we observed the issue in NFS operations. We found that the issues
occurs while trying to “reconnect” with parent ? and shows “No such
file or directory” for directory paths.

As a matter of fact - we were also checking the on-disk layout for the
f2fs_inode. We found that since, F2FS inode is keeping a reference to
the parent-inode within its on-disk information. So, we should use the
same information to reconstruct the parent in case of eviction.

The benefit is:
It also saves the lookup() to be performed for “..” in the directory
entry block while trying to reconnect with the parent. Instead we can
directly use the parent inode number in the inode and generate from
that point itself.  Even though the get_parent() code was made similar
to other filesystem which generally do not have reference to the
parent inode -  so they need to perform a lookup for “..”, but in this
case we sincerely thought we can get rid of that method.
So, we did not tried to figure out what could be possible solution in
the current scenario.
As per your reference, parent inode number for on-disk inode is
introduced for POR intentionally. So, we should not use information
for any reconstruction?

Thanks.
>
>>
>> Thanks.
>> > Thanks,
>> >
>> > --
>> > Jaegeuk Kim
>> > Samsung
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel"
>> in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Jaegeuk Kim
> Samsung
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 5/5] f2fs: Fix for parent inode information during server cache eviction
  2012-12-10  5:25       ` Namjae Jeon
@ 2012-12-10  7:00         ` Jaegeuk Kim
  2012-12-10  7:11           ` Namjae Jeon
  0 siblings, 1 reply; 8+ messages in thread
From: Jaegeuk Kim @ 2012-12-10  7:00 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: linux-fsdevel, linux-f2fs-devel, Namjae Jeon, Amit Sahrawat

[-- Attachment #1: Type: text/plain, Size: 3419 bytes --]

2012-12-10 (월), 14:25 +0900, Namjae Jeon:
> 2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> > 2012-12-10 (월), 12:40 +0900, Namjae Jeon:
> >> 2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> >> > 2012-12-08 (토), 14:55 +0900, Namjae Jeon:
> >> >> From: Namjae Jeon <namjae.jeon@samsung.com>
> >> >>
> >> >> Test Case:
> >> >> [NFS Client]
> >> >> ls -lR .
> >> >>
> >> >> [NFS Server]
> >> >> while [ 1 ]
> >> >> do
> >> >> echo 3 > /proc/sys/vm/drop_caches
> >> >> done
> >> >>
> >> >> Error: "No such file or directory"
> >> >>
> >> >> When cache is dropped at the server, it results in lookup failure at
> >> >> the
> >> >> NFS client. Even though the file exists. Looking at the code to
> >> >> rebuild
> >> >> the inode in case of cache eviction. It tries to initiate a lookup
> >> >> operation
> >> >> for ".." to get the parent information using the on-disk inode number.
> >> >>
> >> >
> >> > Could you describe why this patch resolves that bug?
> >> > Before applying this, we need to figure out why that bug is occurred.
> >> > IMO, from the viewpoint of functionality, ".." resolution should work
> >> > too.
> >> dotdot entry of f2fs is stored when creating only directory not
> >> regular file. Am I correct ?
> >
> > Yep.
> >
> >> So when the parent of file was evicted, I thought we could not get
> >> parent inode number of file thoughout dotdot entry.
> >
> > What do you mean the parent of file? Isn't it a directory?
> >
> >> And f2fs inode is having parent inode number unlike other fs. so I
> >> think we can use this special thing by storing f2fs_inode_info.
> >
> > f2fs stores a dotdot dentry *likewise* other fs.
> > The pino in f2fs_inode is specially added for POR intentionally.
> >
> > Still I cannot imagine the bug scenario.
> When we observed the issue in NFS operations. We found that the issues
> occurs while trying to “reconnect” with parent ? and shows “No such
> file or directory” for directory paths.

Again, you didn't describe why this bug is occurred.
Please, analyze the bug scenario first.

> 
> As a matter of fact - we were also checking the on-disk layout for the
> f2fs_inode. We found that since, F2FS inode is keeping a reference to
> the parent-inode within its on-disk information. So, we should use the
> same information to reconstruct the parent in case of eviction.
> 
> The benefit is:
> It also saves the lookup() to be performed for “..” in the directory
> entry block while trying to reconnect with the parent. Instead we can
> directly use the parent inode number in the inode and generate from
> that point itself.  Even though the get_parent() code was made similar
> to other filesystem which generally do not have reference to the
> parent inode -  so they need to perform a lookup for “..”, but in this
> case we sincerely thought we can get rid of that method.
> So, we did not tried to figure out what could be possible solution in
> the current scenario.
> As per your reference, parent inode number for on-disk inode is
> introduced for POR intentionally. So, we should not use information
> for any reconstruction?

Of course, we can enhance the directory operations by using the "pino".
But what I concern is how to resolve the bug by this enhancement patch.
This is not a bug fix patch. Isn't it?

Thanks,

-- 
Jaegeuk Kim
Samsung

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 5/5] f2fs: Fix for parent inode information during server cache eviction
  2012-12-10  7:00         ` Jaegeuk Kim
@ 2012-12-10  7:11           ` Namjae Jeon
  2012-12-10  7:19             ` Jaegeuk Kim
  0 siblings, 1 reply; 8+ messages in thread
From: Namjae Jeon @ 2012-12-10  7:11 UTC (permalink / raw)
  To: jaegeuk.kim; +Cc: linux-fsdevel, linux-f2fs-devel, Namjae Jeon, Amit Sahrawat

2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> 2012-12-10 (월), 14:25 +0900, Namjae Jeon:
>> 2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
>> > 2012-12-10 (월), 12:40 +0900, Namjae Jeon:
>> >> 2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
>> >> > 2012-12-08 (토), 14:55 +0900, Namjae Jeon:
>> >> >> From: Namjae Jeon <namjae.jeon@samsung.com>
>> >> >>
>> >> >> Test Case:
>> >> >> [NFS Client]
>> >> >> ls -lR .
>> >> >>
>> >> >> [NFS Server]
>> >> >> while [ 1 ]
>> >> >> do
>> >> >> echo 3 > /proc/sys/vm/drop_caches
>> >> >> done
>> >> >>
>> >> >> Error: "No such file or directory"
>> >> >>
>> >> >> When cache is dropped at the server, it results in lookup failure
>> >> >> at
>> >> >> the
>> >> >> NFS client. Even though the file exists. Looking at the code to
>> >> >> rebuild
>> >> >> the inode in case of cache eviction. It tries to initiate a lookup
>> >> >> operation
>> >> >> for ".." to get the parent information using the on-disk inode
>> >> >> number.
>> >> >>
>> >> >
>> >> > Could you describe why this patch resolves that bug?
>> >> > Before applying this, we need to figure out why that bug is
>> >> > occurred.
>> >> > IMO, from the viewpoint of functionality, ".." resolution should
>> >> > work
>> >> > too.
>> >> dotdot entry of f2fs is stored when creating only directory not
>> >> regular file. Am I correct ?
>> >
>> > Yep.
>> >
>> >> So when the parent of file was evicted, I thought we could not get
>> >> parent inode number of file thoughout dotdot entry.
>> >
>> > What do you mean the parent of file? Isn't it a directory?
>> >
>> >> And f2fs inode is having parent inode number unlike other fs. so I
>> >> think we can use this special thing by storing f2fs_inode_info.
>> >
>> > f2fs stores a dotdot dentry *likewise* other fs.
>> > The pino in f2fs_inode is specially added for POR intentionally.
>> >
>> > Still I cannot imagine the bug scenario.
>> When we observed the issue in NFS operations. We found that the issues
>> occurs while trying to “reconnect” with parent ? and shows “No such
>> file or directory” for directory paths.
>
> Again, you didn't describe why this bug is occurred.
> Please, analyze the bug scenario first.
Okay, I will look into more.
>
>>
>> As a matter of fact - we were also checking the on-disk layout for the
>> f2fs_inode. We found that since, F2FS inode is keeping a reference to
>> the parent-inode within its on-disk information. So, we should use the
>> same information to reconstruct the parent in case of eviction.
>>
>> The benefit is:
>> It also saves the lookup() to be performed for “..” in the directory
>> entry block while trying to reconnect with the parent. Instead we can
>> directly use the parent inode number in the inode and generate from
>> that point itself.  Even though the get_parent() code was made similar
>> to other filesystem which generally do not have reference to the
>> parent inode -  so they need to perform a lookup for “..”, but in this
>> case we sincerely thought we can get rid of that method.
>> So, we did not tried to figure out what could be possible solution in
>> the current scenario.
>> As per your reference, parent inode number for on-disk inode is
>> introduced for POR intentionally. So, we should not use information
>> for any reconstruction?
>
> Of course, we can enhance the directory operations by using the "pino".
> But what I concern is how to resolve the bug by this enhancement patch.
> This is not a bug fix patch. Isn't it?
Yes, It is not bug fix. this is new method using f2fs's special thing.
Agreed, I will analyse why this bug occur first. let's discuss again
whether we fix bug or use new method.

And please check the rest of patches except this patch.

Thanks.
>
> Thanks,
>
> --
> Jaegeuk Kim
> Samsung
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 5/5] f2fs: Fix for parent inode information during server cache eviction
  2012-12-10  7:11           ` Namjae Jeon
@ 2012-12-10  7:19             ` Jaegeuk Kim
  0 siblings, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2012-12-10  7:19 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: linux-fsdevel, linux-f2fs-devel, Namjae Jeon, Amit Sahrawat

[-- Attachment #1: Type: text/plain, Size: 4278 bytes --]

2012-12-10 (월), 16:11 +0900, Namjae Jeon:
> 2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> > 2012-12-10 (월), 14:25 +0900, Namjae Jeon:
> >> 2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> >> > 2012-12-10 (월), 12:40 +0900, Namjae Jeon:
> >> >> 2012/12/10, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> >> >> > 2012-12-08 (토), 14:55 +0900, Namjae Jeon:
> >> >> >> From: Namjae Jeon <namjae.jeon@samsung.com>
> >> >> >>
> >> >> >> Test Case:
> >> >> >> [NFS Client]
> >> >> >> ls -lR .
> >> >> >>
> >> >> >> [NFS Server]
> >> >> >> while [ 1 ]
> >> >> >> do
> >> >> >> echo 3 > /proc/sys/vm/drop_caches
> >> >> >> done
> >> >> >>
> >> >> >> Error: "No such file or directory"
> >> >> >>
> >> >> >> When cache is dropped at the server, it results in lookup failure
> >> >> >> at
> >> >> >> the
> >> >> >> NFS client. Even though the file exists. Looking at the code to
> >> >> >> rebuild
> >> >> >> the inode in case of cache eviction. It tries to initiate a lookup
> >> >> >> operation
> >> >> >> for ".." to get the parent information using the on-disk inode
> >> >> >> number.
> >> >> >>
> >> >> >
> >> >> > Could you describe why this patch resolves that bug?
> >> >> > Before applying this, we need to figure out why that bug is
> >> >> > occurred.
> >> >> > IMO, from the viewpoint of functionality, ".." resolution should
> >> >> > work
> >> >> > too.
> >> >> dotdot entry of f2fs is stored when creating only directory not
> >> >> regular file. Am I correct ?
> >> >
> >> > Yep.
> >> >
> >> >> So when the parent of file was evicted, I thought we could not get
> >> >> parent inode number of file thoughout dotdot entry.
> >> >
> >> > What do you mean the parent of file? Isn't it a directory?
> >> >
> >> >> And f2fs inode is having parent inode number unlike other fs. so I
> >> >> think we can use this special thing by storing f2fs_inode_info.
> >> >
> >> > f2fs stores a dotdot dentry *likewise* other fs.
> >> > The pino in f2fs_inode is specially added for POR intentionally.
> >> >
> >> > Still I cannot imagine the bug scenario.
> >> When we observed the issue in NFS operations. We found that the issues
> >> occurs while trying to “reconnect” with parent ? and shows “No such
> >> file or directory” for directory paths.
> >
> > Again, you didn't describe why this bug is occurred.
> > Please, analyze the bug scenario first.
> Okay, I will look into more.
> >
> >>
> >> As a matter of fact - we were also checking the on-disk layout for the
> >> f2fs_inode. We found that since, F2FS inode is keeping a reference to
> >> the parent-inode within its on-disk information. So, we should use the
> >> same information to reconstruct the parent in case of eviction.
> >>
> >> The benefit is:
> >> It also saves the lookup() to be performed for “..” in the directory
> >> entry block while trying to reconnect with the parent. Instead we can
> >> directly use the parent inode number in the inode and generate from
> >> that point itself.  Even though the get_parent() code was made similar
> >> to other filesystem which generally do not have reference to the
> >> parent inode -  so they need to perform a lookup for “..”, but in this
> >> case we sincerely thought we can get rid of that method.
> >> So, we did not tried to figure out what could be possible solution in
> >> the current scenario.
> >> As per your reference, parent inode number for on-disk inode is
> >> introduced for POR intentionally. So, we should not use information
> >> for any reconstruction?
> >
> > Of course, we can enhance the directory operations by using the "pino".
> > But what I concern is how to resolve the bug by this enhancement patch.
> > This is not a bug fix patch. Isn't it?
> Yes, It is not bug fix. this is new method using f2fs's special thing.
> Agreed, I will analyse why this bug occur first. let's discuss again
> whether we fix bug or use new method.
> 
> And please check the rest of patches except this patch.

Other patches look good to me.
And I'm testing and seeing the codes one more time.
Thank you for contribution. :)

> 
> Thanks.
> >
> > Thanks,
> >
> > --
> > Jaegeuk Kim
> > Samsung
> >

-- 
Jaegeuk Kim
Samsung

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-12-10  7:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-08  5:55 [PATCH 5/5] f2fs: Fix for parent inode information during server cache eviction Namjae Jeon
2012-12-10  0:43 ` Jaegeuk Kim
2012-12-10  3:40   ` Namjae Jeon
2012-12-10  4:37     ` Jaegeuk Kim
2012-12-10  5:25       ` Namjae Jeon
2012-12-10  7:00         ` Jaegeuk Kim
2012-12-10  7:11           ` Namjae Jeon
2012-12-10  7:19             ` Jaegeuk Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).