From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933951AbcJXHXc (ORCPT ); Mon, 24 Oct 2016 03:23:32 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:35231 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932597AbcJXHXa (ORCPT ); Mon, 24 Oct 2016 03:23:30 -0400 Date: Mon, 24 Oct 2016 16:23:25 +0900 From: Sergey Senozhatsky To: Minchan Kim Cc: Sergey Senozhatsky , Andrew Morton , Sergey Senozhatsky , linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/3] zram: support page-based parallel write Message-ID: <20161024072325.GC1855@swordfish> References: <1474526565-6676-1-git-send-email-minchan@kernel.org> <1474526565-6676-2-git-send-email-minchan@kernel.org> <20161021060334.GA527@swordfish> <20161024044714.GA4938@blaptop> <20161024052044.GA1855@swordfish> <20161024055818.GA5703@blaptop> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161024055818.GA5703@blaptop> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (10/24/16 14:58), Minchan Kim wrote: [..] > > struct blk_plug_cb *blk_check_plugged(blk_plug_cb_fn unplug, void *data, > > int size) > > { > > struct blk_plug *plug = current->plug; > > struct blk_plug_cb *cb; > > > > if (!plug) > > return NULL; > > > > list_for_each_entry(cb, &plug->cb_list, list) > > if (cb->callback == unplug && cb->data == data) > > return cb; > > Normally, this routine will check and bail out if it has been plugged > rightly so it would be not too many allocation in there. > > Having said that, there is no need to allocate cb in block layer. > drive can allocate one time and reuse it with passing it to the > blk_check_plugged. I was tempted to introduce the API into block layer > but it was just optimization/easy stuff once this patchset settle down > so I didn't consider in this patchset. aha. thanks. > > > We have been used sysfs for tune the zram for a long time. > > > Please suggest ideas if you have better. :) > > > > yeah, but this one feels like a super-hacky knob. basically > > > > "enable when you can't tweak your usage patterns. this will tweak the driver". > > > > so I'd probably prefer to keep it hidden for now (may be eventually > > we will come to some "out-of-zram" solution. but the opposition may > > be "fix your usage pattern"). > > Frankly speaking, I tend to agree. > > As I mentioned in cover-letter or somethine, I don't want to make this knob. > A option is we admit it's trade-off. So, if someone enables this config, > he will lost random/direct IO performance at this moment while he can get much > benefit buffered sequential read/write. > What do you think? yes, sounds like it. a config option, probably with a big-big warning sign and no sysfs knob. > > so this knob is not even guaranteed to be there all the time. > > > > I wish I could suggest any sound alternative, but I don't have one > > at the moment. May be I'll have a chance to speak to block-dev people > > next week. > > Okay. But I think it's not a good idea to hurt wb context you mentioned. > IOW, IO queuing could be parallelized by multiple wb context but > servicing(i.e., compression) should be done in zram contexts, not > wb context. yep. too many things can go wrong. we can schedule requests on a different die/package/socket, probably pressuring data caches and then there are NUMA systems, and so on and on and on. so I can easily imagine a "fix your user space" response. -ss