Lustre devel - Jan 2007 - [Bug 10796] dbench 16 oops with lustre exported over NFS

Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
https://bugzilla.lustre.org/show_bug.cgi?id=10796

           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #9319 is|0                           |1
           obsolete|                            |


Created an attachment (id=9341)
Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
 --> (https://bugzilla.lustre.org/attachment.cgi?id=9341&action=view)
minor changes

chages with last patch.
1. for set MDS_OPEN_OWNEROVERRIDE uses FMODE_WRITE instead of O_WRONLY.
2. few spaces cleanups.

Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
https://bugzilla.lustre.org/show_bug.cgi?id=10796

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #9341|review?(green@clusterfs.com)|review+
               Flag|                            |


(From update of attachment 9341)
my comments are pretty minor this time.
I decided to correct some of your comments in hopes it would be easier to read
them later.
The only big change I still think we might need is that d_rehash();d_unhash();
change to init_list_head
of some sort, may be with configure check. Otherwise is good.
>@@ -383,10 +383,15 @@ int ll_file_open(struct inode *inode, st
>-                if (oit.it_flags & O_CREAT)
>+                /* kernel only call f_op->open in dentry_open.
calls
>+                 * filp_open call dentry_open after open_namei where check
result
filp_open calls dentry_open after call to open_namei that checks for
permissions.
>+                 * permision call. only nfsd_open call dentry_open directly
without
nfsd_open calls...
>+                 * check permision and because it this check is safe.
checking permissions and because of that this code below is safe.
>@@ -492,8 +501,7 @@ out_och_free:
>                 }
>                 up(&lli->lli_och_sem);
>         }
>-
>-        return rc;
>+        RETURN(rc);
> }
this change is wrong. Please drop it, we cannot reach it without going through
GOTO() and thta will print rc
in the log already.
>@@ -475,7 +477,13 @@ static int lookup_it_finish(struct ptlrp
>                         ll_d_add(*de, inode);
>                         spin_unlock(&dcache_lock);
>                 } else {
>-                        (*de)->d_inode = NULL; 
>+                        /* Lustre don`t want hash dentry if don`t have
lock,
We do not want to hash the dentry is we don''t have a lock.
>+                         * but if this dentry used for d_move kernel got
But if this dentry is later used in d_move, we''d hit uninitialised list
head
>+                         * panic when tried access to dentry->d_hash and
d_hash is NULL.
d_hash, so we just do this to
>+                         * init d_hash field but leave dentry unhashed.
(bug 10796)
>+                         */
>+                        d_rehash(*de);
>+                        d_drop(*de);
Now, I understand that we are trying to avoid having a configure check(s) as
different kernels might
have this different, but taking dcache lock twice is not exactly cheap either.
So it worth at least putting a comment here that just initialiseing list head
is possible here.
(in case we ever face perf problems due to this later)
Amd just doing a list head init is probably still much more desirable even at
the expense of one more configure check.
>+static int ll_new_node(struct inode *dir, struct qstr *name,
>+                          char *tgt, int mode,
>+                          int rdev, struct dentry *dchild)
> {
...
>+
>+        if (dchild) {
>+                err = ll_prep_inode(sbi->ll_osc_exp, &inode,
request, 0,
>+                                    dchild->d_sb);
>+                if (err)
>+                     GOTO(err_exit, err);
>+
>+                d_drop(dchild);
>+                d_instantiate(dchild, inode);
>+                EXIT;
>+        }
you need to move that EXIT here, after the bracket.

Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
https://bugzilla.lustre.org/show_bug.cgi?id=10796

           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #9341 is|0                           |1
           obsolete|                            |


Created an attachment (id=9346)
Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
 --> (https://bugzilla.lustre.org/attachment.cgi?id=9346&action=view)
add changelog entry

Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
https://bugzilla.lustre.org/show_bug.cgi?id=10796



Created an attachment (id=9445)
Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
 --> (https://bugzilla.lustre.org/attachment.cgi?id=9445&action=view)
patch with fixes from b_releases_1_4_9

attaches patch includes two fixes founted while testing b_release_1_4_9 (and
landed into b_release_1_4_8).

Please don''t reply to lustre-devel. Instead, comment in Bugzilla by
using the following link:
https://bugzilla.lustre.org/show_bug.cgi?id=10796

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |stuart.mclaren@hp.com


*** Bug 11179 has been marked as a duplicate of this bug. ***