From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shawn Lin Subject: Re: 4.13 on thinkpad x220: oops when writing to SD card Date: Fri, 8 Sep 2017 10:51:49 +0800 Message-ID: References: <20170905194739.GA31241@amd> <8f0f7310-ea4d-a200-75fd-23509947fb38@rock-chips.com> <6689241f-a4d8-7a3e-9f0b-482b034e5710@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Linus Walleij , Ulf Hansson Cc: shawn.lin@rock-chips.com, Adrian Hunter , Pavel Machek , "linux-mmc@vger.kernel.org" , kernel list , Seraphime Kirkovski List-Id: linux-mmc@vger.kernel.org On 2017/9/8 4:02, Linus Walleij wrote: > On Thu, Sep 7, 2017 at 9:18 AM, Ulf Hansson wrote: > >> Even if this fixes the problem it seems like we are papering over the >> real issue, which earlier fixes also did during the release cycle for >> v4.13. > > I think this is the real solution to the issue. > >>> Another unrelated issue with mmc_init_request() is that mmc_exit_request() >>> is not called if mmc_init_request() fails, which means mmc_init_request() >>> must free anything it allocates when it fails. >> >> Yes, the situations it's just too fragile. We need to fix the behavior >> properly, although I haven't myself been able to investigate exactly >> how yet. >> >> Adding, Linus, perhaps he has some ideas. > > Maybe we should simply bite the bullet and do what was suggested > by another contributor when I refactored the bounce buffer handling: > simply delete the bounce buffer code and let any remaining (few?) > legacy devices suffer a bit (performancewise) at the gain of way > simpler code? Are you in the same page with what Adrian pointed to? IIUC, the issue is: init_rq_fn will be called each time when trying to create new reuqest from the pre-allocated request_list memeory pool, and exit_rq_fn will is in the corresponding routine to free request from request_list (blk_free_request) when finished. But if alloc_request_size fails, it won't call exit_rq_fn, so you need to prevent memory leak on your own error path of init_rq_fn. But you seem to talk about removing the bounce buffer and so finally get rid of registering init_rq_fn/exit_rq_fn? That is another thing, and what we right now need to do is to fix the pontential memory leak. It's quite a simple action, right? :) > > I am a bit hesitant about that because Pierre Ossman said it was > actually a big win on the SDHC hosts that made use of it at one > point. You had removed packed cmd support to simplify the code, so I think this is another trade-off need to ask: What you want? and keep consistent with the direction you insisted on. > > Yours, > Linus Walleij > > >