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: Mon, 1 Jun 2020 00:13:21 +0300 From: Vladimir Oltean <olteanv@...il.com> To: Andy Shevchenko <andy.shevchenko@...il.com> Cc: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>, "arnd@...db.de" <arnd@...db.de>, "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>, "sergei.shtylyov@...entembedded.com" <sergei.shtylyov@...entembedded.com>, "bgolaszewski@...libre.com" <bgolaszewski@...libre.com>, "mika.westerberg@...ux.intel.com" <mika.westerberg@...ux.intel.com>, "efremov@...ux.com" <efremov@...ux.com>, "ztuowen@...il.com" <ztuowen@...il.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org> Subject: Re: [PATCH v2] devres: keep both device name and resource name in pretty name On Mon, 1 Jun 2020 at 00:05, Andy Shevchenko <andy.shevchenko@...il.com> wrote: > > > > On Sunday, May 31, 2020, Vladimir Oltean <olteanv@...il.com> wrote: >> >> From: Vladimir Oltean <vladimir.oltean@....com> >> >> Sometimes debugging a device is easiest using devmem on its register >> map, and that can be seen with /proc/iomem. But some device drivers have >> many memory regions. Take for example a networking switch. Its memory >> map used to look like this in /proc/iomem: >> >> 1fc000000-1fc3fffff : pcie@...000000 >> 1fc000000-1fc3fffff : 0000:00:00.5 >> 1fc010000-1fc01ffff : sys >> 1fc030000-1fc03ffff : rew >> 1fc060000-1fc0603ff : s2 >> 1fc070000-1fc0701ff : devcpu_gcb >> 1fc080000-1fc0800ff : qs >> 1fc090000-1fc0900cb : ptp >> 1fc100000-1fc10ffff : port0 >> 1fc110000-1fc11ffff : port1 >> 1fc120000-1fc12ffff : port2 >> 1fc130000-1fc13ffff : port3 >> 1fc140000-1fc14ffff : port4 >> 1fc150000-1fc15ffff : port5 >> 1fc200000-1fc21ffff : qsys >> 1fc280000-1fc28ffff : ana >> >> But after the patch in Fixes: was applied, the information is now >> presented in a much more opaque way: >> >> 1fc000000-1fc3fffff : pcie@...000000 >> 1fc000000-1fc3fffff : 0000:00:00.5 >> 1fc010000-1fc01ffff : 0000:00:00.5 >> 1fc030000-1fc03ffff : 0000:00:00.5 >> 1fc060000-1fc0603ff : 0000:00:00.5 >> 1fc070000-1fc0701ff : 0000:00:00.5 >> 1fc080000-1fc0800ff : 0000:00:00.5 >> 1fc090000-1fc0900cb : 0000:00:00.5 >> 1fc100000-1fc10ffff : 0000:00:00.5 >> 1fc110000-1fc11ffff : 0000:00:00.5 >> 1fc120000-1fc12ffff : 0000:00:00.5 >> 1fc130000-1fc13ffff : 0000:00:00.5 >> 1fc140000-1fc14ffff : 0000:00:00.5 >> 1fc150000-1fc15ffff : 0000:00:00.5 >> 1fc200000-1fc21ffff : 0000:00:00.5 >> 1fc280000-1fc28ffff : 0000:00:00.5 >> >> That patch made a fair comment that /proc/iomem might be confusing when >> it shows resources without an associated device, but we can do better >> than just hide the resource name altogether. Namely, we can print the >> device name _and_ the resource name. Like this: >> >> 1fc000000-1fc3fffff : pcie@...000000 >> 1fc000000-1fc3fffff : 0000:00:00.5 >> 1fc010000-1fc01ffff : 0000:00:00.5 sys >> 1fc030000-1fc03ffff : 0000:00:00.5 rew >> 1fc060000-1fc0603ff : 0000:00:00.5 s2 >> 1fc070000-1fc0701ff : 0000:00:00.5 devcpu_gcb >> 1fc080000-1fc0800ff : 0000:00:00.5 qs >> 1fc090000-1fc0900cb : 0000:00:00.5 ptp >> 1fc100000-1fc10ffff : 0000:00:00.5 port0 >> 1fc110000-1fc11ffff : 0000:00:00.5 port1 >> 1fc120000-1fc12ffff : 0000:00:00.5 port2 >> 1fc130000-1fc13ffff : 0000:00:00.5 port3 >> 1fc140000-1fc14ffff : 0000:00:00.5 port4 >> 1fc150000-1fc15ffff : 0000:00:00.5 port5 >> 1fc200000-1fc21ffff : 0000:00:00.5 qsys >> 1fc280000-1fc28ffff : 0000:00:00.5 ana >> > > All of this seems an ABI change. > But also see below. > Yes, indeed. What should I understand from your comment though? >> >> Fixes: 8d84b18f5678 ("devres: always use dev_name() in devm_ioremap_resource()") >> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com> >> --- >> lib/devres.c | 17 ++++++++++++++++- >> 1 file changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/lib/devres.c b/lib/devres.c >> index 6ef51f159c54..3d67588c15a7 100644 >> --- a/lib/devres.c >> +++ b/lib/devres.c >> @@ -119,6 +119,7 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res, >> { >> resource_size_t size; >> void __iomem *dest_ptr; >> + char *pretty_name; >> >> BUG_ON(!dev); >> >> @@ -129,7 +130,21 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res, >> >> size = resource_size(res); >> >> - if (!devm_request_mem_region(dev, res->start, size, dev_name(dev))) { >> + if (res->name) { >> + int len = strlen(dev_name(dev)) + strlen(res->name) + 2; >> + >> + pretty_name = devm_kzalloc(dev, len, GFP_KERNEL); >> + if (!pretty_name) >> + return IOMEM_ERR_PTR(-ENOMEM); >> + >> + sprintf(pretty_name, "%s %s", dev_name(dev), res->name); > > > Reimplementing devm_kasprintf(), why? > Mostly because I didn't remember how it was called, thanks for pointing it out. >> >> + } else { >> + pretty_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL); >> + if (!pretty_name) >> + return IOMEM_ERR_PTR(-ENOMEM); >> + } >> + >> + if (!devm_request_mem_region(dev, res->start, size, pretty_name)) { >> dev_err(dev, "can't request region for resource %pR\n", res); >> return IOMEM_ERR_PTR(-EBUSY); >> } >> -- >> 2.25.1 >> > > > -- > With Best Regards, > Andy Shevchenko > > Thanks, -Vladimir
Powered by blists - more mailing lists