Re: [PATCH 5/9] libata: implement and use SHT initializers and ops inheritance

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

 



Jeff Garzik wrote:
> Two comments:
> 
> 1) Please split into SHT and ops patches (SHT first, I presume)

Sure.

> 2) It seems like inheritance would be easier and less error-prone if the
> ops were copied, rather than modifying the structures in-place.  Comments?

I thought about making per-port copies on host allocation or host start.
 The pros of such approach would be...

* LLD's ops table can be made const.

* LLD can modify ops table freely after the copying happens.  To take
advantage of this, host allocation is probably the best place to copy
the ops tables.

Cons are...

* It changes the way a libata LLD should handle ops table.  The current
usage model is based on that ops table is used directly and shared and a
separate table should be defined for different variants.  I think it's
better to keep the current way and inheritance makes it easy.  This
almost cancels out the second pro point.

* More overhead (doesn't really matter tho).

I don't see where modifying the ops table in-place is more difficult or
error-prone.  Only LLD's own ops tables need to drop const and it's not
like ops tables are directly manipulated by any code other than the
finalization code.

Another thing is that what was needed here was static class inheritance
and the in-place finalization is run-time variant of inheritance
resolving done by OO compilers.  I think it's nice to keep that familiar
semantics.

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux