From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Snitzer Subject: Re: dm kcopyd: Increase sub-job size to 512KiB Date: Tue, 16 Jul 2019 10:14:15 -0400 Message-ID: <20190716141415.GB17023@redhat.com> References: <20190603134017.9323-1-ntsironis@arrikto.com> <20190715182221.GB15315@redhat.com> <54f185d8-5cf7-b617-1dfe-418da7643004@arrikto.com> <20190716141150.GA17023@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20190716141150.GA17023@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com To: Nikos Tsironis Cc: dm-devel@redhat.com, agk@redhat.com List-Id: dm-devel.ids On Tue, Jul 16 2019 at 10:11am -0400, Mike Snitzer wrote: > On Tue, Jul 16 2019 at 9:59am -0400, > Nikos Tsironis wrote: > > > On 7/15/19 9:22 PM, Mike Snitzer wrote: > > > On Fri, Jul 12 2019 at 9:45am -0400, > > > Nikos Tsironis wrote: > > > > > >> Hi Mike, > > >> > > >> A kind reminder about this patch. Do you require any changes or will you > > >> merge it as is? > > > > > > I think we need changes to expose knob(s) to tune this value on a global > > > _and_ device level via sysfs. E.g.: > > > > > > 1) dm_mod module param for global > > > 2) but also allow a per-device override, like: > > > echo 512 > /sys/block/dm-X/dm/kcopyd_subjob_size > > > > > > > Hi Mike, > > > > Thanks for your feedback. I agree, this sounds like the best thing to do. > > > > > 1 is super easy and is a start. Layering in 2 is a bit more involved. > > > > Maybe I could help with (2). We could discuss about it and how you think > > it's best to do it and then I could proceed with an implementation. > > > > Please let me know what you think. > > > > > > > > In hindsight (given how risk-averse I am on changing the default) I > > > should've kept the default 128 but allowed override with modparam > > > dm_mod.kcopyd_subjob_size=1024 > > > > > > Would this be an OK first step? > > > > Yes, this would be great. > > > > > > > > If so, we're still in the 5.3 merge window, I'll see what I can do. > > > > Shall I proceed with a patch adding the dm_mod.kcopyd_subjob_size > > modparam? > > Sure. And it could be that we won't need 2. > > Ideally the default would work for every setup. Less knobs the better. > But as a stop-gap I think we need to expose a knob that allows override. > > Thinking further, I don't think changing the default to 512K is too > risky (famous last words). So please just update your original patch to > include the modparam so that users can get the old 64K back if needed. > > BTW, the param name should probably be "kcopyd_subjob_size_kb" to > reflect the value is KB. One other thing: not sure what the max should be on this modparam.. maybe 1024K? Mike