LU-13476 llite: Fix lock ordering in pagevec_dirty

[fs/lustre-release.git] / lustre / llite / vvp_io.c

diff --git a/lustre/llite/vvp_io.c b/lustre/llite/vvp_io.c
index 6ae1463..5fb6e13 100644 (file)
--- a/lustre/llite/vvp_io.c
+++ b/lustre/llite/vvp_io.c
@@ -653,7 +653,14 @@ static int vvp_io_setattr_lock(const struct lu_env *env,
        __u32 enqflags = 0;
 
        if (cl_io_is_trunc(io)) {
-               if (io->u.ci_setattr.sa_attr.lvb_size == 0)
+               struct inode *inode = vvp_object_inode(io->ci_obj);
+
+               /* set enqueue flags to CEF_MUST in case of encrypted file,
+                * to prevent lockless truncate
+                */
+               if (S_ISREG(inode->i_mode) && IS_ENCRYPTED(inode))
+                       enqflags = CEF_MUST;
+               else if (io->u.ci_setattr.sa_attr.lvb_size == 0)
                        enqflags = CEF_DISCARD_DATA;
        } else if (cl_io_is_fallocate(io)) {
                lock_start = io->u.ci_setattr.sa_falloc_offset;
@@ -731,13 +738,13 @@ static int vvp_io_setattr_start(const struct lu_env *env,
 
        if (cl_io_is_trunc(io)) {
                trunc_sem_down_write(&lli->lli_trunc_sem);
-               inode_lock(inode);
+               mutex_lock(&lli->lli_setattr_mutex);
                inode_dio_wait(inode);
        } else if (cl_io_is_fallocate(io)) {
                inode_lock(inode);
                inode_dio_wait(inode);
        } else {
-               inode_lock(inode);
+               mutex_lock(&lli->lli_setattr_mutex);
        }
 
        if (io->u.ci_setattr.sa_avalid & TIMES_SET_FLAGS)
@@ -757,12 +764,12 @@ static void vvp_io_setattr_end(const struct lu_env *env,
                /* Truncate in memory pages - they must be clean pages
                 * because osc has already notified to destroy osc_extents. */
                vvp_do_vmtruncate(inode, io->u.ci_setattr.sa_attr.lvb_size);
-               inode_unlock(inode);
+               mutex_unlock(&lli->lli_setattr_mutex);
                trunc_sem_up_write(&lli->lli_trunc_sem);
        } else if (cl_io_is_fallocate(io)) {
                inode_unlock(inode);
        } else {
-               inode_unlock(inode);
+               mutex_unlock(&lli->lli_setattr_mutex);
        }
 }
 
@@ -981,21 +988,33 @@ void vvp_set_pagevec_dirty(struct pagevec *pvec)
        struct page *page = pvec->pages[0];
        struct address_space *mapping = page->mapping;
        unsigned long flags;
+       unsigned long skip_pages = 0;
        int count = pagevec_count(pvec);
        int dirtied = 0;
-       int i = 0;
+       int i;
 
        ENTRY;
 
-       /* From set_page_dirty */
-       for (i = 0; i < count; i++)
-               ClearPageReclaim(pvec->pages[i]);
-
+       BUILD_BUG_ON(PAGEVEC_SIZE > BITS_PER_LONG);
        LASSERTF(page->mapping,
-                "mapping must be set. page %p, page->private (cl_page) %p",
+                "mapping must be set. page %p, page->private (cl_page) %p\n",
                 page, (void *) page->private);
 
-       /* Rest of code derived from __set_page_dirty_nobuffers */
+       for (i = 0; i < count; i++) {
+               page = pvec->pages[i];
+
+               ClearPageReclaim(page);
+
+               lock_page_memcg(page);
+               if (TestSetPageDirty(page)) {
+                       /* page is already dirty .. no extra work needed
+                        * set a flag for the i'th page to be skipped
+                        */
+                       unlock_page_memcg(page);
+                       skip_pages |= (1 << i);
+               }
+       }
+
        ll_xa_lock_irqsave(&mapping->i_pages, flags);
 
        /* Notes on differences with __set_page_dirty_nobuffers:
@@ -1006,17 +1025,13 @@ void vvp_set_pagevec_dirty(struct pagevec *pvec)
         * 3. No mapping is impossible. (Race w/truncate mentioned in
         * dirty_nobuffers should be impossible because we hold the page lock.)
         * 4. All mappings are the same because i/o is only to one file.
-        * 5. We invert the lock order on lock_page_memcg(page) and the mapping
-        * xa_lock, but this is the only function that should use that pair of
-        * locks and it can't race because Lustre locks pages throughout i/o.
         */
        for (i = 0; i < count; i++) {
                page = pvec->pages[i];
-               lock_page_memcg(page);
-               if (TestSetPageDirty(page)) {
-                       unlock_page_memcg(page);
+               /* if the i'th page was unlocked above, skip it here */
+               if ((skip_pages >> i) & 1)
                        continue;
-               }
+
                LASSERTF(page->mapping == mapping,
                         "all pages must have the same mapping.  page %p, mapping %p, first mapping %p\n",
                         page, page->mapping, mapping);