| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 3 Oct 2013 16:48:15 -0700 From: Grant Grundler <grundler@...omium.org> To: Ulf Hansson <ulf.hansson@...aro.org> Cc: Grant Grundler <grundler@...omium.org>, "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>, Seungwon Jeon <tgih.jun@...sung.com>, LKML <linux-kernel@...r.kernel.org>, Chris Ball <cjb@...top.org>, Jaehoon Chung <jh80.chung@...sung.co>, ALIM AKHTAR <alim.akhtar@...sung.com> Subject: Re: [PATCH 0/7] mmc: core: cleanup and locking patches description On Thu, Oct 3, 2013 at 2:35 PM, Ulf Hansson <ulf.hansson@...aro.org> wrote: > > Den 3 okt 2013 22:35 skrev "Grant Grundler" <grundler@...omium.org>: > > >> >> Ping? >> >> Has anyone had a chance to review/test this series and/or be willing >> to do so this week? > > Hi Grant, > > It is on my todo list. My plan is to check the behavior on ux500 board, > which uses mmci and which has been one of the first drivers that utilizes > the async request mechanism. Thanks Ulf for the update. dw_mmc is the only driver that I can see that uses a tasklet (Soft Interrupt context) to handle completions. So it's "timing" will be unique and nothing it calls can acquire semaphores in that context. :/ > I am not that keen of the patches that renames stuff though, not sure it > actually improves anything. I do agree the name changes are secondary to the locking changes. Names matter. "data" doesn't say anything about how the variable is expected to be used. And "err" doesn't suggest this variable is ONLY related to the completion, not the starting of a new async request. mmc_start_req() is very confusing since it's very different from mmc_start_request. At a minimum, mmc_start_req() should be renamed to "<mumble>_areq" to be clear it's not dealing with synchronous requests at all. So yes, renaming stuff can improve what developers expect from the code. > This week is not possible, but likely the next. Ok! Thanks for letting me know you plan on looking at it. I can wait. If necessary, I will poke you again next week. :) thank you! grant > > Kind regards > Ulf Hanson > >> >> grundler@...esword:~$ pwclient list -w 'grundler@...omium.org' -p LKML >> Patches submitted by Grant Grundler <grundler@...omium.org>: >> ID State Name >> -- ----- ---- >> ... >> 2950961 New [1/7] mmc: core: rename "data" to saved_areq >> 2951081 New [2/7] mmc: core: rename local var err to saved_err >> 2950981 New [3/7] mmc: core: restructure error handling for start >> req >> 2951001 New [4/7] mmc: core: use common code path to return error >> 2951061 New [5/7] mmc: core: handling polling more gracefully >> 2951041 New [6/7] mmc: core: protect references to host->areq >> with host->lock >> 2951021 New [7/7] mmc: core: mmc_start_req is a misnomer -> >> mmc_process_areq >> >> >> To remind, this is the fio command line that results in mmcqd hangs on >> the exynos 5250 system (uses dw_mmc storage controller): >> >> $FIO --name=short_randwrite --size=2G --time_based --runtime=90 \ >> --readwrite=randwrite --numjobs=2 --bs=4k --norandommap >> --ioengine=psync \ >> --direct=0 --invalidate=1 --filename=/dev/mmcblk0p5 >> >> thanks, >> grant >> >> On Thu, Sep 26, 2013 at 12:57 PM, Grant Grundler <grundler@...omium.org> >> wrote: >> > Argh...too much wordsmithing... >> > >> > On Thu, Sep 26, 2013 at 12:22 PM, Grant Grundler <grundler@...omium.org> >> > wrote: >> >> Following 7 patches are mostly cleanup with one key patch around >> >> host->areq >> >> locking. The host->areq locking problem description is here: >> >> http://www.spinics.net/lists/linux-mmc/msg21644.html >> >> >> >> I do believe this preposed host->areq locking patch is a complete fix. >> > >> > ... is NOT a complete fix. >> > >> >> But it appears to fix the problem and is better than nothing. >> > >> > Still true. >> > >> > cheers, >> > grant >> > >> >> >> >> This patch sequence applies clean to linus' 3.12-rc2 branch and only >> >> compile >> >> tested in this form. This is a forward port (and cleanup) of the >> >> same patch I've been testing on ChromeOS-3.4 tree using Exynos5250 >> >> chipset. >> >> >> >> cheers, >> >> grant >> >> >> >> 0000-mmc-core-cleanups-and-locking-description (this email) >> >> 0001-mmc-core-rename-data-to-saved_areq.patch >> >> 0002-mmc-core-rename-local-var-err-to-saved_err.patch >> >> 0003-mmc-core-restructure-error-handling-for-start-req.patch >> >> 0004-mmc-core-use-common-code-path-to-return-error.patch >> >> 0005-mmc-core-handling-polling-more-gracefully.patch >> >> 0006-mmc-core-protect-references-to-host-areq-with-host-l.patch >> >> 0007-mmc-core-mmc_start_req-is-a-misnomer-mmc_process_are.patch >> >> >> >> >> >> drivers/mmc/card/block.c | 8 ++-- >> >> drivers/mmc/card/mmc_test.c | 4 +- >> >> drivers/mmc/core/core.c | 103 >> >> +++++++++++++++++++++++++------------------- >> >> include/linux/mmc/core.h | 2 +- >> >> 4 files changed, 66 insertions(+), 51 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists