Whamcloud - gitweb
LU-2576 osc: various fix in osc_enter_cache()
authorNiu Yawei <niu@whamcloud.com>
Sun, 6 Jan 2013 08:21:03 +0000 (03:21 -0500)
committerOleg Drokin <green@whamcloud.com>
Thu, 31 Jan 2013 03:19:01 +0000 (22:19 -0500)
- It should not go to sleep if there isn't any inflight write
  RPC, otherwise, it could not be waked up for a long time,
  until dirty flush triggered on other OSCs or other objects.

- If the page can't be granted due to too many dirty pages
  (> obd_max_dirty_pages), osc_enter_cache() should return
  -EDQUOT.

- It should pass NULL osc object to  osc_io_unplug(), otherwise
  dirty flush can't be triggered if the passed object is
  clean.

- It should not wait in a loop of "while (cli->cl_dirty > 0)",
  trigger the dirty flush once is enough. If there are too
  many threads that consumed grants immediately, then we
  should turn to sync write, but not trigger flush and wait
  for grant again (that'll block io process on grant).

- Refuse setting max_dirty_mb as 0 via proc file.

Signed-off-by: Niu Yawei <yawei.niu@intel.com>
Change-Id: I449cba07bd427749ab023c249d9e200aba1b406a
Reviewed-on: http://review.whamcloud.com/4963
Tested-by: Hudson
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Prakash Surya <surya1@llnl.gov>
Reviewed-by: Jinshan Xiong <jinshan.xiong@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/osc/lproc_osc.c
lustre/osc/osc_cache.c

index cf472d1..29fbebc 100644 (file)
@@ -147,7 +147,7 @@ static int osc_wr_max_dirty_mb(struct file *file, const char *buffer,
         if (rc)
                 return rc;
 
-        if (pages_number < 0 ||
+        if (pages_number <= 0 ||
             pages_number > OSC_MAX_DIRTY_MB_MAX << (20 - CFS_PAGE_SHIFT) ||
             pages_number > cfs_num_physpages / 4) /* 1/4 of RAM */
                 return -ERANGE;
index ebdb17b..fc896eb 100644 (file)
@@ -1483,6 +1483,15 @@ static int osc_enter_cache_try(struct client_obd *cli,
        return rc;
 }
 
+static int ocw_granted(struct client_obd *cli, struct osc_cache_waiter *ocw)
+{
+       int rc;
+       client_obd_list_lock(&cli->cl_loi_list_lock);
+       rc = cfs_list_empty(&ocw->ocw_entry) || cli->cl_w_in_flight == 0;
+       client_obd_list_unlock(&cli->cl_loi_list_lock);
+       return rc;
+}
+
 /**
  * The main entry to reserve dirty page accounting. Usually the grant reserved
  * in this function will be freed in bulk in osc_free_grant() unless it fails
@@ -1524,34 +1533,47 @@ static int osc_enter_cache(const struct lu_env *env, struct client_obd *cli,
        cfs_waitq_init(&ocw.ocw_waitq);
        ocw.ocw_oap   = oap;
        ocw.ocw_grant = bytes;
-       while (cli->cl_dirty > 0 || cli->cl_w_in_flight > 0) {
+       if (cli->cl_dirty > 0 || cli->cl_w_in_flight > 0) {
                cfs_list_add_tail(&ocw.ocw_entry, &cli->cl_cache_waiters);
                ocw.ocw_rc = 0;
                client_obd_list_unlock(&cli->cl_loi_list_lock);
 
+               /* First osc_io_unplug() tries to put current object
+                * on ready list, second osc_io_unplug() makes sure that
+                * dirty flush can still be triggered even if current
+                * object hasn't any dirty pages */
                osc_io_unplug(env, cli, osc, PDL_POLICY_ROUND);
+               osc_io_unplug(env, cli, NULL, PDL_POLICY_ROUND);
 
                CDEBUG(D_CACHE, "%s: sleeping for cache space @ %p for %p\n",
                       cli->cl_import->imp_obd->obd_name, &ocw, oap);
 
-               rc = l_wait_event(ocw.ocw_waitq,
-                                 cfs_list_empty(&ocw.ocw_entry), &lwi);
+               rc = l_wait_event(ocw.ocw_waitq, ocw_granted(cli, &ocw), &lwi);
 
                client_obd_list_lock(&cli->cl_loi_list_lock);
-               cfs_list_del_init(&ocw.ocw_entry);
-               if (rc < 0)
-                       break;
 
-               rc = ocw.ocw_rc;
+               /* l_wait_event is interrupted by signal */
+               if (rc < 0) {
+                       cfs_list_del_init(&ocw.ocw_entry);
+                       GOTO(out, rc);
+               }
+
+               /* If ocw_entry isn't empty, which means it's not waked up
+                * by osc_wake_cache_waiters(), then the page must not be
+                * granted yet. */
+               if (!cfs_list_empty(&ocw.ocw_entry)) {
+                       rc = -EDQUOT;
+                       cfs_list_del_init(&ocw.ocw_entry);
+               } else {
+                       rc = ocw.ocw_rc;
+               }
+
                if (rc != -EDQUOT)
-                       break;
-               if (osc_enter_cache_try(cli, oap, bytes, 0)) {
+                       GOTO(out, rc);
+               if (osc_enter_cache_try(cli, oap, bytes, 0))
                        rc = 0;
-                       break;
-               }
        }
        EXIT;
-
 out:
        client_obd_list_unlock(&cli->cl_loi_list_lock);
        OSC_DUMP_GRANT(cli, "returned %d.\n", rc);