* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main @ 2007-02-07 4:51 Wendy Cheng 2007-02-07 10:49 ` Steven Whitehouse 0 siblings, 1 reply; 14+ messages in thread From: Wendy Cheng @ 2007-02-07 4:51 UTC (permalink / raw) To: cluster-devel.redhat.com Lookup failure found in '07 NFS connectathon. The nfsd is in the middle of readdirplus procedure call where it builds the file handles associated with the directory. GFS2 lookup code has been expected the first 64 bit of gfs2 inode number (formal inode number) but do_filldir_main() wrongly passes on-disk address inode number (no_addr). This patch fixes this. Signed-off-by: S. Wendy Cheng <wcheng@redhat.com> dir.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletion(-) --- linux-git/fs/gfs2/dir.c 2007-02-06 01:07:26.000000000 -0500 +++ linux/fs/gfs2/dir.c 2007-02-06 20:50:16.000000000 -0500 @@ -1241,8 +1241,9 @@ static int do_filldir_main(struct gfs2_i error = filldir(opaque, (const char *)(dent + 1), be16_to_cpu(dent->de_name_len), - off, be64_to_cpu(dent->de_inum.no_addr), + off, be64_to_cpu(dent->de_inum.no_formal_ino), be16_to_cpu(dent->de_type)); + if (error) return 1; ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-07 4:51 [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main Wendy Cheng @ 2007-02-07 10:49 ` Steven Whitehouse 2007-02-07 22:35 ` Wendy Cheng 0 siblings, 1 reply; 14+ messages in thread From: Steven Whitehouse @ 2007-02-07 10:49 UTC (permalink / raw) To: cluster-devel.redhat.com Hi, I'm not sure that this is the right fix. no_formal_ino is used only as a hash value in the lookup, the "real" lookup is always via no_addr. So we can alter that hash if required to just use no_addr, but I'd rather not have to return no_formal_ino here since we have no way to map that back to the disk location of the inode in general. In fact given a maximum sized GFS2 filesystem and enough nodes to fill it with inodes in a reasonable time (yes, I know thats a lot and a long time!), its not even certain to be unique, Steve. On Tue, 2007-02-06 at 23:51 -0500, Wendy Cheng wrote: > Lookup failure found in '07 NFS connectathon. The nfsd is in the middle > of readdirplus procedure call where it builds the file handles > associated with the directory. GFS2 lookup code has been expected the > first 64 bit of gfs2 inode number (formal inode number) but > do_filldir_main() wrongly passes on-disk address inode number (no_addr). > This patch fixes this. > > Signed-off-by: S. Wendy Cheng <wcheng@redhat.com> > > dir.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletion(-) > > --- linux-git/fs/gfs2/dir.c 2007-02-06 01:07:26.000000000 -0500 > +++ linux/fs/gfs2/dir.c 2007-02-06 20:50:16.000000000 -0500 > @@ -1241,8 +1241,9 @@ static int do_filldir_main(struct gfs2_i > > error = filldir(opaque, (const char *)(dent + 1), > be16_to_cpu(dent->de_name_len), > - off, be64_to_cpu(dent->de_inum.no_addr), > + off, be64_to_cpu(dent->de_inum.no_formal_ino), > be16_to_cpu(dent->de_type)); > + > if (error) > return 1; > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-07 10:49 ` Steven Whitehouse @ 2007-02-07 22:35 ` Wendy Cheng 2007-02-07 23:37 ` Wendy Cheng 2007-02-08 9:42 ` Steven Whitehouse 0 siblings, 2 replies; 14+ messages in thread From: Wendy Cheng @ 2007-02-07 22:35 UTC (permalink / raw) To: cluster-devel.redhat.com On Wed, 2007-02-07 at 10:49 +0000, Steven Whitehouse wrote: > Hi, > > I'm not sure that this is the right fix. no_formal_ino is used only as a > hash value in the lookup, the "real" lookup is always via no_addr. So we > can alter that hash if required to just use no_addr, but I'd rather not > have to return no_formal_ino here since we have no way to map that back > to the disk location of the inode in general. In fact given a maximum > sized GFS2 filesystem and enough nodes to fill it with inodes in a > reasonable time (yes, I know thats a lot and a long time!), its not even > certain to be unique, > NFS won't run well without this patch - I originally included this into an "if command == nfsd" clause. Later checking into Ken's description (in cluster-list at redhat.com) about this issue, I think his intention here was that any ino returned to user space should use no_formal_ino. And this is exactly what the filldir should be doing (by returning no_formal_ino back to caller). Another option is temporarily disabling this feature and make this works as GFS1 (with these two inode numbers identical) ? I like the current patch though. -- Wendy ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-07 22:35 ` Wendy Cheng @ 2007-02-07 23:37 ` Wendy Cheng 2007-02-08 9:42 ` Steven Whitehouse 1 sibling, 0 replies; 14+ messages in thread From: Wendy Cheng @ 2007-02-07 23:37 UTC (permalink / raw) To: cluster-devel.redhat.com On Wed, 2007-02-07 at 17:35 -0500, Wendy Cheng wrote: > On Wed, 2007-02-07 at 10:49 +0000, Steven Whitehouse wrote: > > Hi, > > > > I'm not sure that this is the right fix. no_formal_ino is used only as a > > hash value in the lookup, the "real" lookup is always via no_addr. So we > > can alter that hash if required to just use no_addr, but I'd rather not > > have to return no_formal_ino here since we have no way to map that back > > to the disk location of the inode in general. In fact given a maximum > > sized GFS2 filesystem and enough nodes to fill it with inodes in a > > reasonable time (yes, I know thats a lot and a long time!), its not even > > certain to be unique, > > > > NFS won't run well without this patch - I originally included this into > an "if command == nfsd" clause. Later checking into Ken's description > (in cluster-list at redhat.com) about this issue, I think his intention > here was that any ino returned to user space should use no_formal_ino. > And this is exactly what the filldir should be doing (by returning > no_formal_ino back to caller). > > Another option is temporarily disabling this feature and make this works > as GFS1 (with these two inode numbers identical) ? To make it clear .. we need to either change the original design or I think we should take this patch - since 1. The contents of filldir are intended to get passed to user mode - and that should be the formal inode number 2. NFS can't run without this change. -- Wendy ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-07 22:35 ` Wendy Cheng 2007-02-07 23:37 ` Wendy Cheng @ 2007-02-08 9:42 ` Steven Whitehouse 2007-02-24 3:56 ` Wendy Cheng 1 sibling, 1 reply; 14+ messages in thread From: Steven Whitehouse @ 2007-02-08 9:42 UTC (permalink / raw) To: cluster-devel.redhat.com Hi, On Wed, 2007-02-07 at 17:35 -0500, Wendy Cheng wrote: > On Wed, 2007-02-07 at 10:49 +0000, Steven Whitehouse wrote: > > Hi, > > > > I'm not sure that this is the right fix. no_formal_ino is used only as a > > hash value in the lookup, the "real" lookup is always via no_addr. So we > > can alter that hash if required to just use no_addr, but I'd rather not > > have to return no_formal_ino here since we have no way to map that back > > to the disk location of the inode in general. In fact given a maximum > > sized GFS2 filesystem and enough nodes to fill it with inodes in a > > reasonable time (yes, I know thats a lot and a long time!), its not even > > certain to be unique, > > > > NFS won't run well without this patch - I originally included this into > an "if command == nfsd" clause. Later checking into Ken's description > (in cluster-list at redhat.com) about this issue, I think his intention > here was that any ino returned to user space should use no_formal_ino. > And this is exactly what the filldir should be doing (by returning > no_formal_ino back to caller). > I'd rather not use no_formal_ino at all if possible. It is not, in general, unique for any fs which has seen a lot of activity and there is also no way to look up an inode from its no_formal_ino alone without potentially searching the entire fs. To my mind an inode number should be guaranteed to be unique for the lifetime of the inode and also be useable as a reference by which the inode can be looked up (whether thats directly by making it identical to the disk block number or indirectly by using a lookup table, for example), no_formal_ino doesn't appear to satisfy either condition where as no_addr does. If there was an index to map no_formal_ino to a disk block, then I'd agree, but I'd much prefer that we use no_addr in the mean time. I suspect that all you need to do is to alter gfs2_ilookup() and gfs2_iget() to use no_addr rather than no_formal_ino as the hash value. Since that hash is only used internally, there should be no problem in altering it and I think that will result in the same thing. What I've not been able to work out is how your scheme can work in the case that the inode is not already in cache, since filldir doesn't then appear to get passed the no_addr at all. > Another option is temporarily disabling this feature and make this works > as GFS1 (with these two inode numbers identical) ? > > I like the current patch though. > > -- Wendy Well one other option is to use the inode version number that was added recently to each inode. The idea of no_formal_ino is that its not really an inode number at all in the traditional sense, but just a way to ensure that an inode isn't being accessed from a stale filehandle. I added the inode version number with the intention that we'd be able to use it instead of no_formal_ino, eventually, with NFS. One reason is that because the max sized filesystem is 2^64 blocks and the no_formal_ino is also 64 bits in size its quite likely to correlate with the size of the fs and thus generate identical numbers once its wrapped for similar disk offsets. Thats also true for any smaller fs which is a power of two in size I think, but with a large number of rewrites of the fs required in each case. With the version number scheme, the version numbers are unique to a particular resource group so we are getting 2^64/2^32 (for a max sized resource group) = 2^32 (average) reallocations of each block within the rgrp as an inode before the counter will wrap, Steve. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-08 9:42 ` Steven Whitehouse @ 2007-02-24 3:56 ` Wendy Cheng 2007-02-26 15:45 ` Steven Whitehouse 0 siblings, 1 reply; 14+ messages in thread From: Wendy Cheng @ 2007-02-24 3:56 UTC (permalink / raw) To: cluster-devel.redhat.com Steve, I gave this issue some more thoughts - would like to suggest we take this patch (at least for now) since it aligns with current code base. The no_formal_ino is apparently intented to get returned back to user space due to its unique-ness (and we have to trust pick_formal_ino() does its job right). With normal readdir system call, after the inode number is sent to user space, there is no route (I've checked) for it to come back to kernel. So the only user that would use these filldir ino inside the kernel is NFSD. NFSD calls gfs2_ilookup() that requires no_formal_ino. If you look further... The current lookup code actually uses no_formal_ino, not no_addr. The two main "gate" routines that controls ino-to-inode conversion are: * gfs2_ilookup() (used by NFS route) * gfs2_inode_lookup() (used by VFS that calls gfs2_lookup()). Both use no_formal_ino - gfs2_inode_lookup() logic hides this behind the little wrapper "gfs2_iget()". Since current VFS side's lookup has been working fine, this no_formal_ino idea apparently is working. So let's make NFSD side work the *same* way. I'm convinced this patch does a right thing. I don't dispute using generation number may not be a bad idea and may perform better. However, if we really look into the details, it is not easy for current code structure. Since we have something already working, it is not wise to mess this code layout at this moment. Make sense ? -- Wendy ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-24 3:56 ` Wendy Cheng @ 2007-02-26 15:45 ` Steven Whitehouse 2007-02-26 16:11 ` Jonathan E Brassow 0 siblings, 1 reply; 14+ messages in thread From: Steven Whitehouse @ 2007-02-26 15:45 UTC (permalink / raw) To: cluster-devel.redhat.com Hi, On Fri, 2007-02-23 at 22:56 -0500, Wendy Cheng wrote: > Steve, > > I gave this issue some more thoughts - would like to suggest we take > this patch (at least for now) since it aligns with current code base. > > The no_formal_ino is apparently intented to get returned back to user > space due to its unique-ness (and we have to trust pick_formal_ino() > does its job right). With normal readdir system call, after the inode > number is sent to user space, there is no route (I've checked) for it to > come back to kernel. So the only user that would use these filldir ino > inside the kernel is NFSD. NFSD calls gfs2_ilookup() that requires > no_formal_ino. > no_formal_ino is not ever guaranteed to be unique which is why I want to keep it away from userspace. I don't really want to have to change that in order to fix NFS. As you say there is no route for the inode number returned via readdir to come back to the kernel, but it should match the inode number returned by stat and I don't see that we should change either of them just to get NFS to work when userspace is perfectly ok as it is. > If you look further... The current lookup code actually uses > no_formal_ino, not no_addr. The two main "gate" routines that controls > ino-to-inode conversion are: > > * gfs2_ilookup() (used by NFS route) > * gfs2_inode_lookup() (used by VFS that calls gfs2_lookup()). > Neither of them need to use no_formal_ino at all. The lookup is using no_addr as there is no other index which makes sense. I know that no_formal_ino is used as part of the hash, but it shouldn't be really and its trivial to change that. You can't swap to just using no_formal_ino on its own as there is no mapping to find its on-disk location from no_formal_ino. > Both use no_formal_ino - gfs2_inode_lookup() logic hides this behind the > little wrapper "gfs2_iget()". Since current VFS side's lookup has been > working fine, this no_formal_ino idea apparently is working. So let's > make NFSD side work the *same* way. I'm convinced this patch does a > right thing. > > I don't dispute using generation number may not be a bad idea and may > perform better. However, if we really look into the details, it is not > easy for current code structure. Since we have something already > working, it is not wise to mess this code layout at this moment. > > Make sense ? > > -- Wendy > I'm afraid that I'm still not convinced on that. It seems that ext3 uses the method that I'm proposing in that it looks up the inode by inode number and then only afterwards checks the generation number. It also has the advantage of separating the NFS interface from the rest of the filesystem. That seems to me to be a lot of the problem that we are looking at here in that the VFS's lookup function wants to look up by no_addr and (currently) NFS is asking for something slightly different, Steve. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-26 15:45 ` Steven Whitehouse @ 2007-02-26 16:11 ` Jonathan E Brassow 2007-02-26 17:16 ` Steven Whitehouse 0 siblings, 1 reply; 14+ messages in thread From: Jonathan E Brassow @ 2007-02-26 16:11 UTC (permalink / raw) To: cluster-devel.redhat.com Please note, I haven't been following current GFS2 development... On Feb 26, 2007, at 9:45 AM, Steven Whitehouse wrote: > Hi, > > On Fri, 2007-02-23 at 22:56 -0500, Wendy Cheng wrote: >> Steve, >> >> I gave this issue some more thoughts - would like to suggest we take >> this patch (at least for now) since it aligns with current code base. >> >> The no_formal_ino is apparently intented to get returned back to user >> space due to its unique-ness (and we have to trust pick_formal_ino() >> does its job right). With normal readdir system call, after the inode >> number is sent to user space, there is no route (I've checked) for it >> to >> come back to kernel. So the only user that would use these filldir ino >> inside the kernel is NFSD. NFSD calls gfs2_ilookup() that requires >> no_formal_ino. >> > no_formal_ino is not ever guaranteed to be unique which is why I want > to > keep it away from userspace. I don't really want to have to change that > in order to fix NFS. Why is it not unique? (from Ken's doc on GFS2: "If the filesystem allocated 1 billion inodes per second, it would take over 500 years for roll-over.") > As you say there is no route for the inode number returned via readdir > to come back to the kernel, but it should match the inode number > returned by stat and I don't see that we should change either of them > just to get NFS to work when userspace is perfectly ok as it is. There were alot of things done to make NFS work properly with GFS - many of them through great pain, because GFS is a clustered FS. >> If you look further... The current lookup code actually uses >> no_formal_ino, not no_addr. The two main "gate" routines that controls >> ino-to-inode conversion are: >> >> * gfs2_ilookup() (used by NFS route) >> * gfs2_inode_lookup() (used by VFS that calls gfs2_lookup()). >> > Neither of them need to use no_formal_ino at all. The lookup is using > no_addr as there is no other index which makes sense. I know that > no_formal_ino is used as part of the hash, but it shouldn't be really > and its trivial to change that. You can't swap to just using > no_formal_ino on its own as there is no mapping to find its on-disk > location from no_formal_ino. > >> Both use no_formal_ino - gfs2_inode_lookup() logic hides this behind >> the >> little wrapper "gfs2_iget()". Since current VFS side's lookup has been >> working fine, this no_formal_ino idea apparently is working. So let's >> make NFSD side work the *same* way. I'm convinced this patch does a >> right thing. >> >> I don't dispute using generation number may not be a bad idea and may >> perform better. However, if we really look into the details, it is not >> easy for current code structure. Since we have something already >> working, it is not wise to mess this code layout at this moment. >> >> Make sense ? >> >> -- Wendy >> > I'm afraid that I'm still not convinced on that. It seems that ext3 > uses > the method that I'm proposing in that it looks up the inode by inode > number and then only afterwards checks the generation number. It also > has the advantage of separating the NFS interface from the rest of the > filesystem. That seems to me to be a lot of the problem that we are > looking at here in that the VFS's lookup function wants to look up by > no_addr and (currently) NFS is asking for something slightly different, > How do you plan on handling inode migration when using NFS? I believe that was one of the central reasons for having no_formal_inode. There may be other reasons as well, as I am not familiar with the code. brassow ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-26 16:11 ` Jonathan E Brassow @ 2007-02-26 17:16 ` Steven Whitehouse 2007-02-27 20:04 ` Wendy Cheng 0 siblings, 1 reply; 14+ messages in thread From: Steven Whitehouse @ 2007-02-26 17:16 UTC (permalink / raw) To: cluster-devel.redhat.com Hi, On Mon, 2007-02-26 at 10:11 -0600, Jonathan E Brassow wrote: > Please note, I haven't been following current GFS2 development... > > On Feb 26, 2007, at 9:45 AM, Steven Whitehouse wrote: > > > Hi, > > > > On Fri, 2007-02-23 at 22:56 -0500, Wendy Cheng wrote: > >> Steve, > >> > >> I gave this issue some more thoughts - would like to suggest we take > >> this patch (at least for now) since it aligns with current code base. > >> > >> The no_formal_ino is apparently intented to get returned back to user > >> space due to its unique-ness (and we have to trust pick_formal_ino() > >> does its job right). With normal readdir system call, after the inode > >> number is sent to user space, there is no route (I've checked) for it > >> to > >> come back to kernel. So the only user that would use these filldir ino > >> inside the kernel is NFSD. NFSD calls gfs2_ilookup() that requires > >> no_formal_ino. > >> > > no_formal_ino is not ever guaranteed to be unique which is why I want > > to > > keep it away from userspace. I don't really want to have to change that > > in order to fix NFS. > > Why is it not unique? (from Ken's doc on GFS2: "If the filesystem > allocated 1 billion inodes per second, it would take over 500 years for > roll-over.") > Granted it will take a long time, but it does rather look a bit like another "640k will be enough for anyone" type prediction. The main problem here being that when it happens there will be no way to tell that it has happened. > > As you say there is no route for the inode number returned via readdir > > to come back to the kernel, but it should match the inode number > > returned by stat and I don't see that we should change either of them > > just to get NFS to work when userspace is perfectly ok as it is. > > There were alot of things done to make NFS work properly with GFS - > many of them through great pain, because GFS is a clustered FS. > > >> If you look further... The current lookup code actually uses > >> no_formal_ino, not no_addr. The two main "gate" routines that controls > >> ino-to-inode conversion are: > >> > >> * gfs2_ilookup() (used by NFS route) > >> * gfs2_inode_lookup() (used by VFS that calls gfs2_lookup()). > >> > > Neither of them need to use no_formal_ino at all. The lookup is using > > no_addr as there is no other index which makes sense. I know that > > no_formal_ino is used as part of the hash, but it shouldn't be really > > and its trivial to change that. You can't swap to just using > > no_formal_ino on its own as there is no mapping to find its on-disk > > location from no_formal_ino. > > > >> Both use no_formal_ino - gfs2_inode_lookup() logic hides this behind > >> the > >> little wrapper "gfs2_iget()". Since current VFS side's lookup has been > >> working fine, this no_formal_ino idea apparently is working. So let's > >> make NFSD side work the *same* way. I'm convinced this patch does a > >> right thing. > >> > >> I don't dispute using generation number may not be a bad idea and may > >> perform better. However, if we really look into the details, it is not > >> easy for current code structure. Since we have something already > >> working, it is not wise to mess this code layout at this moment. > >> > >> Make sense ? > >> > >> -- Wendy > >> > > I'm afraid that I'm still not convinced on that. It seems that ext3 > > uses > > the method that I'm proposing in that it looks up the inode by inode > > number and then only afterwards checks the generation number. It also > > has the advantage of separating the NFS interface from the rest of the > > filesystem. That seems to me to be a lot of the problem that we are > > looking at here in that the VFS's lookup function wants to look up by > > no_addr and (currently) NFS is asking for something slightly different, > > > > How do you plan on handling inode migration when using NFS? I believe > that was one of the central reasons for having no_formal_inode. There > may be other reasons as well, as I am not familiar with the code. > > brassow > The problem with no_formal_ino is that it doesn't allow inode migration since there is no index in which its possible to lookup no_formal_ino and find out the on-disk location of the inode. If we had such an index then I'd be suggesting using no_formal_ino and not no_addr as our inode number. There are other issues as well. The glock number is determined by no_addr (thats not something I've changed, btw) and in order to migrate an inode it really needs to be kept under the same lock, otherwise all kinds of odd things might happen. So it seemed easier to use the no_addr as the inode's unique number and try to keep everything consistent for now. This should then allow the addition of some kind of indirection layer later on, to allow migration. In the mean time no_formal_ino will still exist, its just that it won't be used as the lookup key, so theres no on-disk format change to consider here, all the same fields will continue to have the same values, so there should be no problem to use it again if required at a later date, Steve. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-26 17:16 ` Steven Whitehouse @ 2007-02-27 20:04 ` Wendy Cheng 2007-02-28 9:03 ` Steven Whitehouse 0 siblings, 1 reply; 14+ messages in thread From: Wendy Cheng @ 2007-02-27 20:04 UTC (permalink / raw) To: cluster-devel.redhat.com Steven Whitehouse wrote: > In the mean time no_formal_ino will still exist, its just that it won't > be used as the lookup key, so theres no on-disk format change to > consider here, all the same fields will continue to have the same > values, so there should be no problem to use it again if required at a > later date, > > ok, as long as we don't rush into removing the code, this is an acceptable plan. We can discuss the issue further when we meet face-to-face next month. In the mean time, will go ahead to make lookup code consistent by tentatively putting no_formal_ino aside. Without the changes, half of the current NFS lookups would fail. We need to have something working right now. I also invite Kent Baxley (one of our TAMs) here to help out with the issue. I recalled he mentioned sometime ago that the "create" table in GFS2-mySQL benchmark was noticeably slow - not sure whether pick_formal_ino() plays any role there. Will give him two gfs2.ko to re-run the benchmark and see what we get. -- Wendy ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-27 20:04 ` Wendy Cheng @ 2007-02-28 9:03 ` Steven Whitehouse 2007-02-28 16:24 ` Wendy Cheng 0 siblings, 1 reply; 14+ messages in thread From: Steven Whitehouse @ 2007-02-28 9:03 UTC (permalink / raw) To: cluster-devel.redhat.com Hi, On Tue, 2007-02-27 at 15:04 -0500, Wendy Cheng wrote: > Steven Whitehouse wrote: > > In the mean time no_formal_ino will still exist, its just that it won't > > be used as the lookup key, so theres no on-disk format change to > > consider here, all the same fields will continue to have the same > > values, so there should be no problem to use it again if required at a > > later date, > > > > > ok, as long as we don't rush into removing the code, this is an > acceptable plan. We can discuss the issue further when we meet > face-to-face next month. In the mean time, will go ahead to make lookup > code consistent by tentatively putting no_formal_ino aside. Without the > changes, half of the current NFS lookups would fail. We need to have > something working right now. > Yes, agreed. > I also invite Kent Baxley (one of our TAMs) here to help out with the > issue. I recalled he mentioned sometime ago that the "create" table in > GFS2-mySQL benchmark was noticeably slow - not sure whether > pick_formal_ino() plays any role there. Will give him two gfs2.ko to > re-run the benchmark and see what we get. > > -- Wendy > Probably it doesn't make much difference. I did some tests a while back when trying to work out why file creates are slow and I discovered that removing the pick_formal_ino code made no noticeable difference to the create speed, so the bottleneck appears to be elsewhere, Steve. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-28 9:03 ` Steven Whitehouse @ 2007-02-28 16:24 ` Wendy Cheng 2007-02-28 16:26 ` Wendy Cheng 2007-02-28 16:46 ` Steven Whitehouse 0 siblings, 2 replies; 14+ messages in thread From: Wendy Cheng @ 2007-02-28 16:24 UTC (permalink / raw) To: cluster-devel.redhat.com ok, the following is the minimum changes to get NFSD going before we settle down this issue .. would appreciate this in the tree so other NFS related works can get done in parallel. -- Wendy -------------- next part -------------- A non-text attachment was scrubbed... Name: gfs2_inum.patch Type: text/x-patch Size: 789 bytes Desc: not available URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20070228/689c764c/attachment.bin> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-28 16:24 ` Wendy Cheng @ 2007-02-28 16:26 ` Wendy Cheng 2007-02-28 16:46 ` Steven Whitehouse 1 sibling, 0 replies; 14+ messages in thread From: Wendy Cheng @ 2007-02-28 16:26 UTC (permalink / raw) To: cluster-devel.redhat.com Wendy Cheng wrote: > ok, the following is the minimum changes to get NFSD going before we > settle down this issue .. would appreciate this in the tree so other > NFS related works can get done in parallel. > Should have said this passes cthon04 test suites this morning... Wendy > > > > ------------------------------------------------------------------------ > > Signed-off-by: S. Wendy Cheng <wcheng@redhat.com> > > inode.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > --- linux-feb-17/fs/gfs2/inode.c 2007-02-17 20:07:16.000000000 -0500 > +++ up-kernel/fs/gfs2/inode.c 2007-02-28 11:34:26.000000000 -0500 > @@ -61,13 +61,13 @@ static int iget_set(struct inode *inode, > > struct inode *gfs2_ilookup(struct super_block *sb, struct gfs2_inum_host *inum) > { > - return ilookup5(sb, (unsigned long)inum->no_formal_ino, > + return ilookup5(sb, (unsigned long)inum->no_addr, > iget_test, inum); > } > > static struct inode *gfs2_iget(struct super_block *sb, struct gfs2_inum_host *inum) > { > - return iget5_locked(sb, (unsigned long)inum->no_formal_ino, > + return iget5_locked(sb, (unsigned long)inum->no_addr, > iget_test, iget_set, inum); > } > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main 2007-02-28 16:24 ` Wendy Cheng 2007-02-28 16:26 ` Wendy Cheng @ 2007-02-28 16:46 ` Steven Whitehouse 1 sibling, 0 replies; 14+ messages in thread From: Steven Whitehouse @ 2007-02-28 16:46 UTC (permalink / raw) To: cluster-devel.redhat.com Hi, Now in the git tree. Thanks, Steve. On Wed, 2007-02-28 at 11:24 -0500, Wendy Cheng wrote: > ok, the following is the minimum changes to get NFSD going before we > settle down this issue .. would appreciate this in the tree so other NFS > related works can get done in parallel. > > -- Wendy > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2007-02-28 16:46 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-02-07 4:51 [Cluster-devel] [GFS2 Patch] pass formal ino in do_filldir_main Wendy Cheng 2007-02-07 10:49 ` Steven Whitehouse 2007-02-07 22:35 ` Wendy Cheng 2007-02-07 23:37 ` Wendy Cheng 2007-02-08 9:42 ` Steven Whitehouse 2007-02-24 3:56 ` Wendy Cheng 2007-02-26 15:45 ` Steven Whitehouse 2007-02-26 16:11 ` Jonathan E Brassow 2007-02-26 17:16 ` Steven Whitehouse 2007-02-27 20:04 ` Wendy Cheng 2007-02-28 9:03 ` Steven Whitehouse 2007-02-28 16:24 ` Wendy Cheng 2007-02-28 16:26 ` Wendy Cheng 2007-02-28 16:46 ` Steven Whitehouse
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.