Whamcloud - gitweb
LU-2720 clio: remove CILR_PEEK optimization
authorVitaly Fertman <vitaly_fertman@xyratex.com>
Thu, 31 Jan 2013 12:40:07 +0000 (16:40 +0400)
committerOleg Drokin <oleg.drokin@intel.com>
Fri, 22 Feb 2013 22:11:16 +0000 (17:11 -0500)
some issues with that optimization in mkwrite are found:
1) ENODATA handling wrote the page and invalidated it, but the page
could have been re-added to the cache in between these 2 steps;
2) lock could be busy, so we get ENODATA on trylock, whereas a racy
ll_fault waits for lock mutex and re-adds the page to the cache;

The patch also adds a loop over trying to take a cached lock while
it is used.

Signed-off-by: Vitaly Fertman <vitaly_fertman@xyratex.com>
Change-Id: I7799b45ff3a0a60f307e05ffd1618d23e0e77b72
Xyratex-bug-id: MRP-752
Reviewed-on: http://review.whamcloud.com/5222
Tested-by: Hudson
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Jinshan Xiong <jinshan.xiong@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
lustre/include/cl_object.h
lustre/llite/llite_mmap.c
lustre/obdclass/cl_io.c

index 087e251..f43bf17 100644 (file)
@@ -2275,9 +2275,7 @@ enum cl_io_lock_dmd {
         /** Layers are free to decide between local and global locking. */
         CILR_MAYBE,
         /** Never lock: there is no cache (e.g., liblustre). */
         /** Layers are free to decide between local and global locking. */
         CILR_MAYBE,
         /** Never lock: there is no cache (e.g., liblustre). */
-        CILR_NEVER,
-        /** Peek lock: use existing locks, don't queue new ones */
-        CILR_PEEK
+        CILR_NEVER
 };
 
 enum cl_fsync_mode {
 };
 
 enum cl_fsync_mode {
index 52c8972..febad1d 100644 (file)
@@ -192,11 +192,6 @@ static int ll_page_mkwrite0(struct vm_area_struct *vma, struct page *vmpage,
        if (result < 0)
                GOTO(out, result);
 
        if (result < 0)
                GOTO(out, result);
 
-       /* Don't enqueue new locks for page_mkwrite().
-        * If the lock has been cancelled then page must have been
-        * truncated, in that case, kernel will handle it.
-        */
-       io->ci_lockreq = CILR_PEEK;
        io->u.ci_fault.ft_mkwrite = 1;
        io->u.ci_fault.ft_writable = 1;
 
        io->u.ci_fault.ft_mkwrite = 1;
        io->u.ci_fault.ft_writable = 1;
 
@@ -219,11 +214,7 @@ static int ll_page_mkwrite0(struct vm_area_struct *vma, struct page *vmpage,
 
        cfs_restore_sigs(set);
 
 
        cfs_restore_sigs(set);
 
-        if (result == -ENODATA) /* peek failed, no lock caching. */
-                CDEBUG(D_MMAP, "race on page_mkwrite: %lx (%lu %p)\n",
-                       vma->vm_flags, io->u.ci_fault.ft_index, vmpage);
-
-        if (result == 0 || result == -ENODATA) {
+        if (result == 0) {
                struct inode *inode = vma->vm_file->f_dentry->d_inode;
                struct ll_inode_info *lli = ll_i2info(inode);
 
                struct inode *inode = vma->vm_file->f_dentry->d_inode;
                struct ll_inode_info *lli = ll_i2info(inode);
 
@@ -236,18 +227,6 @@ static int ll_page_mkwrite0(struct vm_area_struct *vma, struct page *vmpage,
                          * to handle_mm_fault(). */
                         if (result == 0)
                                 result = -ENODATA;
                          * to handle_mm_fault(). */
                         if (result == 0)
                                 result = -ENODATA;
-                } else if (result == -ENODATA) {
-                        /* Invalidate it if the cl_lock is being revoked.
-                         * This piece of code is definitely needed for RHEL5,
-                         * otherwise, SIGBUS will be wrongly returned to
-                         * applications. */
-                        write_one_page(vmpage, 1);
-                        lock_page(vmpage);
-                        if (vmpage->mapping != NULL) {
-                                ll_invalidate_page(vmpage);
-                                LASSERT(vmpage->mapping == NULL);
-                        }
-                        unlock_page(vmpage);
                 } else if (!PageDirty(vmpage)) {
                         /* race, the page has been cleaned by ptlrpcd after
                          * it was unlocked, it has to be added into dirty
                 } else if (!PageDirty(vmpage)) {
                         /* race, the page has been cleaned by ptlrpcd after
                          * it was unlocked, it has to be added into dirty
index aab7682..bcb0ad3 100644 (file)
@@ -390,12 +390,7 @@ static int cl_lockset_lock_one(const struct lu_env *env,
 
         ENTRY;
 
 
         ENTRY;
 
-        if (io->ci_lockreq == CILR_PEEK) {
-                lock = cl_lock_peek(env, io, &link->cill_descr, "io", io);
-                if (lock == NULL)
-                        lock = ERR_PTR(-ENODATA);
-        } else
-                lock = cl_lock_request(env, io, &link->cill_descr, "io", io);
+       lock = cl_lock_request(env, io, &link->cill_descr, "io", io);
 
         if (!IS_ERR(lock)) {
                 link->cill_lock = lock;
 
         if (!IS_ERR(lock)) {
                 link->cill_lock = lock;