From: John Spray <john.spray@redhat.com>
Date: Mon, 19 Sep 2016 16:35:54 +0000 (+0100)
Subject: mds: remove redundant checks for null ScrubHeader
X-Git-Tag: v11.0.1~58^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=0c890282699f877f42870408d674ec1e9f9322a3;p=ceph.git

mds: remove redundant checks for null ScrubHeader

This was originally optional but now all the paths
that kick off a scrub should be going through
enqueue_scrub and thereby getting a header set.

Signed-off-by: John Spray <john.spray@redhat.com>
---

diff --git a/src/mds/CDir.cc b/src/mds/CDir.cc
index 8f325011a5f1..2d6025f2d68e 100644
--- a/src/mds/CDir.cc
+++ b/src/mds/CDir.cc
@@ -2932,6 +2932,7 @@ void CDir::scrub_initialize(const ScrubHeaderRefConst& header)
 {
   dout(20) << __func__ << dendl;
   assert(is_complete());
+  assert(header != nullptr);
 
   // FIXME: weird implicit construction, is someone else meant
   // to be calling scrub_info_create first?
@@ -3019,7 +3020,7 @@ int CDir::_next_dentry_on_set(set<dentry_key_t>& dns, bool missing_okay,
     dns.erase(dnkey);
 
     if (dn->get_projected_version() < scrub_infop->last_recursive.version &&
-	!(scrub_infop->header && scrub_infop->header->force)) {
+	!(scrub_infop->header->force)) {
       dout(15) << " skip dentry " << dnkey.name
 	       << ", no change since last scrub" << dendl;
       continue;
@@ -3125,8 +3126,7 @@ bool CDir::scrub_local()
     scrub_infop->last_scrub_dirty = true;
   } else {
     scrub_infop->pending_scrub_error = true;
-    if (scrub_infop->header &&
-	scrub_infop->header->repair)
+    if (scrub_infop->header->repair)
       cache->repair_dirfrag_stats(this);
   }
   return rval;
diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc
index 9138a0e1622a..e878a26ba351 100644
--- a/src/mds/CInode.cc
+++ b/src/mds/CInode.cc
@@ -3785,7 +3785,7 @@ void CInode::validate_disk_state(CInode::validated_data *results,
 
       // Whether we have a tag to apply depends on ScrubHeader (if one is
       // present)
-      if (in->scrub_infop && in->scrub_infop->header) {
+      if (in->scrub_infop) {
         // I'm a non-orphan, so look up my ScrubHeader via my linkage
         const std::string &tag = in->scrub_infop->header->tag;
         // Rather than using the usual CInode::fetch_backtrace,
@@ -3867,7 +3867,7 @@ next:
           clog->error() << "scrub: inode wrongly marked free: 0x" << std::hex
             << inode.ino;
 
-          if (in->scrub_infop->header && in->scrub_infop->header->repair) {
+          if (in->scrub_infop->header->repair) {
             bool repaired = inotable->repair(inode.ino);
             if (repaired) {
               clog->error() << "inode table repaired for inode: 0x" << std::hex
@@ -3942,7 +3942,7 @@ next:
           ++p) {
         CDir *dir = in->get_or_open_dirfrag(in->mdcache, *p);
 	dir->scrub_info();
-	if (!dir->scrub_infop->header && in->scrub_infop)
+	if (!dir->scrub_infop->header)
 	  dir->scrub_infop->header = in->scrub_infop->header;
         if (dir->is_complete()) {
 	  dir->scrub_local();
@@ -3987,8 +3987,7 @@ next:
 	if (dir->scrub_infop &&
 	    dir->scrub_infop->pending_scrub_error) {
 	  dir->scrub_infop->pending_scrub_error = false;
-	  if (dir->scrub_infop->header &&
-	      dir->scrub_infop->header->repair) {
+	  if (dir->scrub_infop->header->repair) {
 	    results->raw_stats.error_str
 	      << "dirfrag(" << p->first << ") has bad stats (will be fixed); ";
 	  } else {
@@ -4003,7 +4002,6 @@ next:
       if (!dir_info.same_sums(in->inode.dirstat) ||
 	  !nest_info.same_sums(in->inode.rstat)) {
 	if (in->scrub_infop &&
-	    in->scrub_infop->header &&
 	    in->scrub_infop->header->repair) {
 	  results->raw_stats.error_str
 	    << "freshly-calculated rstats don't match existing ones (will be fixed)";
@@ -4344,7 +4342,7 @@ void CInode::scrub_finished(MDSInternalContextBase **c) {
   *c = scrub_infop->on_finish;
   scrub_infop->on_finish = NULL;
 
-  if (scrub_infop->header && scrub_infop->header->origin == this) {
+  if (scrub_infop->header->origin == this) {
     // We are at the point that a tagging scrub was initiated
     LogChannelRef clog = mdcache->mds->clog;
     clog->info() << "scrub complete with tag '" << scrub_infop->header->tag << "'";
diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index cbb9147fc2c4..7d03cf928120 100644
--- a/src/mds/MDCache.cc
+++ b/src/mds/MDCache.cc
@@ -11905,9 +11905,11 @@ void MDCache::enqueue_scrub_work(MDRequestRef& mdr)
   ScrubHeaderRef &header = cs->header;
 
   // Cannot scrub same dentry twice at same time
-  if (in->scrub_info()->scrub_in_progress) {
+  if (in->scrub_infop && in->scrub_infop->scrub_in_progress) {
     mds->server->respond_to_request(mdr, -EBUSY);
     return;
+  } else {
+    in->scrub_info();
   }
 
   header->origin = in;
diff --git a/src/mds/ScrubHeader.h b/src/mds/ScrubHeader.h
index 72231b5888c7..c5939304d4fd 100644
--- a/src/mds/ScrubHeader.h
+++ b/src/mds/ScrubHeader.h
@@ -7,9 +7,6 @@ class CInode;
 /**
  * Externally input parameters for a scrub, associated with the root
  * of where we are doing a recursive scrub
- *
- * TODO: swallow up 'recurse' and 'children' settings here instead of
- * passing them down into every scrub_info structure
  */
 class ScrubHeader {
 public: