From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Snitzer Subject: Re: staged dm_internal_{suspend, resume} related changes for wider review Date: Wed, 5 Nov 2014 11:29:12 -0500 Message-ID: <20141105162912.GA17625@redhat.com> References: <20141028232638.GB29288@redhat.com> <20141029002125.GC29288@redhat.com> <20141029012256.GF29288@redhat.com> <20141105011619.GA11411@redhat.com> <20141105141147.GA14285@redhat.com> Reply-To: device-mapper development Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com To: Mikulas Patocka Cc: device-mapper development , "Bryn M. Reeves" , ejt@redhat.com, Alasdair G Kergon , Zdenek Kabelac List-Id: dm-devel.ids On Wed, Nov 05 2014 at 9:37am -0500, Mikulas Patocka wrote: > The patch series introduces two suspend mechanisms and it is unclear how > should they interact with each other. And this point is not correct. As you know dm_internal_suspend and dm_internal_resume interface predates any of my changes. That existing interface was extended them to be (mostly) fully formed equivalents of dm_suspend() and dm_resume(). I say "mostly" because dm_internal_resume() doesn't call into the targets' resume hooks because no existing callers (dm-stats or dm-thinp) need to. But obviously dm_resume() does need to so it passes @resume_targets as true to __dm_resume(). I'm not trying to suggest there is a bug or bugs in this new code (you already pointed out the locking issue across ioctls that I fixed). But a bug doesn't implicitly mean this is an imperfect way forward -- if/when a bug is found we'll deal with it.. so feel free to pour over this code to see if there is a bug or bugs. I really do welcome your review -- I would just like technical issues to be the focus of any technical review.