Re: [git pull] device mapper fixes for 5.19-rc1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 1, 2022 at 1:59 PM Mike Snitzer <snitzer@xxxxxxxxxx> wrote:
>
> ----------------------------------------------------------------
> - Fix DM core's dm_table_supports_poll to return false if no data
>   devices.

So looking at that one (mainly because of the incomprehensible
explanation), I do note:

 (a) the caller does

        for (i = 0; i < t->num_targets; i++) {
                ti = t->targets + i;

    while the callee does

        unsigned i = 0;

        while (i < dm_table_get_num_targets(t)) {
                ti = dm_table_get_target(t, i++);

Now, those things are entirely equivalent, but that latter form is
likely to generate horribly bad code because those helper functions
aren't some kind of trivial inline, they are actually normal functions
that are defined later in that same source file.

Maybe a compiler will do optimizations within that source file even
for functions that haven't been defined yet. Traditionally not.

Whatever. Probably not a case where anybody cares about performance,
but it does strike me that the "use abstractions" version probably not
only generates worse code, it seems less legible too.

Very odd pattern.

 (b) The commit message (which is why I started looking at this) says
that it used to return true even if there are no data devices.

     But dm_table_supports_poll() actually _still_ returns true for at
least one case of no data devices: if the dm_table has no targets at
all.

So I don't know. Maybe that is a "can't happen". But since I looked at
this, I thought I'd just point out the two oddities I found while
doing so.

                    Linus

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel




[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux