Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1672043 > unrolled thread

Re: [PATCH v2] um: add dummy ioremap and iounmap functions

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2] um: add dummy ioremap and iounmap functions Logan Gunthorpe <logang@deltatee.com> - 2017-06-21 23:30 +0200
    Re: [PATCH v2] um: add dummy ioremap and iounmap functions Arnd Bergmann <arnd@arndb.de> - 2017-06-21 23:40 +0200
    Re: [PATCH v2] um: add dummy ioremap and iounmap functions Logan Gunthorpe <logang@deltatee.com> - 2017-06-21 23:40 +0200
    Re: [PATCH v2] um: add dummy ioremap and iounmap functions Richard Weinberger <richard@nod.at> - 2017-06-21 23:40 +0200

#1672043 — Re: [PATCH v2] um: add dummy ioremap and iounmap functions

Poke.

On 08/06/17 01:17 PM, Richard Weinberger wrote:
> Am 08.06.2017 um 20:53 schrieb Logan Gunthorpe:
> IMHO an ifdef in scatterlist code does not hurt.
> It is equally ugly than mocking ioremap for UML.

I disagree. Having ifdefs scattered around all call sites of a function
is *much* worse than having an extra mock function tucked away in a
header file somewhere.

It's a very common pattern in the kernel for providers of functions that
depend on a configuration option to provide mock functions when the
configuration option is not selected. This prevents needing every caller
of said function to put #ifdefs around the call. For a few examples:

include/linux/blkdev.h:1952
include/linux/dax.h:24
include/linux/pci.h:1329

And, frankly, it's _exactly_ what Linus Torvalds himself was arguing
against in the link I sent up-thread [1].

> So, I'm puzzled.
> Arnd, what do you think?
> Shall !HAS_IOMEM archs just mock these functions?

So, once again, 'um' is now the only architecture that has this problem
since tile and s390 accepted my patches (and without a fuss too). So can
you please consider merging this patch or proposing something that will
also fix the problem?

Thanks,

Logan


[1] http://yarchive.net/comp/linux/ifdefs.html

[toc] | [next] | [standalone]

#1672070

On Wed, Jun 21, 2017 at 11:31 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
>
>
> On 21/06/17 03:30 PM, Richard Weinberger wrote:
>> Since Arnd stays silent and um is the only remaining odd-ball, let's merge this
>> in v4.13.
>
> Great! Thanks!

Sorry for missing most of the discussion earlier. The last proposed version
(returning NULL to always fail) seems fine to me, too.

       Arnd

[toc] | [prev] | [next] | [standalone]

#1672075

On 21/06/17 03:30 PM, Richard Weinberger wrote:
> Since Arnd stays silent and um is the only remaining odd-ball, let's merge this
> in v4.13.

Great! Thanks!

Logan

[toc] | [prev] | [next] | [standalone]

#1672079

Logan,

Am 21.06.2017 um 23:25 schrieb Logan Gunthorpe:
> Poke.
> 
> On 08/06/17 01:17 PM, Richard Weinberger wrote:
>> Am 08.06.2017 um 20:53 schrieb Logan Gunthorpe:
>> IMHO an ifdef in scatterlist code does not hurt.
>> It is equally ugly than mocking ioremap for UML.
> 
> I disagree. Having ifdefs scattered around all call sites of a function
> is *much* worse than having an extra mock function tucked away in a
> header file somewhere.
> 
> It's a very common pattern in the kernel for providers of functions that
> depend on a configuration option to provide mock functions when the
> configuration option is not selected. This prevents needing every caller
> of said function to put #ifdefs around the call. For a few examples:
> 
> include/linux/blkdev.h:1952
> include/linux/dax.h:24
> include/linux/pci.h:1329
> 
> And, frankly, it's _exactly_ what Linus Torvalds himself was arguing
> against in the link I sent up-thread [1].
> 
>> So, I'm puzzled.
>> Arnd, what do you think?
>> Shall !HAS_IOMEM archs just mock these functions?
> 
> So, once again, 'um' is now the only architecture that has this problem
> since tile and s390 accepted my patches (and without a fuss too). So can
> you please consider merging this patch or proposing something that will
> also fix the problem?

Since Arnd stays silent and um is the only remaining odd-ball, let's merge this
in v4.13.

Thanks,
//richard

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web