We stopped dropping the lock when chunky scrub was added. I would prefer to not drop the lock. -Sam ----- Original Message ----- From: "Sage Weil" <sweil@xxxxxxxxxx> To: "Jianpeng Ma" <jianpeng.ma@xxxxxxxxx> Cc: sjust@xxxxxxxxxx, ceph-devel@xxxxxxxxxxxxxxx Sent: Tuesday, March 31, 2015 6:44:39 AM Subject: Re: Release pg.lock in build scrub map for primary On Tue, 31 Mar 2015, Ma, Jianpeng wrote: > Hi Sage, > We know scrub & handle normal osd ops is serial. Especially for deep-scrub, it need read all data, so it can block normal osd op for more time.. > >From the code, the most consume time function is PGBackend::be_scan_list. If for this section, we can release pg.lock and get the main aim. > > My analysis > 1: at this point, we already set scrub.start/end, so the normal write-order op about those objects will blocked. For PGBackend::be_scan_list, it's safe to release pg.lock. > 2:for _scan_rollback_obs, it need last_rollback_info_trimmed_to_applied. To be carefully, we can move _scan_rollback_obs before be_scan_list avoid some rollback objects removed. > 3:becasuse I don't familiar with snap. I don't sure is it safe for _scan_snaps which release and reget pg.lock? > > The fininal code like in build_scrub_map_chunk: > _scan_rollback_obs(rollback_obs, handle); > Pg.unlock() > get_pgbackend()->be_scan_list(map, ls, deep, seed, handle); > pg.lock() > _scan_snaps(map); I actually thought that scrub was doing the object listing without the lock... I think that was the case and perhaps the locking was added to avoid some race? Sam, do you remember? sage -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html