Whamcloud - gitweb
LU-2360 ptlrpc: a few fixes for req_history read
authorLiang Zhen <liang@whamcloud.com>
Sun, 23 Dec 2012 09:53:29 +0000 (17:53 +0800)
committerOleg Drokin <green@whamcloud.com>
Tue, 8 Jan 2013 04:56:37 +0000 (23:56 -0500)
This patch fixed a few issues in seq_file operations of ptlrpc
request history:
- start() function always start from CPT 0 despite of start position,
  this might cause endless read.
- ptlrpc history sequence can't be directly used as read position of
  seq_file, because seq_read() can increase position to indicate
  reading of the next element, this will actually change CPT ID of
  sequence, which means it's possible that next() will never be able
  to finish iteration of requests on a CPT in some case.

Signed-off-by: Liang Zhen <liang@whamcloud.com>
Change-Id: I3ee4e06d3b93dcc61bfd910725d4cab87b555511
Reviewed-on: http://review.whamcloud.com/4888
Tested-by: Hudson
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: wangdi <di.wang@intel.com>
lustre/ptlrpc/lproc_ptlrpc.c

index 6282c28..ab7d146 100644 (file)
@@ -428,7 +428,10 @@ ptlrpc_lprocfs_svc_req_history_seek(struct ptlrpc_service_part *svcpt,
                  * Since the service history is LRU (i.e. culled reqs will
                  * be near the head), we shouldn't have to do long
                  * re-scans */
                  * Since the service history is LRU (i.e. culled reqs will
                  * be near the head), we shouldn't have to do long
                  * re-scans */
-                LASSERT (srhi->srhi_seq == srhi->srhi_req->rq_history_seq);
+               LASSERTF(srhi->srhi_seq == srhi->srhi_req->rq_history_seq,
+                        "%s:%d: seek seq "LPU64", request seq "LPU64"\n",
+                        svcpt->scp_service->srv_name, svcpt->scp_cpt,
+                        srhi->srhi_seq, srhi->srhi_req->rq_history_seq);
                LASSERTF(!cfs_list_empty(&svcpt->scp_hist_reqs),
                         "%s:%d: seek offset "LPU64", request seq "LPU64", "
                         "last culled "LPU64"\n",
                LASSERTF(!cfs_list_empty(&svcpt->scp_hist_reqs),
                         "%s:%d: seek offset "LPU64", request seq "LPU64", "
                         "last culled "LPU64"\n",
@@ -454,15 +457,55 @@ ptlrpc_lprocfs_svc_req_history_seek(struct ptlrpc_service_part *svcpt,
         return -ENOENT;
 }
 
         return -ENOENT;
 }
 
+/*
+ * ptlrpc history sequence is used as "position" of seq_file, in some case,
+ * seq_read() will increase "position" to indicate reading the next
+ * element, however, low bits of history sequence are reserved for CPT id
+ * (check the details from comments before ptlrpc_req_add_history), which
+ * means seq_read() might change CPT id of history sequence and never
+ * finish reading of requests on a CPT. To make it work, we have to shift
+ * CPT id to high bits and timestamp to low bits, so seq_read() will only
+ * increase timestamp which can correctly indicate the next position.
+ */
+
+/* convert seq_file pos to cpt */
+#define PTLRPC_REQ_POS2CPT(svc, pos)                   \
+       ((svc)->srv_cpt_bits == 0 ? 0 :                 \
+        (__u64)(pos) >> (64 - (svc)->srv_cpt_bits))
+
+/* make up seq_file pos from cpt */
+#define PTLRPC_REQ_CPT2POS(svc, cpt)                   \
+       ((svc)->srv_cpt_bits == 0 ? 0 :                 \
+        (cpt) << (64 - (svc)->srv_cpt_bits))
+
+/* convert sequence to position */
+#define PTLRPC_REQ_SEQ2POS(svc, seq)                   \
+       ((svc)->srv_cpt_bits == 0 ? (seq) :             \
+        ((seq) >> (svc)->srv_cpt_bits) |               \
+        ((seq) << (64 - (svc)->srv_cpt_bits)))
+
+/* convert position to sequence */
+#define PTLRPC_REQ_POS2SEQ(svc, pos)                   \
+       ((svc)->srv_cpt_bits == 0 ? (pos) :             \
+        ((__u64)(pos) << (svc)->srv_cpt_bits) |        \
+        ((__u64)(pos) >> (64 - (svc)->srv_cpt_bits)))
+
 static void *
 ptlrpc_lprocfs_svc_req_history_start(struct seq_file *s, loff_t *pos)
 {
        struct ptlrpc_service           *svc = s->private;
        struct ptlrpc_service_part      *svcpt;
        struct ptlrpc_srh_iterator      *srhi;
 static void *
 ptlrpc_lprocfs_svc_req_history_start(struct seq_file *s, loff_t *pos)
 {
        struct ptlrpc_service           *svc = s->private;
        struct ptlrpc_service_part      *svcpt;
        struct ptlrpc_srh_iterator      *srhi;
+       unsigned int                    cpt;
        int                             rc;
        int                             i;
 
        int                             rc;
        int                             i;
 
+       if (sizeof(loff_t) != sizeof(__u64)) { /* can't support */
+               CWARN("Failed to read request history because size of loff_t "
+                     "%d can't match size of u64\n", (int)sizeof(loff_t));
+               return NULL;
+       }
+
        OBD_ALLOC(srhi, sizeof(*srhi));
        if (srhi == NULL)
                return NULL;
        OBD_ALLOC(srhi, sizeof(*srhi));
        if (srhi == NULL)
                return NULL;
@@ -470,14 +513,21 @@ ptlrpc_lprocfs_svc_req_history_start(struct seq_file *s, loff_t *pos)
        srhi->srhi_seq = 0;
        srhi->srhi_req = NULL;
 
        srhi->srhi_seq = 0;
        srhi->srhi_req = NULL;
 
+       cpt = PTLRPC_REQ_POS2CPT(svc, *pos);
+
        ptlrpc_service_for_each_part(svcpt, i, svc) {
        ptlrpc_service_for_each_part(svcpt, i, svc) {
-               srhi->srhi_idx = i;
+               if (i < cpt) /* skip */
+                       continue;
+               if (i > cpt) /* make up the lowest position for this CPT */
+                       *pos = PTLRPC_REQ_CPT2POS(svc, i);
 
                spin_lock(&svcpt->scp_lock);
 
                spin_lock(&svcpt->scp_lock);
-               rc = ptlrpc_lprocfs_svc_req_history_seek(svcpt, srhi, *pos);
+               rc = ptlrpc_lprocfs_svc_req_history_seek(svcpt, srhi,
+                               PTLRPC_REQ_POS2SEQ(svc, *pos));
                spin_unlock(&svcpt->scp_lock);
                if (rc == 0) {
                spin_unlock(&svcpt->scp_lock);
                if (rc == 0) {
-                       *pos = srhi->srhi_seq;
+                       *pos = PTLRPC_REQ_SEQ2POS(svc, srhi->srhi_seq);
+                       srhi->srhi_idx = i;
                        return srhi;
                }
        }
                        return srhi;
                }
        }
@@ -502,28 +552,32 @@ ptlrpc_lprocfs_svc_req_history_next(struct seq_file *s,
        struct ptlrpc_service           *svc = s->private;
        struct ptlrpc_srh_iterator      *srhi = iter;
        struct ptlrpc_service_part      *svcpt;
        struct ptlrpc_service           *svc = s->private;
        struct ptlrpc_srh_iterator      *srhi = iter;
        struct ptlrpc_service_part      *svcpt;
-       int                             rc = 0;
+       __u64                           seq;
+       int                             rc;
        int                             i;
 
        for (i = srhi->srhi_idx; i < svc->srv_ncpts; i++) {
                svcpt = svc->srv_parts[i];
 
        int                             i;
 
        for (i = srhi->srhi_idx; i < svc->srv_ncpts; i++) {
                svcpt = svc->srv_parts[i];
 
-               srhi->srhi_idx = i;
+               if (i > srhi->srhi_idx) { /* reset iterator for a new CPT */
+                       srhi->srhi_req = NULL;
+                       seq = srhi->srhi_seq = 0;
+               } else { /* the next sequence */
+                       seq = srhi->srhi_seq + (1 << svc->srv_cpt_bits);
+               }
 
                spin_lock(&svcpt->scp_lock);
 
                spin_lock(&svcpt->scp_lock);
-               rc = ptlrpc_lprocfs_svc_req_history_seek(svcpt, srhi, *pos + 1);
+               rc = ptlrpc_lprocfs_svc_req_history_seek(svcpt, srhi, seq);
                spin_unlock(&svcpt->scp_lock);
                spin_unlock(&svcpt->scp_lock);
-               if (rc == 0)
-                       break;
+               if (rc == 0) {
+                       *pos = PTLRPC_REQ_SEQ2POS(svc, srhi->srhi_seq);
+                       srhi->srhi_idx = i;
+                       return srhi;
+               }
        }
 
        }
 
-        if (rc != 0) {
-                OBD_FREE(srhi, sizeof(*srhi));
-                return NULL;
-        }
-
-        *pos = srhi->srhi_seq;
-        return srhi;
+       OBD_FREE(srhi, sizeof(*srhi));
+       return NULL;
 }
 
 /* common ost/mdt so_req_printer */
 }
 
 /* common ost/mdt so_req_printer */