Re: [PATCH 03/27] iomap: mark the iomap argument to iomap_sector const
On Mon, Jul 19, 2021 at 09:08:20AM -0700, Darrick J. Wong wrote: > IMHO, constifiying functions is a good way to signal to /programmers/ > that they're not intended to touch the arguments, so Yes, that is the point here. Basically the iomap and iter should be pretty much const, and we almost get there except for the odd size changed flag for gfs2.
Re: [PATCH 03/27] iomap: mark the iomap argument to iomap_sector const
On 19.07.21 г. 19:08, Darrick J. Wong wrote:
> On Mon, Jul 19, 2021 at 12:34:56PM +0200, Christoph Hellwig wrote:
>> Signed-off-by: Christoph Hellwig
>
> /me wonders, does this have any significant effect on the generated
> code?
https://theartofmachinery.com/2019/08/12/c_const_isnt_for_performance.html
>
> It's probably a good idea to feed the optimizer as much usage info as we
> can, though I would imagine that for such a simple function it can
> probably tell that we don't change *iomap.
>
> IMHO, constifiying functions is a good way to signal to /programmers/
> that they're not intended to touch the arguments, so
>
> Reviewed-by: Darrick J. Wong
>
> --D
>
>> ---
>> include/linux/iomap.h | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
>> index 093519d91cc9cc..f9c36df6a3061b 100644
>> --- a/include/linux/iomap.h
>> +++ b/include/linux/iomap.h
>> @@ -91,8 +91,7 @@ struct iomap {
>> const struct iomap_page_ops *page_ops;
>> };
>>
>> -static inline sector_t
>> -iomap_sector(struct iomap *iomap, loff_t pos)
>> +static inline sector_t iomap_sector(const struct iomap *iomap, loff_t pos)
>> {
>> return (iomap->addr + pos - iomap->offset) >> SECTOR_SHIFT;
>> }
>> --
>> 2.30.2
>>
>
Re: [PATCH 03/27] iomap: mark the iomap argument to iomap_sector const
On Mon, Jul 19, 2021 at 12:34:56PM +0200, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig
/me wonders, does this have any significant effect on the generated
code?
It's probably a good idea to feed the optimizer as much usage info as we
can, though I would imagine that for such a simple function it can
probably tell that we don't change *iomap.
IMHO, constifiying functions is a good way to signal to /programmers/
that they're not intended to touch the arguments, so
Reviewed-by: Darrick J. Wong
--D
> ---
> include/linux/iomap.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 093519d91cc9cc..f9c36df6a3061b 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -91,8 +91,7 @@ struct iomap {
> const struct iomap_page_ops *page_ops;
> };
>
> -static inline sector_t
> -iomap_sector(struct iomap *iomap, loff_t pos)
> +static inline sector_t iomap_sector(const struct iomap *iomap, loff_t pos)
> {
> return (iomap->addr + pos - iomap->offset) >> SECTOR_SHIFT;
> }
> --
> 2.30.2
>
